Skip to content

Commit

Permalink
Add better error handling to .find
Browse files Browse the repository at this point in the history
* Offers better ActiveRecord-like errors when trying to call .find with
  either no params, or an empty params hash

* Raises an explicit ResourceNotFound error when a 404 is returned,
  regardless of whether or not the response was parseable.
  • Loading branch information
Derek Lindahl committed Apr 6, 2015
1 parent 87ca70b commit c1cc0f7
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 9 deletions.
8 changes: 7 additions & 1 deletion lib/frenetic/concerns/member_rest_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@ module MemberRestMethods

module ClassMethods
def find(params)
fail ResourceNotFound.new(self, params) if params.blank?
params = { id:params } unless params.is_a?(Hash)
return as_mock(params) if test_mode?
response = api.get(member_url(params))
begin
response = api.get(member_url(params))
rescue ClientParsingError, ClientError => ex
raise if ex.status != 404
raise ResourceNotFound.new(self, params)
end
new(response.body) if response.success?
end

Expand Down
42 changes: 36 additions & 6 deletions lib/frenetic/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,14 @@ class ResponseError < Error
attr_reader :env, :error, :method, :status, :url
def initialize(env)
env ||= {}
body = env.fetch(:body, {})
@env = env
@error = body['error']
@method = env[:method]
@status = env[:status]
@url = env[:url]
if env.respond_to?(:fetch)
body = env.fetch(:body, {})
@env = env
@error = body['error']
@method = env[:method]
@status = env[:status]
@url = env[:url]
end
super(message)
end

Expand All @@ -144,6 +146,34 @@ def message
# Raised when a network response returns a 400-level error
ClientError = Class.new(ResponseError)

# Raise when a network reponse returns a 404 Not Found error
class ResourceNotFound < ClientError
def initialize(resource, params)
@resource = resource.to_s.demodulize
@params = params
@status = 404
super(message)
end

def message
if @params.blank?
"Couldn't find #{@resource} without an ID"
else
"Couldn't find #{@resource} with #{stringified_params}"
end
end

private

def stringified_params
@params.each_with_object([]) do |*pairs, agg|
agg.concat(pairs)
end.map do |pair|
pair.join('=')
end.join(', ')
end
end

# Raised when a network response returns a 500-level error
ServerError = Class.new(ResponseError)

Expand Down
30 changes: 28 additions & 2 deletions spec/concerns/member_rest_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
end

describe '.find' do
let(:params) { 1 }

before { @stubs.api_description }

subject { MyTempResource.find 1 }
subject { MyTempResource.find(params) }

context 'for a known instance' do
before { @stubs.known_instance }
Expand All @@ -41,7 +43,23 @@
before { @stubs.unknown_instance }

it 'raises an error' do
expect{subject}.to raise_error Frenetic::ClientError
expect{subject}.to raise_error Frenetic::ResourceNotFound, %q(Couldn't find MyTempResource with id=1)
end

context 'with an unparseable response' do
before { @stubs.invalid_unknown_instance }

it 'raises an error' do
expect{subject}.to raise_error Frenetic::ResourceNotFound, %q(Couldn't find MyTempResource with id=1)
end
end
end

context 'that results in a client-level error' do
before { @stubs.api_client_error }

it 'raises an error' do
expect{subject}.to raise_error Frenetic::ClientError, %q(422 Unprocessable Entity)
end
end

Expand All @@ -56,6 +74,14 @@
it 'returns a mock resource' do
expect(subject).to be_an_instance_of MyMockResource
end

context 'with blank parameters' do
let(:params) { {} }

it 'raises an error' do
expect{subject}.to raise_error Frenetic::ResourceNotFound, %q(Couldn't find MyTempResource without an ID)
end
end
end
end

Expand Down
10 changes: 10 additions & 0 deletions spec/fixtures/test_api_requests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ def api_client_error(type = :json)
.to_return defaults.merge(body:body, status:404)
end

def api_client_error
@rspec.stub_request(:get, 'example.com/api/my_temp_resources/1')
.to_return response(body:{ 'error' => '422 Unprocessable Entity' }, status:422)
end

def api_description
@rspec.stub_request(:any, 'example.com/api')
.to_return response(body:schema, headers:{ 'Cache-Control' => 'max-age=3600, public' })
Expand All @@ -57,6 +62,11 @@ def unknown_instance
.to_return response(body:{ 'error' => '404 Not Found' }, status:404)
end

def invalid_unknown_instance
@rspec.stub_request(:get, 'example.com/api/my_temp_resources/1')
.to_return response(body:'Unparseable Not Found', status:404)
end

def known_instance
@rspec.stub_request(:get, 'example.com/api/my_temp_resources/1')
.to_return response(body:{ 'name' => 'Resource Name' })
Expand Down

0 comments on commit c1cc0f7

Please sign in to comment.