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

NoMethodError in FaradayServerError #51

Closed
chmorik opened this issue Oct 10, 2018 · 10 comments
Closed

NoMethodError in FaradayServerError #51

chmorik opened this issue Oct 10, 2018 · 10 comments

Comments

@chmorik
Copy link

chmorik commented Oct 10, 2018

Hi,
We've encounter a situation where we get
undefined method `[]' for nil:NilClass in
graphlient-0.3.3/lib/graphlient/errors/faraday_server_error.rb:7
This is very frustrating because we don't know what is the error behind it
We think that changing to
@response = inner_exception.response.try(:[],:body)
@status_code = inner_exception.response.try(:[],:status)

will fix it

@dblock
Copy link
Collaborator

dblock commented Oct 10, 2018

Try writing a spec that reproduces the problem?

@dblock dblock added the bug label Oct 10, 2018
@chmorik
Copy link
Author

chmorik commented Oct 11, 2018

e = Faraday::ClientError.new(nil)
raise Graphlient::Errors::FaradayServerError, e

@dblock
Copy link
Collaborator

dblock commented Oct 11, 2018

In which scenario does this actually happen? What's the stack?

The code should be fixed either way. Appreciate a pull request.

@chmorik
Copy link
Author

chmorik commented Oct 11, 2018

The thing is I don't know. If it'll be fixed I can tell you.
Stack trace for example:


NoMethodError: undefined method `[]' for nil:NilClass
1
File "/app/vendor/bundle/ruby/2.3.0/gems/graphlient-0.3.3/lib/graphlient/errors/faraday_server_error.rb" line 7 in initialize
2
File "/app/vendor/bundle/ruby/2.3.0/gems/graphlient-0.3.3/lib/graphlient/adapters/http/faraday_adapter.rb" line 19 in exception
3
File "/app/vendor/bundle/ruby/2.3.0/gems/graphlient-0.3.3/lib/graphlient/adapters/http/faraday_adapter.rb" line 19 in raise
4
File "/app/vendor/bundle/ruby/2.3.0/gems/graphlient-0.3.3/lib/graphlient/adapters/http/faraday_adapter.rb" line 19 in rescue in execute
5
File "/app/vendor/bundle/ruby/2.3.0/gems/graphlient-0.3.3/lib/graphlient/adapters/http/faraday_adapter.rb" line 8 in execute
6
File "/app/vendor/bundle/ruby/2.3.0/gems/graphql-client-0.13.0/lib/graphql/client.rb" line 328 in block in query
7
File "/app/vendor/bundle/ruby/2.3.0/gems/activesupport-4.1.14/lib/active_support/notifications.rb" line 161 in instrument
8
File "/app/vendor/bundle/ruby/2.3.0/gems/graphql-client-0.13.0/lib/graphql/client.rb" line 327 in query
9
File "/app/vendor/bundle/ruby/2.3.0/gems/graphlient-0.3.3/lib/graphlient/client.rb" line 25 in execute

I can't open one right now. but


@response = inner_exception.response.try(:[],:body)
@status_code = inner_exception.response.try(:[],:status)

should fix it

@ashkan18
Copy link
Owner

Looks like we run into a case where inner_exception, possibly a Faraday one, doesn't have body or status, I can look into it in a bit but would be interesting to find out how that happens as @dblock mentions. If that's a valid case, we should possibly replace body and status to a more generic error instead of just setting them to nil

@dblock
Copy link
Collaborator

dblock commented Oct 11, 2018

If someone wants to just PR a defensive implementation for Faraday::ClientError taking a nil I'd be happy to review it, but I think there's more going on here.

@yuki24
Copy link
Collaborator

yuki24 commented Oct 14, 2018

It's probably the case that the request timed out and the client threw an error without a response object since there's nothing you could assign to the exception when the request didn't go through.

@dfritsch86
Copy link

On that note, is there a way to pass options to Faraday, e.g. to increase the standard request time out duration? I find myself having to query an API that requires more than the default 30s to generate a response, so raising that would be nice.

@dblock
Copy link
Collaborator

dblock commented Apr 29, 2019

On that note, is there a way to pass options to Faraday, e.g. to increase the standard request time out duration? I find myself having to query an API that requires more than the default 30s to generate a response, so raising that would be nice.

I believe this will work (haven't tested)

Graphlient::Client.new('http://test-graphql.biz/graphql') do |client|
      client.http do |h|
        h.connection do |c|
          c.options.timeout = 30
        end
      end
    end

If it does, please add to docs?

We could also expose http options, I wouldn't mind supporting something like

Graphlient::Client.new('http://test-graphql.biz/graphql', http: { connection: { timeout: 30 } }) 

@dblock
Copy link
Collaborator

dblock commented Nov 14, 2019

Closed via #68.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants