fix: Handle application errors in Sentry (#1982)

- Handle notification errors for attachment messages
- Fix empty identifiers being passed
- Fix 404 when invalid source id
- Handle webhook exceptions

app/actions/contact_identify_action.rb

@@ -34,7 +34,9 @@ class ContactIdentifyAction
 
   def update_contact
     custom_attributes = params[:custom_attributes] ? @contact.custom_attributes.merge(params[:custom_attributes]) : @contact.custom_attributes
-    @contact.update!(params.slice(:name, :email, :identifier).merge({ custom_attributes: custom_attributes }))
+    # blank identifier or email will throw unique index error
+    # TODO: replace reject { |_k, v| v.blank? } with compact_blank when rails is upgraded
+    @contact.update!(params.slice(:name, :email, :identifier).reject { |_k, v| v.blank? }.merge({ custom_attributes: custom_attributes }))
     ContactAvatarJob.perform_later(@contact, params[:avatar_url]) if params[:avatar_url].present?
   end
 

app/controllers/api/v1/widget/base_controller.rb

@@ -32,7 +32,8 @@ class Api::V1::Widget::BaseController < ApplicationController
     @contact_inbox = @web_widget.inbox.contact_inboxes.find_by(
       source_id: auth_token_params[:source_id]
     )
-    @contact = @contact_inbox.contact
+    @contact = @contact_inbox&.contact
+    raise ActiveRecord::RecordNotFound unless @contact
   end
 
   def create_conversation

app/mailboxes/application_mailbox.rb

@@ -6,7 +6,7 @@ class ApplicationMailbox < ActionMailbox::Base
   def self.reply_mail?
     proc do |inbound_mail_obj|
       is_a_reply_email = false
-      inbound_mail_obj.mail.to.each do |email|
+      inbound_mail_obj.mail.to&.each do |email|
         username = email.split('@')[0]
         match_result = username.match(REPLY_EMAIL_USERNAME_PATTERN)
         if match_result
@@ -21,7 +21,7 @@ class ApplicationMailbox < ActionMailbox::Base
   def self.support_mail?
     proc do |inbound_mail_obj|
       is_a_support_email = false
-      inbound_mail_obj.mail.to.each do |email|
+      inbound_mail_obj.mail.to&.each do |email|
         channel = Channel::Email.find_by('email = ? OR forward_to_email = ?', email, email)
         if channel.present?
           is_a_support_email = true

app/models/notification.rb

@@ -70,7 +70,7 @@ class Notification < ApplicationRecord
       I18n.t(
         'notifications.notification_title.assigned_conversation_new_message',
         display_id: conversation.display_id,
-        content: primary_actor.content.truncate_words(10)
+        content: primary_actor.content&.truncate_words(10)
       )
     when 'conversation_mention'
       I18n.t('notifications.notification_title.conversation_mention', display_id: conversation.display_id, name: secondary_actor.name)

lib/exception_list.rb

@@ -1,4 +1,5 @@
 module ExceptionList
   URI_EXCEPTIONS = [Errno::ETIMEDOUT, Errno::ECONNREFUSED, URI::InvalidURIError, Net::OpenTimeout, SocketError].freeze
-  REST_CLIENT_EXCEPTIONS = [RestClient::NotFound, RestClient::GatewayTimeout, RestClient::BadRequest, RestClient::MethodNotAllowed].freeze
+  REST_CLIENT_EXCEPTIONS = [RestClient::NotFound, RestClient::GatewayTimeout, RestClient::BadRequest,
+                            RestClient::MethodNotAllowed, RestClient::Forbidden].freeze
 end

spec/actions/contact_identify_action_spec.rb

@@ -44,5 +44,15 @@ describe ::ContactIdentifyAction do
         expect { contact.reload }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
+
+    context 'when contacts with blank identifiers exist and identify action is called with blank identifier' do
+      it 'updates the attributes of contact passed in to identify action' do
+        create(:contact, account: account, identifier: '')
+        params = { identifier: '', name: 'new name' }
+        result = described_class.new(contact: contact, params: params).perform
+        expect(result.id).to eq contact.id
+        expect(result.name).to eq 'new name'
+      end
+    end
   end
 end

spec/controllers/api/v1/widget/conversations_controller_spec.rb

@@ -25,6 +25,21 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do
         expect(json_response['status']).to eq(conversation.status)
       end
     end
+
+    context 'with a conversation but invalid source id' do
+      it 'returns the correct conversation params' do
+        allow(Rails.configuration.dispatcher).to receive(:dispatch)
+
+        payload = { source_id: 'invalid source id', inbox_id: web_widget.inbox.id }
+        token = ::Widget::TokenService.new(payload: payload).generate_token
+        get '/api/v1/widget/conversations',
+            headers: { 'X-Auth-Token' => token },
+            params: { website_token: web_widget.website_token },
+            as: :json
+
+        expect(response).to have_http_status(:not_found)
+      end
+    end
   end
 
   describe 'POST /api/v1/widget/conversations' do

spec/models/notification_spec.rb

@@ -41,6 +41,14 @@ RSpec.describe Notification do
 #{message.content.truncate_words(10)}"
     end
 
+    it 'returns appropriate title suited for the notification type assigned_conversation_new_message when attachment message' do
+      # checking content nil should be suffice for attachments
+      message = create(:message, sender: create(:user), content: nil)
+      notification = create(:notification, notification_type: 'assigned_conversation_new_message', primary_actor: message)
+
+      expect(notification.push_message_title).to eq "[New message] - ##{notification.conversation.display_id} "
+    end
+
     it 'returns appropriate title suited for the notification type conversation_mention' do
       message = create(:message, sender: create(:user))
       notification = create(:notification, notification_type: 'conversation_mention', primary_actor: message, secondary_actor: message.sender)