Wrong error code thrown for GraphQL validation failures in arguments using query variables #3498

jcfi · 2019-11-11T18:19:35Z

Package Name/Version

Name: apollo-server

Version: 2.x.x. All versions from 2.9.7 through 2.0.0 have this bug.

Expected Behavior

Queries that fail due to errors in scalar parsing should return the error code GRAPHQL_VALIDATION_FAILED.

Actual Behavior

If an argument passed as a non-null query variable fails to validate, the server response has the code INTERNAL_SERVER_ERROR. This issue occurs with both built-in scalars and custom scalar types.

Reproduction

Sandbox Link

https://codesandbox.io/s/apollo-error-code-bug-ho6vr

Instructions

In the query field of the sandbox, enter

query($var:String!) {
  hello(name:$var)
}

And in the query variables section, enter

{
  "var": 2
}

The query above returns this response:

{
  "error": {
    "errors": [
      {
        "message": "Variable \"$var\" got invalid value 2; Expected type String. String cannot represent a non string value: 2",
        "locations": [
          {
            "line": 1,
            "column": 8
          }
        ],
        "extensions": {
          "code": "INTERNAL_SERVER_ERROR",
          "exception": {
            "stacktrace": [
              "TypeError: String cannot represent a non string value: 2",
              "    at GraphQLScalarType.coerceString [as parseValue] (/sandbox/node_modules/graphql/type/scalars.js:164:11)",
              "    at coerceInputValueImpl (/sandbox/node_modules/graphql/utilities/coerceInputValue.js:127:26)",
              "    at coerceInputValue (/sandbox/node_modules/graphql/utilities/coerceInputValue.js:37:10)",
              "    at _loop (/sandbox/node_modules/graphql/execution/values.js:107:69)",
              "    at coerceVariableValues (/sandbox/node_modules/graphql/execution/values.js:119:16)",
              "    at getVariableValues (/sandbox/node_modules/graphql/execution/values.js:48:19)",
              "    at buildExecutionContext (/sandbox/node_modules/graphql/execution/execute.js:184:61)",
              "    at executeImpl (/sandbox/node_modules/graphql/execution/execute.js:89:20)",
              "    at Object.execute (/sandbox/node_modules/graphql/execution/execute.js:64:35)",
              "    at /sandbox/node_modules/apollo-server-core/dist/requestPipeline.js:240:46"
            ]
          }
        }
      }
    ]
  }
}

Type errors in arguments sent without using query variables, on the other hand, give a more accurate error code. Compare the above to the result for this query:

query {
  hello(name:2)
}

It gives the response

{
  "error": {
    "errors": [
      {
        "message": "Expected type String, found 2.",
        "locations": [
          {
            "line": 2,
            "column": 15
          }
        ],
        "extensions": {
          "code": "GRAPHQL_VALIDATION_FAILED",
          "exception": {
            "stacktrace": [
              "GraphQLError: Expected type String, found 2.",
              "    at isValidScalar (/sandbox/node_modules/graphql/validation/rules/ValuesOfCorrectType.js:159:27)",
              "    at Object.IntValue (/sandbox/node_modules/graphql/validation/rules/ValuesOfCorrectType.js:116:14)",
              "    at Object.enter (/sandbox/node_modules/graphql/language/visitor.js:324:29)",
              "    at Object.enter (/sandbox/node_modules/graphql/language/visitor.js:375:25)",
              "    at visit (/sandbox/node_modules/graphql/language/visitor.js:242:26)",
              "    at Object.validate (/sandbox/node_modules/graphql/validation/validate.js:73:24)",
              "    at validate (/sandbox/node_modules/apollo-server-core/dist/requestPipeline.js:212:32)",
              "    at Object.<anonymous> (/sandbox/node_modules/apollo-server-core/dist/requestPipeline.js:125:42)",
              "    at Generator.next (<anonymous>)",
              "    at fulfilled (/sandbox/node_modules/apollo-server-core/dist/requestPipeline.js:5:58)"
            ]
          }
        }
      }
    ]
  }
}

which contains a much more useful error code for the client.

A similar problem was noted in several previous issues (#3361 #1410 #2204), but those issues were all linked to mutations, instead of queries.

The text was updated successfully, but these errors were encountered:

n1ru4l · 2019-11-18T11:03:46Z

The hacky workaround would be to fix it inside formatError:

 if (
  error.extensions &&
   (error.message.startsWith(`Variable "`) ||
     error.extensions.code === "GRAPHQL_VALIDATION_FAILED")
 ) {
  error.extensions.code = "GRAPHQL_VALIDATION_FAILED";
  return error;
}

Tydax · 2020-01-22T16:21:13Z

Unfortunately, Apollo failing to validate the query variables results in a TypeError being thrown, which are also thrown if attempting to read a property from undefined .

I wish we could rely on something else than the message to check for the error type, as I personally believe that the former should be passed down to the client while the latter should be wrapped as an internal system error.

fiznool · 2020-04-02T10:44:25Z

Also related to #3304.

jeromemeichelbeck · 2020-05-19T11:56:19Z

I came across the same issue.
I already use a custom formatError() so I will fix it as n1ru4l suggested, but it's not ideal...

mshwery · 2020-06-20T02:32:41Z

Ah, we are running into this same issue after upgrading both graphql (which now has stricter coercion / validation) and upgrading apollo-server.

The big issue here is that there is no reliable way to differentiate graphql validation errors from actual internal errors that are unexpected. GraphQL validation errors are essentially user-error... unactionable, whereas other types of internal errors are unexpected and potentially actionable.

Agree that TypeErrors thrown during document validation should be categorized as such.

EllaSharakanski · 2021-04-04T17:52:36Z

Are there any news on this issue?

glasser · 2021-04-06T23:00:07Z

You're right: INTERNAL_SERVER_ERROR shouldn't be something that should show up due to normal user input.

I don't think GRAPHQL_VALIDATION_FAILED is correct here. "Validation" is a specific stage in GraphQL processing that compares an operation document to a schema. It's a stage that's completely cacheable for a given document/schema pair and does not depend on other inputs like variables.

I think we should catch errors thrown by execution and if they are this particular type, give them an appropriate non-internal error code.

This particular error can be trivially triggered by clients, so it doesn't make sense to use `INTERNAL_SERVER_ERROR` for it. It seems like a good fit for `BAD_USER_INPUT`, which previously was only used if you explicitly throw a `UserInputError` in your app. Fixes #3498.

glasser · 2021-04-07T18:49:03Z

Fixing this in #5091. I've chosen BAD_USER_INPUT as the error code. This error code already existed in Apollo Server but was never thrown by AS itself (rather, as a user you can choose to throw UserInputError in your resolvers).

…5091) This particular error can be trivially triggered by clients, so it doesn't make sense to use `INTERNAL_SERVER_ERROR` for it. It seems like a good fit for `BAD_USER_INPUT`, which previously was only used if you explicitly throw a `UserInputError` in your app. Fixes #3498.

glasser · 2021-04-09T00:06:07Z

I've released a prerelease with this fix, version 2.23.0-alpha.0. Try out the alpha and see if it works for you! Please provide any feedback in #5094.

glasser · 2021-04-14T23:25:42Z

This is released in Apollo Server 2.23.0.

iBlueDust · 2021-06-18T10:52:06Z

Same thing happens when you feed null to a required variable.

Example

Version: 2.25.1
https://codesandbox.io/s/apollo-error-code-bug-ho6vr

query ($var: String!) {
  hello(name: $var)
}

Variables:

{} /* or */ { "var": null }

Response:

{
  "error": {
    "errors": [
      {
        "message": "Variable \"$var\" of non-null type \"String!\" must not be null.",
        "locations": [
          {
            "line": 1,
            "column": 8
          }
        ],
        "extensions": {
          "code": "INTERNAL_SERVER_ERROR",
          "exception": {
            "stacktrace": [ /* ... */ ]
          }
        }
      }
    ]
  }
}

Expected BAD_USER_INPUT instead of INTERNAL_SERVER_ERROR

This is derived from OP's example since my own example has unnecessary implementation details that would only bloat this post.

glasser · 2021-06-22T21:59:16Z

@iBlueDust Thanks for the report and reproduction. I filed #5353 which would be a great first issue for anyone interested in contributing.

n1ru4l mentioned this issue Apr 2, 2020

input coercion errors should be ValidationErrors #3304

Closed

gaastonsr mentioned this issue Aug 11, 2020

GraphQL error doesn't have stack trace #4474

Closed

benoitguigal mentioned this issue Nov 16, 2020

Fix GraphQL validation error displayed as internal server error MTES-MCT/trackdechets#711

Merged

3 tasks

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

glasser mentioned this issue Apr 7, 2021

apollo-server-core: use UserInputError for variable coercion errors #5091

Merged

glasser closed this as completed in #5091 Apr 8, 2021

glasser mentioned this issue Jun 22, 2021

Some variable-related errors should be BAD_USER_INPUT rather than INTERNAL_SERVER_ERROR #5353

Closed

github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrong error code thrown for GraphQL validation failures in arguments using query variables #3498

Wrong error code thrown for GraphQL validation failures in arguments using query variables #3498

jcfi commented Nov 11, 2019 •

edited

Loading

n1ru4l commented Nov 18, 2019

Tydax commented Jan 22, 2020 •

edited

Loading

fiznool commented Apr 2, 2020

jeromemeichelbeck commented May 19, 2020

mshwery commented Jun 20, 2020

EllaSharakanski commented Apr 4, 2021

glasser commented Apr 6, 2021

glasser commented Apr 7, 2021

glasser commented Apr 9, 2021

glasser commented Apr 14, 2021

iBlueDust commented Jun 18, 2021

glasser commented Jun 22, 2021

Wrong error code thrown for GraphQL validation failures in arguments using query variables #3498

Wrong error code thrown for GraphQL validation failures in arguments using query variables #3498

Comments

jcfi commented Nov 11, 2019 • edited Loading

Package Name/Version

Expected Behavior

Actual Behavior

Reproduction

Sandbox Link

Instructions

n1ru4l commented Nov 18, 2019

Tydax commented Jan 22, 2020 • edited Loading

fiznool commented Apr 2, 2020

jeromemeichelbeck commented May 19, 2020

mshwery commented Jun 20, 2020

EllaSharakanski commented Apr 4, 2021

glasser commented Apr 6, 2021

glasser commented Apr 7, 2021

glasser commented Apr 9, 2021

glasser commented Apr 14, 2021

iBlueDust commented Jun 18, 2021

Example

glasser commented Jun 22, 2021

jcfi commented Nov 11, 2019 •

edited

Loading

Tydax commented Jan 22, 2020 •

edited

Loading