Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure correct Faraday JSON parsing/response body for invalid response header #110

Merged
merged 1 commit into from
Jan 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
### (Next)
* Your contribution here.
* [#110](https://github.com/ashkan18/graphlient/pull/110): Ensure correct Faraday JSON response body parsing with invalid response header - [@taylorthurlow](https://github.com/taylorthurlow).
* [#107](https://github.com/ashkan18/graphlient/pull/107): Pass in initialized schema as an option - [@kbaum](https://github.com/kbaum).
* [#106](https://github.com/ashkan18/graphlient/pull/106): Add 3.2 to the list of ruby ci versions - [@igor-drozdov](https://github.com/igor-drozdov).
* [#102](https://github.com/ashkan18/graphlient/pull/102): Update ci to test latest jruby - [@ashkan18](https://github.com/ashkan18).
Expand Down
36 changes: 35 additions & 1 deletion lib/graphlient/adapters/http/faraday_adapter.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'faraday'
require 'json'

module Graphlient
module Adapters
Expand All @@ -13,7 +14,8 @@ def execute(document:, operation_name:, variables:, context:)
variables: variables
}.to_json
end
response.body

parse_body(response.body)
rescue Faraday::ConnectionFailed => e
raise Graphlient::Errors::ConnectionFailedError, e
rescue Faraday::TimeoutError => e
Expand All @@ -39,6 +41,38 @@ def connection
end
end
end

private

# Faraday 2.x's JSON response middleware will only parse a JSON
# response body into a Hash (or Array) object if the response headers
# match a specific content type regex. See Faraday's response JSON
# middleware definition for specifics on what the datatype of the
# response body will be. This method will handle the situation where
# the response header is not set appropriately, but contains JSON
# anyways. If the body cannot be parsed as JSON, an exception will be
# raised.
def parse_body(body)
case body
when Hash, Array
body
when String
begin
JSON.parse(body)
rescue JSON::ParserError
raise Graphlient::Errors::ServerError, 'Failed to parse response body as JSON'
end
else
inner_exception = StandardError.new <<~ERR.strip.tr("\n", ' ')
Unexpected response body type '#{body.class}'. Graphlient doesn't
know how to handle a response body of this type, but Faraday is
returning it. Please open an issue, particularly if the response
body does actually contain valid JSON.
ERR

raise Graphlient::Errors::ClientError, inner_exception
end
end
end
end
end
Expand Down
55 changes: 55 additions & 0 deletions spec/graphlient/adapters/http/faraday_adapter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,61 @@
end
end

context 'a non-error response without an appropriate JSON header' do
let(:url) { 'http://example.com/graphql' }
let(:headers) { { 'Content-Type' => 'application/notjson' } }
let(:client) { Graphlient::Client.new(url) }
let(:response_headers) { { 'Content-Type' => 'application/notjson' } }

context 'when the response body is valid JSON' do
before do
stub_request(:post, url).to_return(
status: 200,
headers: response_headers,
body: DummySchema.execute(GraphQL::Introspection::INTROSPECTION_QUERY).to_json
)
end

it 'retrieves schema' do
expect(client.schema).to be_a Graphlient::Schema
end
end

context 'when the response body is not valid JSON' do
before do
stub_request(:post, url).to_return(
status: 200,
headers: response_headers,
body: ''
)
end

it 'raises Graphlient::Errors::ServerError' do
expect { client.schema }.to raise_error(Graphlient::Errors::ServerError) { |error|
expect(error.message).to include('Failed to parse response body as JSON')
}
end
end

context 'when the Faraday response body object is not a type we expect from Faraday' do
before do
stub_request(:post, url).to_return(
status: 200,
headers: response_headers
)
end

it 'raises Graphlient::Errors::ClientError' do
mock_response = double('response', body: nil)
allow(client.http.connection).to receive(:post).and_return(mock_response)

expect { client.schema }.to raise_error(Graphlient::Errors::ClientError) { |error|
expect(error.message).to include "Unexpected response body type 'NilClass'"
}
end
end
end

context 'Failed to open TCP connection error' do
let(:url) { 'http://example.com/graphql' }
let(:client) { Graphlient::Client.new(url) }
Expand Down