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

Error from GraphQLScalarType is not properly propagated to formatError hook #7178

Closed
junajan opened this issue Nov 21, 2022 · 3 comments · Fixed by #7183
Closed

Error from GraphQLScalarType is not properly propagated to formatError hook #7178

junajan opened this issue Nov 21, 2022 · 3 comments · Fixed by #7183

Comments

@junajan
Copy link

junajan commented Nov 21, 2022

Hello,
we have a problem when throwing a GraphQLError from GraphQLScalarType in the new apollo v4. Before (in v3) it properly propagated the error into formatError hook but now it always changes its extensions.code to BAD_USER_INPUT and removes all additional fields.

This is happening only on errors thrown from GraphQLScalarType (e.g. errors from mutation/query resolvers work just fine) and only when we define GQL schema using GraphQLSchema. I tried this example where the schema is defined using typeDefs and resolvers keys and it also worked.

Here is the code where you can reproduce the error.
const { unwrapResolverError } = require('@apollo/server/errors');
const { ApolloServer } = require('@apollo/server');
const { startStandaloneServer } = require('@apollo/server/standalone');
const { GraphQLScalarType, GraphQLError } = require('graphql');
const { GraphQLSchema, GraphQLObjectType, GraphQLInputObjectType } = require('graphql');

const ColorScalarField = new GraphQLScalarType({
  name: 'ScalarField',
  serialize: (value) => value,
  parseValue: () => {
    console.log("Serializing")

    throw new GraphQLError('Scalar field custom error', {
      extensions: { code: 'CUSTOM_ERROR_CODE', extraErrorField: 123 },
    });
  },
  parseValue: () => {
    console.log("Parsing value")

    throw new GraphQLError('Scalar field custom error', {
      extensions: { code: 'CUSTOM_ERROR_CODE', extraErrorField: 123 },
    });
  },
  parseLiteral: () => {
    console.log("Parsing literal")

    throw new GraphQLError('Scalar field custom error', {
      extensions: { code: 'CUSTOM_ERROR_CODE', extraErrorField: 123 },
    });
  },
});

const ItemType = new GraphQLObjectType({
  name: 'item',
  fields: () => ({
    color: {
      type: ColorScalarField,
    },
  }),
});

const ItemUpdateType = new GraphQLInputObjectType({
  name: 'ItemUpdate',
  fields: () => ({
    color: {
      type: ColorScalarField,
    },
  }),
});

const schema = new GraphQLSchema({
  query: new GraphQLObjectType({
    name: 'Query',
    fields: () => ({
      item: {
        type: ItemType,
        description: 'Fetch the logged in user.',
        resolve: () => {},
      }
    }),
  }),
  mutation: new GraphQLObjectType({
    name: 'Mutation',
    fields: () => ({
      itemUpdate: {
        name: 'itemUpdate',
        args: {
          input: {
            type: ItemUpdateType,
          },
        },
        type: ItemType,
        resolve: () => {},
      }
    }),
  }),
});

const server = new ApolloServer({
  schema,
  debug: true,
  formatError: (err, err2) => {
    console.log("FORMAT ERROR:", err, err2)
    console.log("UNWRAPPED ERROR:", unwrapResolverError(err), unwrapResolverError(err2))

    return { err: 123 };
  }
});

(async () => {
  const { url } = await startStandaloneServer(server);
  console.log(`🚀 Server listening at: ${url}`);

  const response = await server.executeOperation({
    query: `
      mutation itemUpdate($input: ItemUpdate!) {
        itemUpdate(input: $input) {
          color
        }
      }
    `,
    variables: {
      input: {
        color: 'red',
      },
    },
  });

  console.log(response.body.singleResult.data)
  console.log(response.body.singleResult.errors)
})();
And here is the output.
Parsing value
FORMAT ERROR: {
  message: 'Variable "$input" got invalid value "red" at "input.color"; Scalar field custom error',
  locations: [ { line: 2, column: 27 } ],
  extensions: {
    code: 'BAD_USER_INPUT',
    stacktrace: [
      'GraphQLError: Variable "$input" got invalid value "red" at "input.color"; Scalar field custom error',
      '    at /test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:147:11',
      '    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:154:9)',
      '    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:117:34)',
      '    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:49:14)',
      '    at coerceInputValue (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:32:10)',
      '    at coerceVariableValues (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:132:69)',
      '    at getVariableValues (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:45:21)',
      '    at buildExecutionContext (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/execute.js:280:63)',
      '    at execute (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/execute.js:116:22)',
      '    at executeIncrementally (/test/node_modules/.pnpm/@[email protected][email protected]/node_modules/@apollo/server/dist/cjs/incrementalDeliveryPolyfill.js:47:34)'
    ]
  }
} GraphQLError: Variable "$input" got invalid value "red" at "input.color"; Scalar field custom error
    at /test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:147:11
    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:154:9)
    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:117:34)
    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:49:14)
    at coerceInputValue (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:32:10)
    at coerceVariableValues (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:132:69)
    at getVariableValues (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:45:21)
    at buildExecutionContext (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/execute.js:280:63)
    at execute (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/execute.js:116:22)
    at executeIncrementally (/test/node_modules/.pnpm/@[email protected][email protected]/node_modules/@apollo/server/dist/cjs/incrementalDeliveryPolyfill.js:47:34) {
  path: undefined,
  locations: [ { line: 2, column: 27 } ],
  extensions: { code: 'BAD_USER_INPUT' }
}
UNWRAPPED ERROR: {
  message: 'Variable "$input" got invalid value "red" at "input.color"; Scalar field custom error',
  locations: [ { line: 2, column: 27 } ],
  extensions: {
    code: 'BAD_USER_INPUT',
    stacktrace: [
      'GraphQLError: Variable "$input" got invalid value "red" at "input.color"; Scalar field custom error',
      '    at /test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:147:11',
      '    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:154:9)',
      '    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:117:34)',
      '    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:49:14)',
      '    at coerceInputValue (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:32:10)',
      '    at coerceVariableValues (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:132:69)',
      '    at getVariableValues (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:45:21)',
      '    at buildExecutionContext (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/execute.js:280:63)',
      '    at execute (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/execute.js:116:22)',
      '    at executeIncrementally (/test/node_modules/.pnpm/@[email protected][email protected]/node_modules/@apollo/server/dist/cjs/incrementalDeliveryPolyfill.js:47:34)'
    ]
  }
} GraphQLError: Variable "$input" got invalid value "red" at "input.color"; Scalar field custom error
    at /test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:147:11
    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:154:9)
    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:117:34)
    at coerceInputValueImpl (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:49:14)
    at coerceInputValue (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/utilities/coerceInputValue.js:32:10)
    at coerceVariableValues (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:132:69)
    at getVariableValues (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/values.js:45:21)
    at buildExecutionContext (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/execute.js:280:63)
    at execute (/test/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/execute.js:116:22)
    at executeIncrementally (/test/node_modules/.pnpm/@[email protected][email protected]/node_modules/@apollo/server/dist/cjs/incrementalDeliveryPolyfill.js:47:34) {
  path: undefined,
  locations: [ { line: 2, column: 27 } ],
  extensions: { code: 'BAD_USER_INPUT' }
}
undefined
[ { err: 123 } ]

I would expect the error received in formatError hook to have extensions.code === 'CUSTOM_ERROR_CODE' and extensions.extraErrorField === 123.

Is there anything we missed? Thank you for any help.

@glasser
Copy link
Member

glasser commented Nov 22, 2022

So it looks to me that the information is lost inside graphql (graphql-js): the code that runs the parseValue responds to errors by wrapping the error in a new GraphQLError that forgets everything about error other than error.message and error.originalError, and you have no originalError here:
https://github.com/graphql/graphql-js/blob/6b5c8af150350201d0d67f3eb6f6f44cb6f92288/src/execution/values.ts#L134

(I think maybe that line should say originalError: error.originalError ?? error. Thoughts @IvanGoncharov ?)

This change was introduced in graphql v14.5.0 in graphql/graphql-js#2062 I think.

What's a bit confusing to me is that this ever worked (with graphql 14.5 or newer). You say it works when using the resolvers signature, and also in AS3. Can you share reproductions of that? (Ideally as a repo I can git clone so that I am sure I have the same dependency versions as you.)

Note that unwrapResolverError isn't really expected to work here as this isn't a resolver error (it has no path), but you could use err.originalError yourself if it's actually there...

As a workaround for now you could do this:

    const originalError = new GraphQLError('Scalar field custom error', {
      extensions: { code: 'CUSTOM_ERROR_CODE', extraErrorField: 123 },
    });
    throw new GraphQLError(originalError.message, {originalError})

You'll then end up with formatError called where the first (formatted) error is something like:

{
  message: 'Variable "$input" got invalid value "red" at "input.color"; Scalar field custom error',
  locations: [ { line: 2, column: 27 } ],
  extensions: { code: 'BAD_USER_INPUT', extraErrorField: 123 }
}

and the second argument err2 has an originalError field equal to

GraphQLError: Scalar field custom error
    at GraphQLScalarType.parseValue (/private/tmp/scalar-errors/errors.js:20:27)
    at coerceInputValueImpl (/private/tmp/scalar-errors/node_modules/graphql/utilities/coerceInputValue.js:151:26)
    at coerceInputValueImpl (/private/tmp/scalar-errors/node_modules/graphql/utilities/coerceInputValue.js:117:34)
    at coerceInputValueImpl (/private/tmp/scalar-errors/node_modules/graphql/utilities/coerceInputValue.js:49:14)
    at coerceInputValue (/private/tmp/scalar-errors/node_modules/graphql/utilities/coerceInputValue.js:32:10)
    at coerceVariableValues (/private/tmp/scalar-errors/node_modules/graphql/execution/values.js:132:69)
    at getVariableValues (/private/tmp/scalar-errors/node_modules/graphql/execution/values.js:45:21)
    at buildExecutionContext (/private/tmp/scalar-errors/node_modules/graphql/execution/execute.js:280:63)
    at execute (/private/tmp/scalar-errors/node_modules/graphql/execution/execute.js:116:22)
    at executeIncrementally (/private/tmp/scalar-errors/node_modules/@apollo/server/dist/cjs/incrementalDeliveryPolyfill.js:47:34) {
  path: undefined,
  locations: undefined,
  extensions: { code: 'CUSTOM_ERROR_CODE', extraErrorField: 123 }
}

I think this is close to what you want... though arguably our thing that adds BAD_USER_INPUT should only do that if there's not already a code there.

However it does feel like the main issue is that graphql-js should use the error you threw as originalError if it has no originalError on it...

@junajan
Copy link
Author

junajan commented Nov 22, 2022

Hi @glasser, thank you for your quick response.

I tried again the example from docs and it does not work either (sorry for confusion, on the first try I was getting INTERNAL_SERVER_ERROR error code and then it got changed to that BAD_USER_INPUT so I thought it started working but it was due to some invalid plugin).

That trick with originalError works and fields (except code) are being properly propagated to formatError hook now, thank you.

glasser added a commit to glasser/graphql-js that referenced this issue Nov 22, 2022
Previously, the only fields observed on an error thrown by (for example)
parseValue were `message` and `originalError`. Now, if an error has no
`originalError`, it is itself used as the original error.

Addresses an issue raised in
apollographql/apollo-server#7178
glasser added a commit that referenced this issue Nov 22, 2022
Adding `BAD_USER_INPUT` is a nice default (and overrides the
inappropriate default of `INTERNAL_SERVER_ERROR`) but if somebody has
set a `code` already, we shouldn't override.

(Note that there's a separate issue where graphql-js throws out
extensions from the thrown error itself and only pays attention to
extensions on the error's originalError; we're trying to fix that in
graphql/graphql-js#3785 but this is orthogonal.)

Fixes #7178.
@glasser
Copy link
Member

glasser commented Nov 22, 2022

I've filed #7183 to fix the "we write BAD_USER_INPUT even if there's already a code" issue in Apollo Server, and graphql/graphql-js#3785 to fix the "throw away extensions that are directly on error rather than nested in error.originalError" issue in graphql-js. I'll close this issue when the former merges since the latter is out of our control (and has an awkward workaround).

glasser added a commit that referenced this issue Nov 23, 2022
…7183)

Adding `BAD_USER_INPUT` is a nice default (and overrides the
inappropriate default of `INTERNAL_SERVER_ERROR`) but if somebody has
set a `code` already, we shouldn't override.

(Note that there's a separate issue where graphql-js throws out
extensions from the thrown error itself and only pays attention to
extensions on the error's originalError; we're trying to fix that in
graphql/graphql-js#3785 but this is orthogonal.)

Fixes #7178.

Co-authored-by: Trevor Scheer <[email protected]>
glasser added a commit to glasser/graphql-js that referenced this issue Nov 23, 2022
Previously, the only fields observed on an error thrown by (for example)
parseValue were `message` and `originalError`. Now, the error itself is
used as the `originalError`; this may be mildly backwards-incompatible
if you added extensions on the error itself which you for some reason
wanted to disappear, but that seems unlikely.

Addresses an issue raised in
apollographql/apollo-server#7178
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants