From e7b4a339b3a21d0229db24632b41c3677e1d03c5 Mon Sep 17 00:00:00 2001 From: Steven Burnett Date: Thu, 14 Jan 2016 10:01:09 -0700 Subject: [PATCH] fix error handling with notifications sqs queue fixes CNVS-25481 Test-Plan: - change the sqs queue in a way to cause a failure. Notice message object reports as such and error is logged Change-Id: I5ac0b9ac720b41546aa605e835b3badcfbbd17fa Reviewed-on: https://gerrit.instructure.com/70262 Reviewed-by: Alex Boyd Tested-by: Jenkins Product-Review: Gentry Beckmann QA-Review: Gentry Beckmann --- app/models/message.rb | 25 +++++++++++++++++++----- spec/models/notification_service_spec.rb | 20 ++++++++++++++----- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/app/models/message.rb b/app/models/message.rb index 335751ec3cc..c9f170ade28 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -549,12 +549,27 @@ class Message < ActiveRecord::Base end if user.account.feature_enabled?(:notification_service) && path_type != "yo" - message_body = path_type == "email" ? Mailer.create_message(self).to_s : body - NotificationService.process(message_body, path_type, to, remote_configuration) - complete_dispatch - return + enqueue_to_sqs + else + send(delivery_method) end - send(delivery_method) + end + + def enqueue_to_sqs + message_body = path_type == "email" ? Mailer.create_message(self).to_s : body + NotificationService.process(message_body, path_type, to, remote_configuration) + complete_dispatch + rescue AWS::SQS::Errors::ServiceError => e + Canvas::Errors.capture( + e, + message: 'Message delivery failed', + to: to, + object: inspect.to_s + ) + error_string = "Exception: #{e.class}: #{e.message}\n\t#{e.backtrace.join("\n\t")}" + self.transmission_errors = error_string + self.errored_dispatch + raise end class RemoteConfigurationError < StandardError; end diff --git a/spec/models/notification_service_spec.rb b/spec/models/notification_service_spec.rb index 8337ec05602..c2af411b020 100644 --- a/spec/models/notification_service_spec.rb +++ b/spec/models/notification_service_spec.rb @@ -27,33 +27,43 @@ describe NotificationService do @message.user.account.root_account.enable_feature!(:notification_service) @message.save! end + before(:each) do + @queue = stub('notification queue') + NotificationService.stubs(:notification_queue).returns(@queue) + end describe "notification Service" do it "processes email message type" do - NotificationService.expects(:process).once + @queue.expects(:send_message).once @message.path_type = "email" expect{@message.deliver}.not_to raise_error end it "processes twitter message type" do - NotificationService.expects(:process).once + @queue.expects(:send_message).once @message.path_type = "twitter" expect{@message.deliver}.not_to raise_error end it "processes twilio message type" do - NotificationService.expects(:process).once + @queue.expects(:send_message).once @message.path_type = "sms" expect{@message.deliver}.not_to raise_error end it "processes sms message type" do - NotificationService.expects(:process).once + @queue.expects(:send_message).once @message.path_type = "sms" @message.to = "+18015550100" expect{@message.deliver}.not_to raise_error end it "processes push notification message type" do - NotificationService.expects(:process).once + @queue.expects(:send_message).once @message.path_type = "push" expect{@message.deliver}.not_to raise_error end + it "throws error if cannot connect to queue" do + @queue.stubs(:send_message).raises(AWS::SQS::Errors::ServiceError) + expect{@message.deliver}.to raise_error(AWS::SQS::Errors::ServiceError) + expect(@message.transmission_errors).to include("AWS::SQS::Errors::ServiceError") + expect(@message.workflow_state).to eql("staged") + end end end