search

ashkan18 / graphlient

Ruby GraphQL Client
MIT License
251 stars 43 forks source link

Add Graphlient::Errors::ConnectionFailedError #68

Closed neroleung closed 4 years ago

neroleung commented 4 years ago

This is essentially https://github.com/ashkan18/graphlient/pull/59 with test.

Whenever there is an Errno::ECONNREFUSED error, Faraday wraps it in a Faraday::ConnectionFailed error. Since it inherits Faraday::ClientError without a response (see errors/faraday_server_error.rb), the code would return a NoMethodError: undefined method []' for nil:NilClass error.

So basically the fix is to rescue Faraday::ConnectionFailed before rescuing Faraday::ClientError so that we can raise a more meaningful error.

Result:

Graphlient::Errors::ConnectionFailedError: Failed to open TCP connection to localhost:3000 (Connection refused - connect(2) for "localhost" port 3000)
dblock commented 4 years ago

Oh one thing. You should add an UPGRADING document in this PR like this one because this does change the interface. If someone was handling the Faraday error they now get a ServerError.

neroleung commented 4 years ago

@dblock, I've updated CHANGELOG.md and README.md. Also added UPGRADING.md. Question though, do you want a new section in README.md like https://github.com/dblock/slack-ruby-client#stable-release?

dblock commented 4 years ago

This is perfect, thank you. Feel free to PR something linking to UPGRADING.

neroleung commented 4 years ago

Thanks. This should close https://github.com/ashkan18/graphlient/issues/51, https://github.com/ashkan18/graphlient/pull/52 and https://github.com/ashkan18/graphlient/pull/59 once a new version is released.

dblock commented 4 years ago

I'll cut a release unless @yuki24 or @ashkan18 can beat me to it (please).

ashkan18 commented 4 years ago

thanks @neroleung ! this is awesome! going to cut the release, hopefully before @dblock 😄

ashkan18 commented 4 years ago

alright 0.3.7 is now published.