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

input coercion errors should be ValidationErrors #3304

Closed
schmod opened this issue Sep 17, 2019 · 3 comments
Closed

input coercion errors should be ValidationErrors #3304

schmod opened this issue Sep 17, 2019 · 3 comments
Labels
🍐 error-handling Pertaining to error handling (or lack thereof), not just for just general errors/bugs.

Comments

@schmod
Copy link
Contributor

schmod commented Sep 17, 2019

If I make a request that is illegal because the input arguments/variables are the wrong type, I would expect Apollo to emit a ValidationError. Instead, the more generic GraphQLError is thrown.

A very naive example:

This Query:

query UserQuery($userId: Int!) {
  user(userId: $userId) {
    firstName
  }
}

with the variables:

{ "userId": null }

produces an error object (passed to formatError) that looks something like:

{
  "message":  "Variable \"$userId\" of non-null type \"Int!\" must not be null.",
  "name":"GraphQLError"
}

Because these errors are typically the result of a bad client request, I would like to have some sort of mechanism where I can ensure that they are logged with a lower severity (compared to an actual execution error).

@schmod
Copy link
Contributor Author

schmod commented Sep 17, 2019

The coercion process is called by graphql/execution/values.js
which is in turn called by graphql/execution/execute.js

I wasn't able to locate a good point where Apollo could unambiguously intercept these errors 😕and convert them to ValidationErrors – I wonder if this is a change that would need to be made in graphql-js itself....

@n1ru4l
Copy link
Contributor

n1ru4l commented Apr 2, 2020

@schmod I mentioned a possible (but really ugly) fix here: #3498 (comment)

Is there currently any discussion ongoing on changing this behaviour you mentioned? Would this involve changing the GraphQL spec or is this something that must be handled inside the implementations (such as graphql-js)?

@abernix abernix added the 🍐 error-handling Pertaining to error handling (or lack thereof), not just for just general errors/bugs. label Dec 31, 2020
@glasser
Copy link
Member

glasser commented Jul 16, 2021

This is a duplicate of #5353, I think. Note that we fixed some of this in #5091.

@glasser glasser closed this as completed Jul 16, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🍐 error-handling Pertaining to error handling (or lack thereof), not just for just general errors/bugs.
Projects
None yet
Development

No branches or pull requests

4 participants