chore: Handle conversation participation creation race condition error (#9449)
We observed some race condition errors in the conversation participation listener while trying to create a conversation participation assignment. This PR handles this error and also adds additional debug information for future. fixes: https://linear.app/chatwoot/issue/CW-3296/activerecordrecordnotunique-pguniqueviolation-error-duplicate-key ## Changelog - handles `ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvald` errors so that they won't pollute sentry - Adds a debug statement to log the cases - Add previous_changes into the dispatcher so that we know the exact attribute changes which trigger `assignee_changed, team_changed` events ( would be handy in future )
This commit is contained in:
@@ -3,6 +3,13 @@ class ParticipationListener < BaseListener
|
|||||||
|
|
||||||
def assignee_changed(event)
|
def assignee_changed(event)
|
||||||
conversation, _account = extract_conversation_and_account(event)
|
conversation, _account = extract_conversation_and_account(event)
|
||||||
conversation.conversation_participants.find_or_create_by!(user_id: conversation.assignee_id) if conversation.assignee_id.present?
|
return if conversation.assignee_id.blank?
|
||||||
|
|
||||||
|
conversation.conversation_participants.find_or_create_by!(user_id: conversation.assignee_id)
|
||||||
|
# We have observed race conditions triggering these errors
|
||||||
|
# example: Assignment happening via automation, while auto assignment is also configured.
|
||||||
|
rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
|
||||||
|
Rails.logger.warn "Failed to create conversation participant for account #{conversation.account.id} " \
|
||||||
|
": user #{conversation.assignee_id} : conversation #{conversation.id}"
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -32,7 +32,7 @@ module AssignmentHandler
|
|||||||
ASSIGNEE_CHANGED => -> { saved_change_to_assignee_id? },
|
ASSIGNEE_CHANGED => -> { saved_change_to_assignee_id? },
|
||||||
TEAM_CHANGED => -> { saved_change_to_team_id? }
|
TEAM_CHANGED => -> { saved_change_to_team_id? }
|
||||||
}.each do |event, condition|
|
}.each do |event, condition|
|
||||||
condition.call && dispatcher_dispatch(event)
|
condition.call && dispatcher_dispatch(event, previous_changes)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -9,8 +9,6 @@ describe ParticipationListener do
|
|||||||
|
|
||||||
before do
|
before do
|
||||||
create(:inbox_member, inbox: inbox, user: agent)
|
create(:inbox_member, inbox: inbox, user: agent)
|
||||||
Current.user = nil
|
|
||||||
Current.account = nil
|
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#assignee_changed' do
|
describe '#assignee_changed' do
|
||||||
@@ -22,5 +20,19 @@ describe ParticipationListener do
|
|||||||
listener.assignee_changed(event)
|
listener.assignee_changed(event)
|
||||||
expect(conversation.conversation_participants.map(&:user_id)).to include(agent.id)
|
expect(conversation.conversation_participants.map(&:user_id)).to include(agent.id)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'does not fail if the conversation participant already exists' do
|
||||||
|
conversation.conversation_participants.create!(user: agent)
|
||||||
|
expect { listener.assignee_changed(event) }.not_to raise_error
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'logs a debug message if participant save fails due to a race condition' do
|
||||||
|
allow(Rails.logger).to receive(:warn)
|
||||||
|
allow(conversation).to receive(:conversation_participants).and_return(double)
|
||||||
|
allow(conversation.conversation_participants).to receive(:find_or_create_by!).and_raise(ActiveRecord::RecordNotUnique)
|
||||||
|
expect { listener.assignee_changed(event) }.not_to raise_error
|
||||||
|
expect(Rails.logger).to have_received(:warn).with('Failed to create conversation participant for account ' \
|
||||||
|
"#{account.id} : user #{agent.id} : conversation #{conversation.id}")
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -156,7 +156,7 @@ RSpec.describe Conversation do
|
|||||||
changed_attributes: nil, performed_by: nil)
|
changed_attributes: nil, performed_by: nil)
|
||||||
expect(Rails.configuration.dispatcher).to have_received(:dispatch)
|
expect(Rails.configuration.dispatcher).to have_received(:dispatch)
|
||||||
.with(described_class::ASSIGNEE_CHANGED, kind_of(Time), conversation: conversation, notifiable_assignee_change: true,
|
.with(described_class::ASSIGNEE_CHANGED, kind_of(Time), conversation: conversation, notifiable_assignee_change: true,
|
||||||
changed_attributes: nil, performed_by: nil)
|
changed_attributes: changed_attributes, performed_by: nil)
|
||||||
expect(Rails.configuration.dispatcher).to have_received(:dispatch)
|
expect(Rails.configuration.dispatcher).to have_received(:dispatch)
|
||||||
.with(described_class::CONVERSATION_UPDATED, kind_of(Time), conversation: conversation, notifiable_assignee_change: true,
|
.with(described_class::CONVERSATION_UPDATED, kind_of(Time), conversation: conversation, notifiable_assignee_change: true,
|
||||||
changed_attributes: changed_attributes, performed_by: nil)
|
changed_attributes: changed_attributes, performed_by: nil)
|
||||||
|
|||||||
Reference in New Issue
Block a user