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

Created on 7 Jul 2020  ยท  6Comments  ยท  Source: apollographql/apollo-server

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.

๐Ÿ”Œ plugins

Most helpful comment

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.

All 6 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.

There are use cases to propagate information with error, which is request-specific. This would be nice to have.

Think about request traceability between multiple components, Apollo GW -> Apollo Server. HTTP header have transaction unique identifier (myproduct-transaction: <random string>). It will help if this transaction id could be added to error in one place using formatError or some other methods out of the box instead of handling it within application code.

I just write the plugin and it works properly:

_Edited_: Warning!!!! Don't use the code below, I just keep it for documention purposes. see the new answer

module.exports = {
    requestDidStart: (reqCtx) => {
        return {
            didEncounterErrors: ({ errors, request, context }) => {
                errors.forEach(({ extensions }) => {
                    extensions.request = request;
                    extensions.context = context;
                })
            },
        }
    },
}

Note: delete context in formatError to avoid sending back to users:

formatError.js

const handler = (error) => {
      const context = error.extensions.context;
      // do something with context...

      delete error.extensions["context"];

      return error;
} 

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.

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;
} 
Was this page helpful?
0 / 5 - 0 ratings

Related issues

danilobuerger picture danilobuerger  ยท  3Comments

veeramarni picture veeramarni  ยท  3Comments

Magneticmagnum picture Magneticmagnum  ยท  3Comments

hiucimon picture hiucimon  ยท  3Comments

dobesv picture dobesv  ยท  3Comments