mirror of
https://github.com/lingble/chatwoot.git
synced 2025-10-29 18:22:53 +00:00
fix: Error shouldn't halt the campaign for entire audience (#11980)
## Summary - handle Twilio failures per contact when running one-off SMS campaigns - rescue errors in WhatsApp and generic SMS one-off campaigns so they continue - add specs confirming campaigns continue sending when a single contact fails fixes: https://github.com/chatwoot/chatwoot/issues/9000 Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
This commit is contained in:
@@ -29,5 +29,7 @@ class Sms::OneoffSmsCampaignService
|
||||
|
||||
def send_message(to:, content:)
|
||||
channel.send_text_message(to, content)
|
||||
rescue StandardError => e
|
||||
Rails.logger.error("[SMS Campaign #{campaign.id}] Failed to send to #{to}: #{e.message}")
|
||||
end
|
||||
end
|
||||
|
||||
@@ -23,7 +23,13 @@ class Twilio::OneoffSmsCampaignService
|
||||
next if contact.phone_number.blank?
|
||||
|
||||
content = Liquid::CampaignTemplateService.new(campaign: campaign, contact: contact).call(campaign.message)
|
||||
channel.send_message(to: contact.phone_number, body: content)
|
||||
|
||||
begin
|
||||
channel.send_message(to: contact.phone_number, body: content)
|
||||
rescue Twilio::REST::TwilioError, Twilio::REST::RestError => e
|
||||
Rails.logger.error("[Twilio Campaign #{campaign.id}] Failed to send to #{contact.phone_number}: #{e.message}")
|
||||
next
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -89,6 +89,7 @@ class Whatsapp::OneoffCampaignService
|
||||
rescue StandardError => e
|
||||
Rails.logger.error "Failed to send WhatsApp template message to #{to}: #{e.message}"
|
||||
Rails.logger.error "Backtrace: #{e.backtrace.first(5).join('\n')}"
|
||||
raise e
|
||||
# continue processing remaining contacts
|
||||
nil
|
||||
end
|
||||
end
|
||||
|
||||
@@ -20,6 +20,7 @@ describe Sms::OneoffSmsCampaignService do
|
||||
body: { 'id' => '1' }.to_json,
|
||||
headers: {}
|
||||
)
|
||||
allow_any_instance_of(described_class).to receive(:channel).and_return(sms_channel) # rubocop:disable RSpec/AnyInstance
|
||||
end
|
||||
|
||||
it 'raises error if the campaign is completed' do
|
||||
@@ -52,5 +53,21 @@ describe Sms::OneoffSmsCampaignService do
|
||||
|
||||
sms_campaign_service.perform
|
||||
end
|
||||
|
||||
it 'continues processing contacts when sending message raises an error' do
|
||||
contact_error, contact_success = FactoryBot.create_list(:contact, 2, :with_phone_number, account: account)
|
||||
contact_error.update_labels([label1.title])
|
||||
contact_success.update_labels([label1.title])
|
||||
|
||||
error_message = 'SMS provider error'
|
||||
|
||||
expect(sms_channel).to receive(:send_text_message).with(contact_error.phone_number, anything).and_raise(StandardError, error_message)
|
||||
expect(sms_channel).to receive(:send_text_message).with(contact_success.phone_number, anything).and_return(nil)
|
||||
|
||||
expect(Rails.logger).to receive(:error).with("[SMS Campaign #{campaign.id}] Failed to send to #{contact_error.phone_number}: #{error_message}")
|
||||
|
||||
sms_campaign_service.perform
|
||||
expect(campaign.reload.completed?).to be true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -70,5 +70,36 @@ describe Twilio::OneoffSmsCampaignService do
|
||||
|
||||
sms_campaign_service.perform
|
||||
end
|
||||
|
||||
it 'continues processing contacts when Twilio raises an error' do
|
||||
contact_error, contact_success = FactoryBot.create_list(:contact, 2, :with_phone_number, account: account)
|
||||
contact_error.update_labels([label1.title])
|
||||
contact_success.update_labels([label1.title])
|
||||
|
||||
error = Twilio::REST::TwilioError.new("The 'To' number #{contact_error.phone_number} is not a valid phone number.")
|
||||
|
||||
allow(twilio_messages).to receive(:create).and_return(nil)
|
||||
|
||||
expect(twilio_messages).to receive(:create).with(
|
||||
body: campaign.message,
|
||||
messaging_service_sid: twilio_sms.messaging_service_sid,
|
||||
to: contact_error.phone_number,
|
||||
status_callback: 'http://localhost:3000/twilio/delivery_status'
|
||||
).and_raise(error)
|
||||
|
||||
expect(twilio_messages).to receive(:create).with(
|
||||
body: campaign.message,
|
||||
messaging_service_sid: twilio_sms.messaging_service_sid,
|
||||
to: contact_success.phone_number,
|
||||
status_callback: 'http://localhost:3000/twilio/delivery_status'
|
||||
).once
|
||||
|
||||
expect(Rails.logger).to receive(:error).with(
|
||||
"[Twilio Campaign #{campaign.id}] Failed to send to #{contact_error.phone_number}: #{error.message}"
|
||||
)
|
||||
|
||||
sms_campaign_service.perform
|
||||
expect(campaign.reload.completed?).to be true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -151,18 +151,23 @@ describe Whatsapp::OneoffCampaignService do
|
||||
end
|
||||
|
||||
context 'when send_template raises an error' do
|
||||
it 'logs error and re-raises' do
|
||||
contact = create(:contact, :with_phone_number, account: account)
|
||||
contact.update_labels([label1.title])
|
||||
it 'logs error and continues processing remaining contacts' do
|
||||
contact_error, contact_success = create_list(:contact, 2, :with_phone_number, account: account)
|
||||
contact_error.update_labels([label1.title])
|
||||
contact_success.update_labels([label1.title])
|
||||
error_message = 'WhatsApp API error'
|
||||
|
||||
allow(whatsapp_channel).to receive(:send_template).and_raise(StandardError, error_message)
|
||||
allow(whatsapp_channel).to receive(:send_template).and_return(nil)
|
||||
|
||||
expect(whatsapp_channel).to receive(:send_template).with(contact_error.phone_number, anything).and_raise(StandardError, error_message)
|
||||
expect(whatsapp_channel).to receive(:send_template).with(contact_success.phone_number, anything).once
|
||||
|
||||
expect(Rails.logger).to receive(:error)
|
||||
.with("Failed to send WhatsApp template message to #{contact.phone_number}: #{error_message}")
|
||||
.with("Failed to send WhatsApp template message to #{contact_error.phone_number}: #{error_message}")
|
||||
expect(Rails.logger).to receive(:error).with(/Backtrace:/)
|
||||
|
||||
expect { described_class.new(campaign: campaign).perform }.to raise_error(StandardError, error_message)
|
||||
described_class.new(campaign: campaign).perform
|
||||
expect(campaign.reload.completed?).to be true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user