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

GraphQL error doesn't have stack trace #4474

Closed
gaastonsr opened this issue Aug 11, 2020 · 6 comments
Closed

GraphQL error doesn't have stack trace #4474

gaastonsr opened this issue Aug 11, 2020 · 6 comments
Labels
🍐 error-handling Pertaining to error handling (or lack thereof), not just for just general errors/bugs.

Comments

@gaastonsr
Copy link

Apollo version "apollo-server-express": "^2.16.1".

Inside formatError I'm reporting unexpected errors like this:

formatError: (graphqlError) => {
  if (shouldReportError(graphqlError) {
    reportError(graphqlError)
  }

  return isKnownError(graphqlError) ? graphqlError : new Error('ServerError')
}

It works great for errors returned inside my resolvers, however, when the error is a parsing error because my GraphQL query was malformed:

  1. graphqlError doesn't have a stack property, so reporting it to a monitoring tool like Sentry is awkward because it only has an error name and message.
  2. Neither I have a way to know if it was a GraphQL parsing error so I know it's safe to return it to the user and I don't mask it with a ServerError.

So my questions are:

  1. Is it normal that graphqlError doesn't have a stack property?
  2. How do you know if it was a parsing or query validation error so those errors are not reported?
@gaastonsr
Copy link
Author

Oh, besides graphqlError not having a stack trace, the validation error seems to be reported a bunch of times:

Without properly identifying client errors we can receive a flood of errors...

@bhishp
Copy link

bhishp commented Oct 5, 2020

I have an issue somewhat related to this, I am seeing Resolver errors with a stack trace, but it is a masked stack trace. The Error type/stack/metadata is changed when being executed within a Resolver. This is particularly an issue for Sentry as a tool because Sentry groups reported exceptions based on their stack trace - therefore we might see various different types of errors being counted as the same error purely because they have the same stack trace.

We do something like this:

      didEncounterErrors(ctx) {
          for (const err of ctx.errors) {
            Sentry.captureException(err.originalError);
          }
        });
      },

The stack is exactly the same for every error coming through this event hook, therefore Sentry thinks they are all the same error.

Original Error (Thrown by calling tedious lib within resovler)

QueryFailedError: Error: Timeout: Request failed to complete in 50ms
    at new QueryFailedError (/app/src/error/QueryFailedError.ts:9:9)
    at /app/src/driver/sqlserver/SqlServerQueryRunner.ts:223:37
    at /app/node_modules/mssql/lib/base.js:1346:25
    at Request.userCallback (/app/node_modules/mssql/lib/tedious.js:671:15)
    at Request.callback (/app/node_modules/tedious/lib/request.js:37:27)
    at Connection.message (/app/node_modules/tedious/lib/connection.js:2136:24)
    at Connection.dispatchEvent (/app/node_modules/tedious/lib/connection.js:1084:36)
    at MessageIO.<anonymous> (/app/node_modules/tedious/lib/connection.js:984:14)
    at MessageIO.emit (events.js:311:20)
    at MessageIO.EventEmitter.emit (domain.js:505:15)

Same error seen in didEncounterErrors

ctx.errors[0].stack OR ctx.errors[0].originalError.stack (both hold the same stack and new Error type)

Error: Error: Timeout: Request failed to complete in 50ms
    at new CombinedError (/app/node_modules/graphql-tools/src/stitching/errors.ts:90:5)
    at Object.checkResultAndHandleErrors (/app/node_modules/graphql-tools/src/stitching/errors.ts:111:11)
    at CheckResultAndHandleErrors.transformResult (/app/node_modules/graphql-tools/src/transforms/CheckResultAndHandleErrors.ts:15:12)
    at /app/node_modules/graphql-tools/src/transforms/transforms.ts:37:45
    at Array.reduce (<anonymous>)
    at applyResultTransforms (/app/node_modules/graphql-tools/src/transforms/transforms.ts:35:21)
    at /app/node_modules/graphql-tools/src/stitching/delegateToSchema.ts:104:12
    at step (/app/node_modules/graphql-tools/dist/stitching/delegateToSchema.js:32:23)
    at Object.next (/app/node_modules/graphql-tools/dist/stitching/delegateToSchema.js:13:53)
    at fulfilled (/app/node_modules/graphql-tools/dist/stitching/delegateToSchema.js:4:58)

Not only do we lose the stack trace but we also lose the Error type (this prevent us from doing things like if (originalError instanceof QueryFailedError) { ... }.

An Error manually thrown from a resolver and seen in didEncounterErrors

Error: Test throw
    at new CombinedError (/app/node_modules/graphql-tools/src/stitching/errors.ts:90:5)
    at Object.checkResultAndHandleErrors (/app/node_modules/graphql-tools/src/stitching/errors.ts:111:11)
    at CheckResultAndHandleErrors.transformResult (/app/node_modules/graphql-tools/src/transforms/CheckResultAndHandleErrors.ts:15:12)
    at /app/node_modules/graphql-tools/src/transforms/transforms.ts:37:45
    at Array.reduce (<anonymous>)
    at applyResultTransforms (/app/node_modules/graphql-tools/src/transforms/transforms.ts:35:21)
    at /app/node_modules/graphql-tools/src/stitching/delegateToSchema.ts:104:12
    at step (/app/node_modules/graphql-tools/dist/stitching/delegateToSchema.js:32:23)
    at Object.next (/app/node_modules/graphql-tools/dist/stitching/delegateToSchema.js:13:53)
    at fulfilled (/app/node_modules/graphql-tools/dist/stitching/delegateToSchema.js:4:58)

This is a completely different error but because the stack trace is exactly the same it gets reported as the same error in sentry

Note: using "apollo-server-express": "2.18.1",

@bhishp
Copy link

bhishp commented Oct 6, 2020

Fyi, the way I got these to group in Sentry is by overriding the fingerprint as such:

      didEncounterErrors(ctx) {
          Sentry.withScope(scope => {
            const graphqlKind = ctx.operation.operation; // e.g. query
            const operationName = ctx.operationName; // e.g. GetUsers
            for (const err of ctx.errors) {
              scope.setFingerprint([graphqlKind, operationName, err.originalError.message]);
              Sentry.captureException(err.originalError);
            }
          });
      },

But as mentioned above, it is still not ideal because the stack is not reported, therefore we won't be able to pinpoint exactly where this error was thrown from.

@bhishp
Copy link

bhishp commented Oct 6, 2020

Update on my issue, looks like this could be related to an issue in graphql-tools (ardatan/graphql-tools#743). Following the thread I believe it should be fixed in v5 but apollo-server-express is still dependent upon ^4.0.0.

This workaround worked for me. In the end I managed to get the correct stack as below (note, the Error type is still lost).

      didEncounterErrors(ctx) {
            for (const err of ctx.errors) {
              if (err.originalError) {
                // eslint-disable-next-line // need this because ApolloServerPlugin is typed as Maybe<Error>
                for (const originalErrorI of (err.originalError as any).errors) {
                  console.log(originalErrorI.name); // GraphQLError
                  console.log(originalErrorI.stack); // the original stack
                  Sentry.captureException(originalErrorI);
                }
              }
            }
      },

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

xinghul commented Mar 18, 2022

any updates on this? thanks

@glasser
Copy link
Member

glasser commented Mar 18, 2022

There's a lot of subtlety involved in Apollo Server error handling, and this issue (which was filed during an unfortunate and I sure hope one-time period in 2020 where Apollo Server was not being actively maintained) doesn't contain a full reproduction recipe that allows me to see the issue on my machine without taking creative steps. So I'm going to close this as lacking a reproduction. I note that some of the above comments seem to be about schema stitching (which is a graphql-tools project and not part of Apollo Server itself, and also Apollo Server 3 does use a newer version of graphql-tools now) and others don't so there might be multiple issues here.

@glasser glasser closed this as completed Mar 18, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 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

5 participants