apollo-server: Plugin didEncounterErrors should allow a way to add additional information from context

As per other issues, the current recommended way to access the context when handling error in a plugin is to use the hook didEncounterErrors. Something similar to:


export const GraphQLErrorsHandler: ApolloServerPlugin<Context> = {
  requestDidStart(a) {
    return {
      didEncounterErrors(reqCtx) {
        const errors = reqCtx.errors || []
        for (const error of errors) {
          handleError(error, reqCtx.context)
        }
      },
    }
  },
}

The problem is that those error are all readonly and cannot really be modified. For example, it is not possible to add extensions if the caller did not define it as not null. Thus there is not reliable way to add information to the error that could be used by formatError down the line.

A very common use case is adding a request ID to the extensions in case of an error so we can match the backend logs with the frontend error. My current work around is to add the information to the originalError:

  const cause = error.originalError as ExtendedError
  cause.requestId = ctx.request.id

And then read it back in the formatError to add it to the extensions. But I realized recently that originalError is not always set, so my workaround fails sometimes. We also have to note that properties attached to the GraphQLError sent to didEncounterErrors are not carried over in formatErrors for some reason.

For all those reasons, I think there should be an official way to deal with that problem since it was possible to do it with Extensions that are now deprecated.

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 16
Comments: 19 (6 by maintainers)

Most upvoted comments

Another Problem is: The error object in formatError contains additional data of the original error. This is completely missing in all ApolloServer plugin hooks. The error object there is already a stripped and formatted version by ApolloServer and so we cant use that for logging details of e.g. database errors.

We simply need: formatError with context… this will solve all problems

My current use case is: I want to use Sentry to log errors in Graphql resolvers. I want to put all the query details into the Sentry meta informations. This are only available during formatError, however i need the context to access request information to have them as well in the Sentry meta informations.

So there is just no way around either fixing the Plugin hooks to expose the original error or adding context to formatError.

+18

ch1ll0ut1 on Jul 14, 2020

another question on this - why is the error type on didencountererrors and formatError ‘GraphqlError’ and not ‘ApolloError’? Thrown apollo errors lose their codes and make defining them totally pointless?

RichardWright on Aug 18, 2021

This seems valuable but I’m going to remove it from AS3 blockers because I don’t think fixing this needs to be a breaking change: we can start passing context as a second argument to formatError callbacks or allow RequestContext.errors to be mutated without breaking any existing code.

It’s tempting to just drop the readonly on RequestContext.errors or change it to be an array rather than ReadonlyArray, but this presents some trickiness — eg, if you delete all the errors, are we still sending an error response? I realize that @abernix recently commented that we’re trying to keep the formatError API similar to the graphql-js version of it and that didEncounterErrors would be a better place to improve things, but I’m leaning towards passing context to formatError as a better approach since that lets us keep the constraint that the number of errors that exists before didEncounterError matches the number of errors afterwards.

glasser on Jun 10, 2021

How are you supposed to decorate an error object in a plugin if they are readonly?

+1 to overhauling error handling. This area feels very circular. You can transform in formatError but can’t access the request context for logging/tracing, and in plugins you can do so but get the error before it is transformed and can’t pipeline the data you need by adding fields 🔁.

alexkrolick on Dec 9, 2021

extensions is not always defined, so this method is not 100% reliable. I don’t remember exactly when it was not defined in my tests.

Unfortunately Yes! extensions isn’t writable and when it’s undefined or null we can’t change it to object and then adding extra props. But I have another idea:

we can use the message prop (which is writable) as a transport layer, so I changed my code as shown below:

module.exports = {
    requestDidStart: (reqCtx) => {
        return {
            didEncounterErrors: ({ errors, request, context }) => {
                errors.forEach(error => {
                    const msg = error.message;
                    error.message = {
                        request,
                        context,
                        toString: () => msg,
                    }
                })
            },
        }
    },
}

Then we can access additional info in formatError and change message to the default string format (for serializing purposes):

const handler = (error) => {
      const { message, locations, path, extensions = {} } = error;
      const { request, context } = message;

      error.message = message + "";

      return error;
}

fuads96 on Mar 15, 2021