Add new http_component that would return the body of the response and custom error messages. #685
Conversation
pri1012
changed the title
lib/koala/api/graph_batch_api.rb
Outdated
| # from graph_call so this needs to be parsed | ||
| # as generate_results method handles only JSON response | ||
| if http_options[:http_component] | ||
| response = JSON.load(response.body) |
There was a problem hiding this comment.
Could you handle JSONError and add a spec for it. If fb sends a messed up body we will end up with a json error instead of a koala error.
And from experience, fb loves to send html error pages or null instead of well formed json
There was a problem hiding this comment.
We have added the code change to handle the JSON error and added test coverage as well.
spec/cases/graph_api_batch_spec.rb
Outdated
| @@ -406,6 +416,13 @@ | |||
| Koala::Facebook::API.new("foo").batch {|batch_api| batch_api.get_object('me') } | |||
| }.to raise_exception(Koala::Facebook::BadFacebookResponse) | |||
There was a problem hiding this comment.
| }.to raise_exception(Koala::Facebook::BadFacebookResponse) | |
| }.to raise_exception(Koala::Facebook::BadFacebookResponse, /"Facebook returned an empty body"/) |
There was a problem hiding this comment.
We have updated the test to check for the error message as well.
lib/koala/api/graph_batch_api.rb
Outdated
| # when http_component is set we receive Koala::Http_service response object | ||
| # from graph_call so this needs to be parsed | ||
| # as generate_results method handles only JSON response | ||
| if http_options[:http_component] |
There was a problem hiding this comment.
here we need to add a check for :response because if you set http_component = header or status it will fail see https://github.com/pri1012/koala/blob/c3279139fab69d25d017cb9b83d1de9e18fd5018/lib/koala/api.rb#L57
spec/cases/graph_api_batch_spec.rb
Outdated
| it "raises a BadFacebookResponse if the body is non-empty, non-array" do | ||
| allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, "200", {})) | ||
| expect { | ||
| Koala::Facebook::API.new("foo").batch(http_component: :response) {|batch_api| batch_api.get_object('me') } |
There was a problem hiding this comment.
Could you add a test where http_component is status, and another one with headers
There was a problem hiding this comment.
Sure. We have added tests for http_component :status and :headers as well.
lib/koala/api/graph_batch_api.rb
Outdated
| @@ -138,6 +146,11 @@ def desired_component(component:, response:, headers:) | |||
| # facebook returns the headers as an array of k/v pairs, but we want a regular hash | |||
| when :headers then headers | |||
| # (see note in regular api method about JSON parsing) | |||
| when :response | |||
| Koala::HTTPService::Response.new( | |||
There was a problem hiding this comment.
let's return result which is defined above. And also you are mixing code and status in this initialization
There was a problem hiding this comment.
Good point @ylecuyer. We have use result defined above.
saikambaiyyagari
force-pushed
the
vvaradarjan/new_http_component
branch
from
bf514d6
to
d9cd078
Compare
saikambaiyyagari
force-pushed
the
vvaradarjan/new_http_component
branch
from
d9cd078
to
f93bddf
Compare
saikambaiyyagari
force-pushed
the
vvaradarjan/new_http_component
branch
from
f93bddf
to
96e7cf9
Compare
Description:
This PR includes changes which will help use the latest koala version when http_component is set
Changes included:
Parsing of response: when http_component is set, we receive Koala::Http_service response instead of a hash and generate_results method handles only JSON response. This affects all batch request.
Custom Error message when the body is nil