Skip to content

Commit

Permalink
Always raise a FrederickAPI::V2::Errors::Error if result set or resou…
Browse files Browse the repository at this point in the history
…rce contains errors when making a request
  • Loading branch information
Aaron Severs committed Dec 13, 2017
1 parent f2fc832 commit d43c772
Show file tree
Hide file tree
Showing 9 changed files with 317 additions and 6 deletions.
1 change: 1 addition & 0 deletions lib/frederick_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

# FrederickAPI libs
require 'frederick_api/configuration'
require 'frederick_api/v2/errors/errors'
require 'frederick_api/v2/helpers/has_many'
require 'frederick_api/v2/helpers/paginator'
require 'frederick_api/v2/helpers/query_builder'
Expand Down
32 changes: 32 additions & 0 deletions lib/frederick_api/v2/errors/errors.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

module FrederickAPI
module V2
module Errors
# Base exception class for client errors (i.e. validation, bad request)
class Error < JsonApiClient::Errors::ClientError
attr_reader :errors

# Initialize with a JsonApiClient::ResultSet or a Resource
def initialize(result)
# @env is used in base class JsonApiClient::Errors::Error
@env = result
@errors = result.errors || []
end

def to_s
return "Client Error: #{self.errors.first['detail']}" if self.errors.any?
super
end
end

class BadRequest < Error; end
class UnprocessableEntity < Error; end

ERROR_CODES = {
'400' => BadRequest,
'422' => UnprocessableEntity
}.freeze
end
end
end
13 changes: 11 additions & 2 deletions lib/frederick_api/v2/helpers/requestor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,20 @@ def resource_path(parameters)

# Retry once on unhandled server errors
def request(type, path, params)
super
handle_errors(super)
rescue JsonApiClient::Errors::ConnectionError, JsonApiClient::Errors::ServerError => ex
raise ex if ex.is_a?(JsonApiClient::Errors::NotFound) || ex.is_a?(JsonApiClient::Errors::Conflict)
super
handle_errors(super)
end

private

def handle_errors(result)
return result unless result.has_errors?
error_klass = FrederickAPI::V2::Errors::ERROR_CODES[result.errors.first[:status]] ||
FrederickAPI::V2::Errors::Error
raise error_klass, result
end
end
end
end
Expand Down
4 changes: 4 additions & 0 deletions lib/frederick_api/v2/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ def initialize(params = {})
super
end

def has_errors?
self.errors.present?
end

def self.all_records
self.all.pages.all_records
end
Expand Down
2 changes: 1 addition & 1 deletion lib/frederick_api/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

module FrederickAPI
# Current gem version
VERSION = '0.3.2'
VERSION = '0.4.0'
end
54 changes: 54 additions & 0 deletions spec/frederick_api/v2/errors/errors_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# frozen_string_literal: true

require 'spec_helper'

describe FrederickAPI::V2::Errors do
it 'ERROR_CODES' do
expect(described_class::ERROR_CODES).to eq(
'400' => FrederickAPI::V2::Errors::BadRequest,
'422' => FrederickAPI::V2::Errors::UnprocessableEntity
)
end
end

module FrederickAPI::V2::Errors
describe Error do
let(:result) { FrederickAPI::V2::Resource.new }
let(:instance) { described_class.new(result) }
let(:errors) { 'the errors' }

before do
allow(result).to receive(:errors).and_return errors
end

describe '#initialize' do
context 'result has errors' do
it 'sets env and errors' do
expect(instance.errors).to be errors
expect(instance.env).to be result
end
end

context 'result does not have errors' do
let(:errors) { nil }

it 'sets env and errors' do
expect(instance.errors).to eq []
expect(instance.env).to be result
end
end
end
end

describe BadRequest do
subject { described_class.new(FrederickAPI::V2::Resource.new) }

it { is_expected.to be_a(Error) }
end

describe UnprocessableEntity do
subject { described_class.new(FrederickAPI::V2::Resource.new) }

it { is_expected.to be_a(Error) }
end
end
103 changes: 102 additions & 1 deletion spec/frederick_api/v2/helpers/requestor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,106 @@
end
end

describe '#handle_errors' do
let(:has_errors) { false }

context 'with a ResultSet' do
let(:result) { instance_double(JsonApiClient::ResultSet) }

before { allow(result).to receive(:has_errors?).and_return has_errors }

context 'no errors' do
it 'returns result' do
expect(requestor.send(:handle_errors, result)).to be(result)
end
end

context 'has errors' do
let(:has_errors) { true }
let(:errors) { instance_double(JsonApiClient::ErrorCollector) }
let(:error) { instance_double(JsonApiClient::ErrorCollector::Error) }

before do
allow(result).to receive(:errors).and_return errors
allow(errors).to receive(:first).and_return error
allow(error).to receive(:[]).with(:status).and_return status
end

context 'error has a known status' do
let(:status) { '400' }

it 'raises correct error' do
expect { requestor.send(:handle_errors, result) }.to raise_error FrederickAPI::V2::Errors::BadRequest
end
end

context 'error has a unanticipated status' do
let(:status) { '483' }

it 'raises FrederickAPI::V2::Errors::Error' do
expect { requestor.send(:handle_errors, result) }.to raise_error FrederickAPI::V2::Errors::Error
end
end

context 'error has no status' do
let(:status) { nil }

it 'raises FrederickAPI::V2::Errors::Error' do
expect { requestor.send(:handle_errors, result) }.to raise_error FrederickAPI::V2::Errors::Error
end
end
end
end

context 'with a Resource' do
let(:result) { instance_double(FrederickAPI::V2::Resource) }

before { expect(result).to receive(:has_errors?).and_return has_errors }

context 'no errors' do
it 'returns result' do
expect(requestor.send(:handle_errors, result)).to be(result)
end
end

context 'has errors' do
let(:has_errors) { true }
let(:errors) { instance_double(JsonApiClient::ErrorCollector) }
let(:error) { instance_double(JsonApiClient::ErrorCollector::Error) }

before do
allow(result).to receive(:errors).and_return errors
allow(errors).to receive(:first).and_return error
allow(error).to receive(:[]).with(:status).and_return status
end

context 'error has a known status' do
let(:status) { '400' }

it 'raises correct error' do
expect { requestor.send(:handle_errors, result) }.to raise_error FrederickAPI::V2::Errors::BadRequest
end
end

context 'error has a unanticipated status' do
let(:status) { '483' }

it 'raises FrederickAPI::V2::Errors::Error' do
expect { requestor.send(:handle_errors, result) }.to raise_error FrederickAPI::V2::Errors::Error
end
end

context 'error has no status' do
let(:status) { nil }

it 'raises FrederickAPI::V2::Errors::Error' do
expect { requestor.send(:handle_errors, result) }.to raise_error FrederickAPI::V2::Errors::Error
end
end
end
end
end

describe '#request' do
let(:error) {}
let(:type) { 'type' }
Expand All @@ -51,9 +151,10 @@
let(:request_args) { [type, path, param] }
let(:super_instance) { superklass.new(String) }
let(:super_request_call_args) { [] }
let(:request_return) { 'request_return' }
let(:request_return) { instance_double(JsonApiClient::ResultSet) }

before do
allow(request_return).to receive(:has_errors?).and_return false
allow_any_instance_of(superklass).to receive(:request) do |*args|
super_request_call_args << args[1..-1]
raise(error) if super_request_call_args.length == 1 && error
Expand Down
16 changes: 16 additions & 0 deletions spec/frederick_api/v2/resource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,22 @@
end
end

describe '#has_errors?' do
context 'no errors' do
it 'false' do
expect(instance.has_errors?).to be false
end
end

context 'with errors' do
before { allow(instance).to receive(:errors).and_return 'some errors' }

it 'true' do
expect(instance.has_errors?).to be true
end
end
end

describe '.all_records' do
let(:result_set) { JsonApiClient::ResultSet.new }
let(:paginator) { FrederickAPI::V2::Helpers::Paginator.new(result_set, {}) }
Expand Down
98 changes: 96 additions & 2 deletions spec/integration/v2/resource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,45 @@

describe 'GET index' do
context 'with no query params' do
let(:resp) do
{
status: 200,
headers: {
content_type: 'application/vnd.api+json'
},
body: {
data: [
{
'id': id,
'type': 'users',
'links': {},
'attributes': {
'email': '[email protected]'
},
'relationships': {}
}
]
}.to_json
}
end

before do
stub_request(:get, base_url)
.with(headers: request_headers)
resource.with_access_token(access_token) { resource.all }
.with(headers: request_headers).to_return(resp)
end

it 'makes GET request' do
resource.with_access_token(access_token) { resource.all }
expect(
a_request(:get, base_url)
).to have_been_made.once
end

it 'returns data' do
expect(
resource.with_access_token(access_token) { resource.all }.first
).to be_a FrederickAPI::V2::User
end
end

context 'with query params' do
Expand Down Expand Up @@ -175,6 +203,72 @@
end
end
end

context 'bad request error' do
let(:resp) do
{
status: 400,
headers: {
content_type: 'application/vnd.api+json'
},
body: {
'errors' => [
{
'title' => 'property `not_a_real_contact_property` does not exist',
'detail' => 'properties - property `not_a_real_contact_property` does not exist',
'code' => '100',
'source' => { 'pointer' => '/data/attributes/properties' },
'status' => '400'
}
]
}.to_json
}
end

before do
stub_request(:get, base_url)
.with(headers: request_headers).to_return(resp)
end

it 'raises' do
expect do
resource.with_access_token(access_token) { resource.all }.first
end.to raise_error FrederickAPI::V2::Errors::BadRequest
end
end

context 'unprocessable entity error' do
let(:resp) do
{
status: 422,
headers: {
content_type: 'application/vnd.api+json'
},
body: {
'errors' => [
{
'title' => 'property `not_a_real_contact_property` does not exist',
'detail' => 'properties - property `not_a_real_contact_property` does not exist',
'code' => '100',
'source' => { 'pointer' => '/data/attributes/properties' },
'status' => '422'
}
]
}.to_json
}
end

before do
stub_request(:get, base_url)
.with(headers: request_headers).to_return(resp)
end

it 'raises' do
expect do
resource.with_access_token(access_token) { resource.all }.first
end.to raise_error FrederickAPI::V2::Errors::UnprocessableEntity
end
end
end

describe 'PATCH' do
Expand Down

0 comments on commit d43c772

Please sign in to comment.