EngineReportingOptions.rewriteError hides error subtypes

[lpgera]

First of all, huge thanks for taking my original PR further and implementing rewriteError in https://github.com/apollographql/apollo-server/pull/2618! 🙇

I was trying to get rewriteError to work with our Apollo Engine integration, but checking the type of the error like in the example doesn't work for me, because AuthenticationError and ForbiddenError are converted to GraphQLError somewhere along the way, and the only way to distinguish them from other errors is checking the error code.

I created a repo which can be used to reproduce the issue: https://github.com/lpgera/apollo-server-rewriteerror-testing

After setting the apiKey, and running a hello query the isAuthenticationError should be true in the log, but it's always false.

Am I missing something or is the documentation wrong about checking the type of the error with instanceof?

[abernix]

Thanks for reporting this and for providing the reproduction code.

Is it possible that you need to check err.originalError instanceof AuthenticationError? Could you try?

[lpgera]

No luck with that either, err.originalError is undefined. 😞

[robross0606]

We were having the same issue until I just figured out that graphql-tools was swallowing the errors at some point during the stitching through mergeSchema().

[rdougan]

Looks like the TreeBuilder is cloning the original error which then ditches all custom properties:

https://github.com/apollographql/apollo-server/blob/master/packages/apollo-engine-reporting/src/treeBuilder.ts#L189

[jacob-israel-turner]

Has anybody solved this issue? I believe we're running into this now, at least in some cases.

[jacob-israel-turner]

Ok, I was able to get around this by drilling into error.extensions.code in rewriteError. For AuthenticationError, the code is UNAUTHENTICATED, and for UserInputError, the code is BAD_USER_INPUT. While this workaround should work, this is clearly different than what's in the documentation. Either the rewriteError method should be updated to retain the type of the original error, or the documentation should be updated to show how to get the code, and the codes should be readily available as well.

[jkepps]

it's been over two years since this was reported and it still hasn't been addressed.... any updates @abernix? If this isn't going to be fixed the documentation should be updated.

[glasser]

I think the simplest thing to do here would be to change the docs to compare codes rather than use instanceof, since that seems less brittle. Happy to take a PR to fix that. (I'm not sure I 100% understand the issue as the code in traceTreeBuilder.ts does appear to try to preserve the prototype and properties?)

[jkepps]

I think the simplest thing to do here would be to change the docs to compare codes rather than use instanceof, since that seems less brittle.

@glasser This is what we have in our project and it still isn't working. We are still getting an extremely high rate of errors in apollo studio for mutations that handle login and form submissions that are simply the result of invalid inputs.

ApolloServerPluginUsageReporting({
 rewriteError(error) {
    if (
      error.extensions?.code === 'BAD_USER_INPUT' ||
      error.extensions?.code === 'UNAUTHENTICATED' ||
      error.extensions?.code === 'FORBIDDEN'
    ) {
      return null
    }
    return error
  },
})

@trevor-scheer @abernix anything?

[glasser]

I don't think I'll be able to help you debug this without a complete reproduction (eg something I can git clone or look at in codesandbox.io).

[jkepps]

yeah that's alright @glasser, appreciate the response.

i think i was able to solve it by including both the UsageReporting plugin and the InlineTracing plugin and passing the rewriteError function to both. for some reason it appears the inline tracing filters the errors in my prod env but not my dev, and the usage reporting filters in my dev env, but not prod

[glasser]

I think the simplest thing to do here would be to change the docs to compare codes rather than use instanceof, since that seems less brittle. Happy to take a PR to fix that. (I'm not sure I 100% understand the issue as the code in traceTreeBuilder.ts does appear to try to preserve the prototype and properties?)

We leaned into this approach: in Apollo Server 4 there are no special error subclasses (in fact, even ApolloError is gone), and we explicitly document looking at error.extensions.code to figure out what error you have (and export an enum covering the codes for the errors that Apollo Server's own code produces).