diff --git a/app/adapters/signal_adapter/api.rb b/app/adapters/signal_adapter/api.rb new file mode 100644 index 000000000..3295fb535 --- /dev/null +++ b/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 diff --git a/app/adapters/signal_adapter/bad_request_error.rb b/app/adapters/signal_adapter/bad_request_error.rb new file mode 100644 index 000000000..6e338cdce --- /dev/null +++ b/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 diff --git a/app/adapters/signal_adapter/outbound/file.rb b/app/adapters/signal_adapter/outbound/file.rb index 30b399aa4..516db7f0c 100644 --- a/app/adapters/signal_adapter/outbound/file.rb +++ b/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 diff --git a/app/adapters/signal_adapter/outbound/text.rb b/app/adapters/signal_adapter/outbound/text.rb index de3715b90..95eb238a8 100644 --- a/app/adapters/signal_adapter/outbound/text.rb +++ b/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 diff --git a/docker-compose.yml b/docker-compose.yml index b628422e9..7242e90c7 100644 --- a/docker-compose.yml +++ b/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) diff --git a/spec/adapters/signal_adapter/api_spec.rb b/spec/adapters/signal_adapter/api_spec.rb new file mode 100644 index 000000000..2e69af3bb --- /dev/null +++ b/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 diff --git a/spec/adapters/signal_adapter/outbound/file_spec.rb b/spec/adapters/signal_adapter/outbound/file_spec.rb index ea8ea06fa..30a829064 100644 --- a/spec/adapters/signal_adapter/outbound/file_spec.rb +++ b/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 diff --git a/spec/adapters/signal_adapter/outbound/text_spec.rb b/spec/adapters/signal_adapter/outbound/text_spec.rb index 71bb0625b..1654d2566 100644 --- a/spec/adapters/signal_adapter/outbound/text_spec.rb +++ b/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