Merge pull request #41 from alphagov/error-handling

Add rudimentary error handling to `MessageProcessor`

lib/document_sync_worker/message_processor.rb

@@ -13,6 +13,21 @@ def process(message)
       document.synchronize_to(repository)
 
       message.ack
+    rescue StandardError
+      # TODO: Consider options for handling errors more granularly, and for differentiating between
+      # retriable (e.g. transient connection issue in repository) and fatal (e.g. malformed document
+      # on queue) errors. For now while we aren't live, send the message to Sentry and reject it to
+      # avoid unnecessary retries that would probably fail again while we're very actively
+      # iterating.
+      extra_info = if message.payload.is_a?(Hash)
+                     # Omit details as it may be large and take us over the Sentry metadata limit
+                     message.payload.except("details")
+                   else
+                     { message_payload: message.payload.to_s }
+                   end
+      GovukError.notify("Failed to process incoming document message", extra: extra_info)
+
+      message.discard
     end
   end
 end

spec/lib/document_sync_worker/message_processor_spec.rb

@@ -33,12 +33,23 @@
     context "when creating the document fails" do
       before do
         allow(DocumentSyncWorker::Document).to receive(:for).and_raise("Something went wrong")
+        allow(GovukError).to receive(:notify)
       end
 
-      it "bubbles the error up and does not ack the message" do
-        expect { processor.process(message) }.to raise_error("Something went wrong")
+      it "reports the error to Sentry" do
+        processor.process(message)
+
+        expect(GovukError).to have_received(:notify).with(
+          "Failed to process incoming document message",
+          extra: payload,
+        )
+      end
+
+      it "rejects the message" do
+        processor.process(message)
 
         expect(message).not_to be_acked
+        expect(message).to be_discarded
       end
     end
   end