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

Throw exception for graphql errors #941

Merged
merged 1 commit into from
Nov 30, 2020

Conversation

acrobat
Copy link
Collaborator

@acrobat acrobat commented Nov 29, 2020

I noticed that errors from the graphql api didn't throw errors by our ExceptionThrower plugin. That's because the graphql api return HTTP code 200 even when the api had an error.

The GraphQL spec 6 doesn’t make any mention of HTTP status codes in the case of an error. Additionally, I would suggest that the error you show in your post is an application level error, not an HTTP protocol level error. It’s the same thing as when someone submits a value to an HTML form field that is incorrectly formatted … such as an invalid email address. Your web application returns a 200 HTTP status code because everything is fine at the HTTP protocol level, but displays an error message to the user asking them to supply a valid email address in the form.

See https://git.luolix.topmunity/t/github-api-v4-error-returns-200-status-code/14178/2

So I've adjusted the exception thrower to check HTTP 200 responses for grapql errors, with the early return's in the check function the overhead should be minimal.

@acrobat acrobat added the bug label Nov 29, 2020
@acrobat acrobat merged commit 50a1c51 into KnpLabs:2.x Nov 30, 2020
@acrobat acrobat deleted the graphql-exception-thrower branch November 30, 2020 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants