Skip to content
This repository was archived by the owner on Mar 20, 2023. It is now read-only.

Alternative fix for #427 #471

Closed
wants to merge 1 commit into from

Conversation

rogierschouten
Copy link

Issue #427 already has a PR #451 but this solution is a bit more flexible.

This fixes notably the issue that when the client forgets a mandatory field in a GraphQL mutation input,
the underlying graphql-js package does not throw; rather it returns a normal response with errors. This should result in status code 400 (bad request) because it is a client problem, but express-graphql makes it a 500.

This PR allows to specify separately what to do with normal responses that

  • contain no data and no errors (typically 500)
  • contain no data and errors (typically 400)
  • contain both data and errors (typically 200)

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@jsonmaur
Copy link

@rogierschouten I was about to open a similar PR for this. It really needs to be fixed... issues with the data coming from clients should never return a 500 status code.

@IvanGoncharov Can we get this merged?

@jsonmaur
Copy link

jsonmaur commented Oct 25, 2019

After looking at other PR's, this idea of using Express middleware for error handling might be a better approach.

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

Successfully merging this pull request may close these issues.

3 participants