diff --git a/app/services/service_response.rb b/app/services/service_response.rb index c7ab75a44266c0adebcdce5f38d3ada39df49672..848f90e7f25116f3bec8ed537a4886de8bd23b99 100644 --- a/app/services/service_response.rb +++ b/app/services/service_response.rb @@ -2,20 +2,28 @@ class ServiceResponse def self.success(message: nil, payload: {}, http_status: :ok) - new(status: :success, message: message, payload: payload, http_status: http_status) + new(status: :success, + message: message, + payload: payload, + http_status: http_status) end - def self.error(message:, payload: {}, http_status: nil) - new(status: :error, message: message, payload: payload, http_status: http_status) + def self.error(message:, payload: {}, http_status: nil, reason: nil) + new(status: :error, + message: message, + payload: payload, + http_status: http_status, + reason: reason) end - attr_reader :status, :message, :http_status, :payload + attr_reader :status, :message, :http_status, :payload, :reason - def initialize(status:, message: nil, payload: {}, http_status: nil) + def initialize(status:, message: nil, payload: {}, http_status: nil, reason: nil) self.status = status self.message = message self.payload = payload self.http_status = http_status + self.reason = reason end def track_exception(as: StandardError, **extra_data) @@ -41,7 +49,11 @@ def [](key) end def to_h - (payload || {}).merge(status: status, message: message, http_status: http_status) + (payload || {}).merge( + status: status, + message: message, + http_status: http_status, + reason: reason) end def success? @@ -60,5 +72,5 @@ def errors private - attr_writer :status, :message, :http_status, :payload + attr_writer :status, :message, :http_status, :payload, :reason end diff --git a/doc/development/reusing_abstractions.md b/doc/development/reusing_abstractions.md index ef4e8b0310fba85e16212efbd392f1b305eed930..826782d703677012d629bd925cb2c1099909e6ca 100644 --- a/doc/development/reusing_abstractions.md +++ b/doc/development/reusing_abstractions.md @@ -206,6 +206,31 @@ response = ServiceResponse.success(payload: { issue: issue }) response.payload[:issue] # => issue ``` +Error responses can also specify the failure `reason` which can be used by the caller +to understand the nature of the failure. +The caller, if an HTTP endpoint, could translate the reason symbol into an HTTP status code: + +```ruby +response = ServiceResponse.error( + message: 'Job is in a state that cannot be retried', + reason: :job_not_retrieable) + +if response.success? + head :ok +if response.reason == :job_not_retriable + head :unprocessable_entity +else + head :bad_request +end +``` + +For common failures such as resource `:not_found` or operation `:forbidden`, we could +leverage the Rails [HTTP status symbols](http://www.railsstatuscodes.com/) as long as +they are sufficiently specific for the domain logic involved. +For other failures use domain-specific reasons whenever possible. + +For example: `:job_not_retriable`, `:duplicate_package`, `:merge_request_not_mergeable`. + ### Finders Everything in `app/finders`, typically used for retrieving data from a database. diff --git a/spec/services/service_response_spec.rb b/spec/services/service_response_spec.rb index 3ede90fbc44cb052fd9d7cd5bfeedb916fe6e029..2d70979dd3a5ce7fa763b8f1a40e679f62d5b67b 100644 --- a/spec/services/service_response_spec.rb +++ b/spec/services/service_response_spec.rb @@ -43,14 +43,14 @@ end describe '.error' do - it 'creates a failed response without HTTP status' do + it 'creates an error response without HTTP status' do response = described_class.error(message: 'Bad apple') expect(response).to be_error expect(response.message).to eq('Bad apple') end - it 'creates a failed response with HTTP status' do + it 'creates an error response with HTTP status' do response = described_class.error(message: 'Bad apple', http_status: 400) expect(response).to be_error @@ -58,7 +58,7 @@ expect(response.http_status).to eq(400) end - it 'creates a failed response with payload' do + it 'creates an error response with payload' do response = described_class.error(message: 'Bad apple', payload: { bad: 'apple' }) @@ -66,6 +66,15 @@ expect(response.message).to eq('Bad apple') expect(response.payload).to eq(bad: 'apple') end + + it 'creates an error response with a reason' do + response = described_class.error(message: 'Bad apple', + reason: :permission_denied) + + expect(response).to be_error + expect(response.message).to eq('Bad apple') + expect(response.reason).to eq(:permission_denied) + end end describe '#success?' do