fix: Improve WhatsApp template message error handling (#12168)
WhatsApp template message errors were not being properly handled because the `@message instance` variable was only set in the `send_message` method but not in `send_template`. When template sending failed, the `handle_error` method couldn't update the message status due to the missing @message reference, resulting in silent failures with no user feedback.
This commit is contained in:
@@ -84,7 +84,7 @@ class Whatsapp::OneoffCampaignService
|
|||||||
namespace: namespace,
|
namespace: namespace,
|
||||||
lang_code: lang_code,
|
lang_code: lang_code,
|
||||||
parameters: processed_parameters
|
parameters: processed_parameters
|
||||||
})
|
}, nil)
|
||||||
|
|
||||||
rescue StandardError => e
|
rescue StandardError => e
|
||||||
Rails.logger.error "Failed to send WhatsApp template message to #{to}: #{e.message}"
|
Rails.logger.error "Failed to send WhatsApp template message to #{to}: #{e.message}"
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ class Whatsapp::Providers::BaseService
|
|||||||
raise 'Overwrite this method in child class'
|
raise 'Overwrite this method in child class'
|
||||||
end
|
end
|
||||||
|
|
||||||
def send_template(_phone_number, _template_info)
|
def send_template(_phone_number, _template_info, _message)
|
||||||
raise 'Overwrite this method in child class'
|
raise 'Overwrite this method in child class'
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -31,27 +31,27 @@ class Whatsapp::Providers::BaseService
|
|||||||
raise 'Overwrite this method in child class'
|
raise 'Overwrite this method in child class'
|
||||||
end
|
end
|
||||||
|
|
||||||
def process_response(response)
|
def process_response(response, message)
|
||||||
parsed_response = response.parsed_response
|
parsed_response = response.parsed_response
|
||||||
if response.success? && parsed_response['error'].blank?
|
if response.success? && parsed_response['error'].blank?
|
||||||
parsed_response['messages'].first['id']
|
parsed_response['messages'].first['id']
|
||||||
else
|
else
|
||||||
handle_error(response)
|
handle_error(response, message)
|
||||||
nil
|
nil
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_error(response)
|
def handle_error(response, message)
|
||||||
Rails.logger.error response.body
|
Rails.logger.error response.body
|
||||||
return if @message.blank?
|
return if message.blank?
|
||||||
|
|
||||||
# https://developers.facebook.com/docs/whatsapp/cloud-api/support/error-codes/#sample-response
|
# https://developers.facebook.com/docs/whatsapp/cloud-api/support/error-codes/#sample-response
|
||||||
error_message = error_message(response)
|
error_message = error_message(response)
|
||||||
return if error_message.blank?
|
return if error_message.blank?
|
||||||
|
|
||||||
@message.external_error = error_message
|
message.external_error = error_message
|
||||||
@message.status = :failed
|
message.status = :failed
|
||||||
@message.save!
|
message.save!
|
||||||
end
|
end
|
||||||
|
|
||||||
def create_buttons(items)
|
def create_buttons(items)
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ class Whatsapp::Providers::Whatsapp360DialogService < Whatsapp::Providers::BaseS
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def send_template(phone_number, template_info)
|
def send_template(phone_number, template_info, message)
|
||||||
response = HTTParty.post(
|
response = HTTParty.post(
|
||||||
"#{api_base_path}/messages",
|
"#{api_base_path}/messages",
|
||||||
headers: api_headers,
|
headers: api_headers,
|
||||||
@@ -21,7 +21,7 @@ class Whatsapp::Providers::Whatsapp360DialogService < Whatsapp::Providers::BaseS
|
|||||||
}.to_json
|
}.to_json
|
||||||
)
|
)
|
||||||
|
|
||||||
process_response(response)
|
process_response(response, message)
|
||||||
end
|
end
|
||||||
|
|
||||||
def sync_templates
|
def sync_templates
|
||||||
@@ -68,7 +68,7 @@ class Whatsapp::Providers::Whatsapp360DialogService < Whatsapp::Providers::BaseS
|
|||||||
}.to_json
|
}.to_json
|
||||||
)
|
)
|
||||||
|
|
||||||
process_response(response)
|
process_response(response, message)
|
||||||
end
|
end
|
||||||
|
|
||||||
def send_attachment_message(phone_number, message)
|
def send_attachment_message(phone_number, message)
|
||||||
@@ -90,7 +90,7 @@ class Whatsapp::Providers::Whatsapp360DialogService < Whatsapp::Providers::BaseS
|
|||||||
}.to_json
|
}.to_json
|
||||||
)
|
)
|
||||||
|
|
||||||
process_response(response)
|
process_response(response, message)
|
||||||
end
|
end
|
||||||
|
|
||||||
def error_message(response)
|
def error_message(response)
|
||||||
@@ -123,6 +123,6 @@ class Whatsapp::Providers::Whatsapp360DialogService < Whatsapp::Providers::BaseS
|
|||||||
}.to_json
|
}.to_json
|
||||||
)
|
)
|
||||||
|
|
||||||
process_response(response)
|
process_response(response, message)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -11,7 +11,7 @@ class Whatsapp::Providers::WhatsappCloudService < Whatsapp::Providers::BaseServi
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def send_template(phone_number, template_info)
|
def send_template(phone_number, template_info, message)
|
||||||
template_body = template_body_parameters(template_info)
|
template_body = template_body_parameters(template_info)
|
||||||
|
|
||||||
request_body = {
|
request_body = {
|
||||||
@@ -28,7 +28,7 @@ class Whatsapp::Providers::WhatsappCloudService < Whatsapp::Providers::BaseServi
|
|||||||
body: request_body.to_json
|
body: request_body.to_json
|
||||||
)
|
)
|
||||||
|
|
||||||
process_response(response)
|
process_response(response, message)
|
||||||
end
|
end
|
||||||
|
|
||||||
def sync_templates
|
def sync_templates
|
||||||
@@ -92,7 +92,7 @@ class Whatsapp::Providers::WhatsappCloudService < Whatsapp::Providers::BaseServi
|
|||||||
}.to_json
|
}.to_json
|
||||||
)
|
)
|
||||||
|
|
||||||
process_response(response)
|
process_response(response, message)
|
||||||
end
|
end
|
||||||
|
|
||||||
def send_attachment_message(phone_number, message)
|
def send_attachment_message(phone_number, message)
|
||||||
@@ -115,7 +115,7 @@ class Whatsapp::Providers::WhatsappCloudService < Whatsapp::Providers::BaseServi
|
|||||||
}.to_json
|
}.to_json
|
||||||
)
|
)
|
||||||
|
|
||||||
process_response(response)
|
process_response(response, message)
|
||||||
end
|
end
|
||||||
|
|
||||||
def error_message(response)
|
def error_message(response)
|
||||||
@@ -179,6 +179,6 @@ class Whatsapp::Providers::WhatsappCloudService < Whatsapp::Providers::BaseServi
|
|||||||
}.to_json
|
}.to_json
|
||||||
)
|
)
|
||||||
|
|
||||||
process_response(response)
|
process_response(response, message)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -33,7 +33,7 @@ class Whatsapp::SendOnWhatsappService < Base::SendOnChannelService
|
|||||||
namespace: namespace,
|
namespace: namespace,
|
||||||
lang_code: lang_code,
|
lang_code: lang_code,
|
||||||
parameters: processed_parameters
|
parameters: processed_parameters
|
||||||
})
|
}, message)
|
||||||
message.update!(source_id: message_id) if message_id.present?
|
message.update!(source_id: message_id) if message_id.present?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -133,7 +133,8 @@ describe Whatsapp::OneoffCampaignService do
|
|||||||
)
|
)
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
)
|
),
|
||||||
|
nil
|
||||||
)
|
)
|
||||||
|
|
||||||
described_class.new(campaign: campaign).perform
|
described_class.new(campaign: campaign).perform
|
||||||
@@ -164,8 +165,8 @@ describe Whatsapp::OneoffCampaignService do
|
|||||||
|
|
||||||
allow(whatsapp_channel).to receive(:send_template).and_return(nil)
|
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_error.phone_number, anything, nil).and_raise(StandardError, error_message)
|
||||||
expect(whatsapp_channel).to receive(:send_template).with(contact_success.phone_number, anything).once
|
expect(whatsapp_channel).to receive(:send_template).with(contact_success.phone_number, anything, nil).once
|
||||||
|
|
||||||
expect(Rails.logger).to receive(:error)
|
expect(Rails.logger).to receive(:error)
|
||||||
.with("Failed to send WhatsApp template message to #{contact_error.phone_number}: #{error_message}")
|
.with("Failed to send WhatsApp template message to #{contact_error.phone_number}: #{error_message}")
|
||||||
|
|||||||
@@ -187,7 +187,7 @@ describe Whatsapp::Providers::WhatsappCloudService do
|
|||||||
)
|
)
|
||||||
.to_return(status: 200, body: whatsapp_response.to_json, headers: response_headers)
|
.to_return(status: 200, body: whatsapp_response.to_json, headers: response_headers)
|
||||||
|
|
||||||
expect(service.send_template('+123456789', template_info)).to eq('message_id')
|
expect(service.send_template('+123456789', template_info, message)).to eq('message_id')
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
@@ -287,7 +287,7 @@ describe Whatsapp::Providers::WhatsappCloudService do
|
|||||||
context 'when there is a message' do
|
context 'when there is a message' do
|
||||||
it 'logs error and updates message status' do
|
it 'logs error and updates message status' do
|
||||||
service.instance_variable_set(:@message, message)
|
service.instance_variable_set(:@message, message)
|
||||||
service.send(:handle_error, error_response_object)
|
service.send(:handle_error, error_response_object, message)
|
||||||
|
|
||||||
expect(message.reload.status).to eq('failed')
|
expect(message.reload.status).to eq('failed')
|
||||||
expect(message.reload.external_error).to eq(error_message)
|
expect(message.reload.external_error).to eq(error_message)
|
||||||
@@ -305,7 +305,7 @@ describe Whatsapp::Providers::WhatsappCloudService do
|
|||||||
|
|
||||||
it 'logs error but does not update message' do
|
it 'logs error but does not update message' do
|
||||||
service.instance_variable_set(:@message, message)
|
service.instance_variable_set(:@message, message)
|
||||||
service.send(:handle_error, error_response_object)
|
service.send(:handle_error, error_response_object, message)
|
||||||
|
|
||||||
expect(message.reload.status).not_to eq('failed')
|
expect(message.reload.status).not_to eq('failed')
|
||||||
expect(message.reload.external_error).to be_nil
|
expect(message.reload.external_error).to be_nil
|
||||||
|
|||||||
Reference in New Issue
Block a user