Update signal-cli-rest-api with more verbose error messages and report them

app/adapters/signal_adapter/api.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module SignalAdapter
+  class Api
+    class << self
+      def perform_request(request)
+        uri = request.uri
+        response = Net::HTTP.start(uri.host, uri.port) do |http|
+          http.request(request)
+        end
+        case response
+        when Net::HTTPSuccess
+          yield response if block_given?
+        else
+          error_message = JSON.parse(response.body)['error']
+          exception = SignalAdapter::BadRequestError.new(error_code: response.code, message: error_message)
+          context = {
+            code: response.code,
+            message: response.message,
+            headers: response.to_hash,
+            body: error_message
+          }
+          ErrorNotifier.report(exception, context: context)
+        end
+      end
+    end
+  end
+end

app/adapters/signal_adapter/bad_request_error.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module SignalAdapter
+  class BadRequestError < StandardError
+    def initialize(error_code:, message:)
+      super("Message was not delivered with error code `#{error_code}` and message `#{message}`")
+    end
+  end
+end

app/adapters/signal_adapter/outbound/file.rb

@@ -9,23 +9,17 @@ class File < ApplicationJob
 
       def perform(message:)
         @message = message
-        url = URI.parse("#{Setting.signal_cli_rest_api_endpoint}/v2/send")
-        request = Net::HTTP::Post.new(url.to_s, {
+        uri = URI.parse("#{Setting.signal_cli_rest_api_endpoint}/v2/send")
+        request = Net::HTTP::Post.new(uri, {
                                         Accept: 'application/json',
                                         'Content-Type': 'application/json'
                                       })
         request.body = data.to_json
-        response = Net::HTTP.start(url.host, url.port) do |http|
-          http.request(request)
+        SignalAdapter::Api.perform_request(request) do
+          # TODO: Do something on success. For example, mark the message as delivered?
+          # Or should we use deliver receipts as the source of truth.
+          Rails.logger.debug 'Great!'
         end
-        response.value # may raise exception
-      rescue Net::HTTPClientException => e
-        ErrorNotifier.report(e, context: {
-                               code: e.response.code,
-                               message: e.response.message,
-                               headers: e.response.to_hash,
-                               body: e.response.body
-                             })
       end
 
       def data

app/adapters/signal_adapter/outbound/text.rb

@@ -10,23 +10,17 @@ class Text < ApplicationJob
       def perform(recipient:, text:)
         @recipient = recipient
         @text = text
-        url = URI.parse("#{Setting.signal_cli_rest_api_endpoint}/v2/send")
-        request = Net::HTTP::Post.new(url.to_s, {
+        uri = URI.parse("#{Setting.signal_cli_rest_api_endpoint}/v2/send")
+        request = Net::HTTP::Post.new(uri, {
                                         Accept: 'application/json',
                                         'Content-Type': 'application/json'
                                       })
         request.body = data.to_json
-        response = Net::HTTP.start(url.host, url.port) do |http|
-          http.request(request)
+        SignalAdapter::Api.perform_request(request) do
+          # TODO: Do something on success. For example, mark the message as delivered?
+          # Or should we use deliver receipts as the source of truth.
+          Rails.logger.debug 'Great!'
         end
-        response.value # may raise exception
-      rescue Net::HTTPClientException => e
-        ErrorNotifier.report(e, context: {
-                               code: e.response.code,
-                               message: e.response.message,
-                               headers: e.response.to_hash,
-                               body: e.response.body
-                             })
       end
 
       def data

docker-compose.yml

@@ -26,7 +26,7 @@ services:
       - postgres_data:/var/lib/postgresql/data
 
   signal:
-    image: bbernhard/signal-cli-rest-api:0.67
+    image: bbernhard/signal-cli-rest-api:0.119-dev
     environment:
       - MODE=native
         #- AUTO_RECEIVE_SCHEDULE=0 22 * * * #enable this parameter on demand (see description below)

spec/adapters/signal_adapter/api_spec.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'webmock/rspec'
+
+RSpec.describe SignalAdapter::Api do
+  let(:api) { described_class }
+
+  describe '#perform_request' do
+    let(:uri) { URI.parse('http://signal:8080/v2/send') }
+    let(:request) { Net::HTTP::Post.new(uri) }
+
+    before do
+      allow(ErrorNotifier).to receive(:report)
+    end
+
+    describe 'http response code' do
+      describe '200' do
+        before(:each) do
+          stub_request(:post, uri).to_return(status: 200)
+        end
+        specify { expect { |block| api.perform_request(request, &block) }.to yield_control }
+
+        describe 'ErrorNotifier' do
+          subject { ErrorNotifier }
+          before { api.perform_request(request) }
+          it { should_not have_received(:report) }
+        end
+      end
+
+      describe '400' do
+        before(:each) do
+          stub_request(:post, uri).to_return(status: 400, body: { error: 'Ouch!' }.to_json)
+        end
+
+        specify { expect { |block| api.perform_request(request, &block) }.not_to yield_control }
+
+        describe 'ErrorNotifier' do
+          subject { ErrorNotifier }
+          before { api.perform_request(request) }
+          it { should have_received(:report) }
+        end
+      end
+    end
+  end
+end

spec/adapters/signal_adapter/outbound/file_spec.rb

@@ -24,10 +24,11 @@
       end
 
       describe 'on error' do
-        before(:each) { stub_request(:post, 'http://signal:8080/v2/send').to_return(status: 400) }
+        let(:error_message) { 'User is not registered' }
+        before(:each) { stub_request(:post, 'http://signal:8080/v2/send').to_return(status: 400, body: { error: error_message }.to_json) }
 
         it 'reports the error' do
-          expect(Sentry).to receive(:capture_exception).with(Net::HTTPClientException)
+          expect(Sentry).to receive(:capture_exception).with(SignalAdapter::BadRequestError.new(error_code: 400, message: error_message))
 
           subject.call
         end

spec/adapters/signal_adapter/outbound/text_spec.rb

@@ -24,10 +24,11 @@
       end
 
       describe 'on error' do
-        before(:each) { stub_request(:post, 'http://signal:8080/v2/send').to_return(status: 400) }
+        let(:error_message) { 'User is not registered' }
+        before(:each) { stub_request(:post, 'http://signal:8080/v2/send').to_return(status: 400, body: { error: error_message }.to_json) }
 
         it 'reports the error' do
-          expect(Sentry).to receive(:capture_exception).with(Net::HTTPClientException)
+          expect(Sentry).to receive(:capture_exception).with(SignalAdapter::BadRequestError.new(error_code: 400, message: error_message))
 
           subject.call
         end