4330: Better Twilio error handling (#2765)

* 4330: raise on all twilio errors

* fix message patients script

* Simplify the error handling

We can set the context as tags for Sentry, so that it will roll things
up nicely in the Sentry UI.  Also, we don't need to capture the message
as an ivar, in fact the original exception is captured by Ruby for us as
the `cause` - and Sentry will show details on that.

I'm adding the original message into the custom message we have just for
a bit easier scanning when looking at errors in Sentry.

* Count attempted messages, sent, and errors

* Maintain original response

Some calls depend on this

Co-authored-by: Rob Sanheim <rsanheim@gmail.com>

app/services/twilio_api_service.rb

@@ -11,7 +11,9 @@
 # are idempotent and retries would yield the same errors.
 
 class TwilioApiService
+  attr_accessor :communication_type
   attr_reader :client
+  attr_reader :metrics
   attr_reader :twilio_sender_sms_number
   attr_reader :twilio_sender_whatsapp_number
 
@@ -21,12 +23,6 @@ class TwilioApiService
   delegate :logger, to: Rails
 
   class Error < StandardError
-    attr_reader :exception_message, :context
-    def initialize(message, exception_message:, context:)
-      super(message)
-      @exception_message = exception_message
-      @context = context
-    end
   end
 
   def initialize(sms_sender: nil)
@@ -56,6 +52,7 @@ def initialize(sms_sender: nil)
     else
       prod_client
     end
+    @metrics = Metrics.with_object(self)
   end
 
   def test_mode?
@@ -71,12 +68,16 @@ def test_client
   end
 
   def send_sms(recipient_number:, message:, callback_url: nil, context: {})
+    self.communication_type = "sms"
+    metrics.increment("#{communication_type}.attempts")
     sender_number = twilio_sender_sms_number
 
     send_twilio_message(sender_number, recipient_number, message, callback_url, context)
   end
 
   def send_whatsapp(recipient_number:, message:, callback_url: nil, context: {})
+    self.communication_type = "whatsapp"
+    metrics.increment("#{communication_type}.attempts")
     sender_number = "whatsapp:" + twilio_sender_whatsapp_number
     recipient_number = "whatsapp:" + recipient_number
 
@@ -86,28 +87,17 @@ def send_whatsapp(recipient_number:, message:, callback_url: nil, context: {})
   private
 
   def send_twilio_message(sender_number, recipient_number, message, callback_url, context)
-    client.messages.create(
+    Sentry.set_tags(context)
+    response = client.messages.create(
       from: sender_number,
       to: recipient_number,
       status_callback: callback_url,
       body: message
     )
+    metrics.increment("#{communication_type}.sent")
+    response
   rescue Twilio::REST::TwilioError => exception
-    if exception.respond_to?(:code)
-      log_error(exception, sender_number, recipient_number, context)
-      nil
-    else
-      raise Error.new("Error while calling Twilio API", exception_message: exception.to_s, context: context)
-    end
-  end
-
-  def log_error(e, sender_number, recipient_number, context)
-    logger.info({
-      class: self.class.name,
-      error: e,
-      sender: sender_number,
-      recipient: recipient_number,
-      **context
-    })
+    metrics.increment("#{communication_type}.errors")
+    raise Error.new("Error while calling Twilio API: #{exception.message}")
   end
 end

lib/tasks/scripts/message_patients.rb

@@ -56,12 +56,9 @@ def send_messages
         end
       rescue TwilioApiService::Error
         update_report(:exception, patient: patient)
+        next
       end
-      if response
-        update_report(:responses, response: response, patient: patient)
-      else
-        update_report(:exception, patient: patient)
-      end
+      update_report(:responses, response: response, patient: patient)
     end
   end
 

spec/services/twilio_api_service_spec.rb

@@ -80,7 +80,7 @@ def stub_client
       )
     end
 
-    it "raises a custom error on twilio error that does not specify an error code" do
+    it "raises a custom error on twilio error" do
       stub_client
       allow(twilio_client).to receive_message_chain("messages.create").and_raise(Twilio::REST::TwilioError)
       expect {
@@ -91,15 +91,6 @@ def stub_client
         )
       }.to raise_error(TwilioApiService::Error)
     end
-
-    it "logs when twilio specifies an error code" do
-      expect(Rails).to receive_message_chain(:logger, :info)
-      notification_service.send_sms(
-        recipient_number: invalid_phone_number,
-        message: "test sms message",
-        callback_url: fake_callback_url
-      )
-    end
   end
 
   describe "#send_whatsapp" do
@@ -120,7 +111,7 @@ def stub_client
       )
     end
 
-    it "raises a custom error on twilio error that does not specify an error code" do
+    it "raises a custom error on twilio error" do
       stub_client
       allow(twilio_client).to receive_message_chain("messages.create").and_raise(Twilio::REST::TwilioError)
 
@@ -132,15 +123,5 @@ def stub_client
         )
       }.to raise_error(TwilioApiService::Error)
     end
-
-    it "logs when twilio specifies an error code" do
-      expect(Rails).to receive_message_chain(:logger, :info)
-
-      notification_service.send_whatsapp(
-        recipient_number: invalid_phone_number,
-        message: "test whatsapp message",
-        callback_url: fake_callback_url
-      )
-    end
   end
 end