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

Include response body in non-200 OK error text. #30

Merged
merged 1 commit into from
Sep 25, 2018

Conversation

dmitshur
Copy link
Member

When a GraphQL server returns a non-200 OK status code, it may be very helpful to for caller to know what the response body was. So, when such an error occurs, fetch the response body and include it in the returned error text.

Fixes #29.
Updates #24.

/cc @cjwagner @jorgesece @robermorales

When a GraphQL server returns a non-200 OK status code, it may be
very helpful to for caller to know what the response body was.
So, when such an error occurs, fetch the response body and
include it in the returned error text.

Fixes #29.
Updates #24.
Copy link

@cjwagner cjwagner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks!

@dmitshur dmitshur merged commit e4a3a37 into master Sep 25, 2018
@dmitshur dmitshur deleted the resp-body-in-error branch September 25, 2018 04:35
dmitshur added a commit to shurcooL/githubv4 that referenced this pull request Sep 25, 2018
The error text has changed in commit shurcooL/graphql@e4a3a37
to include response body. Change the test for new behavior.

Follows shurcooL/graphql#30.
@robermorales
Copy link

Great advance! It is not covering all the cases that we covered in #24, but I can understand the limitations of guessing if a body with a 4xx status code can be try to be decoded as a graphql valid response. At least the client can see the body response if something went wrong, so the troubleshooting can be improved a lot.

cjwagner added a commit to cjwagner/test-infra that referenced this pull request Sep 25, 2018
k8s-ci-robot added a commit to kubernetes/test-infra that referenced this pull request Sep 26, 2018
Update vendored graphql library to include shurcooL/graphql#30.
grihabor pushed a commit to grihabor/graphql that referenced this pull request Jul 20, 2022
… for unit tests (shurcooL#30)

add debug mode, expose GraphQL builder functions and UnmarshalGraphQL for unit tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants