Apollo-server: Request: Provide context on global error handlers

[thanpolas]

It would be very beneficial to have the request context on the global error handlers so we can have better insights as to who caused an error and have a better chance at debugging it.

[martijnwalraven]

@thanpolas What do you mean by global error handlers here?

[thanpolas]

The options from the GraphQL.... variants for each service (express, koa, etc) have a key named formatError expecting a function.

That callback function, should also receive the context of the request as defined again in the same graphqlExpress() method.

• Context I'm talking about.
• formatError

[evans]

@thanpolas That's interesting! What would you like to do with the context in formatError? The function is generally used to mask error values, which often doesn't require the context.

Another option is to construct an Apollo Error with the values from the context that you need. https://www.apollographql.com/docs/apollo-server/v2/features/errors.html#Other-errors

[thanpolas]

My use case would be quite specific but I imagine there are plenty of uses one can have with the context available at the global error handler.

We are using knex in our project, instead of an ORM, and want to simplify and make more robust the operation of doing SQL Transactions. The way it's done now is that you'd request a transaction object from knex, perform your SQL operations very carefully and all in one place so as to have absolute control on any of them failing so you can rollback and not have open connections (due to the open transaction) ghosting on you.

This method presents a few challenges primarily because we have to concentrate all DB operations in one place.

If we have the context available on the global error handler, we can assign the transaction object on the context and on the global handler roll it back if it's there.

This will capture all exception cases including the ones that were not captured or controlled & generated by the codebase.... Which is the reason why assigning the transaction object on the thrown Error object of a controlled exception is not a good solution for us as it doesn't account for the exception that will get thrown outside our control or our try/catch statements.

tl;dr:

• It will dramatically simplify sql transaction management for us as we'll be able to rely on context as the central point of reference and be able to decouple the DB operations into their respective models.
• It will be a robust and absolute solution for managing failing transactions and rolling them back.

[thanpolas]

One more point is that we'll more easily debug untraced errors. Meaning, we can flag all the errors we throw in the codebase so as to indicate they originate from a known occasion. Then on the global error handler filter for the errors that don't have that flag, which means those are errors that were thrown from unexpected places... if context is present in those cases, it would greatly help the debugging efforts as the error would log along with all the context of the context and help engineers debug faster...

[maxtechera]

This would be really useful for us too ! As we are injecting an instance of i18n in the context and using it to translate error messages.
On [email protected] we accomplish this by using the options as a function for graphqlExpress and creating a format error for each new request (not optimal) but works.
AFAIK, there's no way to do this in V2.

[ak99372]

@thanpolas we do transactions too but they have to be encapsulated around execution of resolvers. I would not recommend doing rollback or transactions on formatError (which purpose is to format after the result is ready)

However if you need to access the context there is also [formatResponse] that includes all server options including the context.

[thanpolas]

yes, that is correct @ak99372, transactions should be handled within the closure that provides them in knex to benefit from automatic commit/rollback. Do you know if formatResponse gets executed before or after formatError is invoked?

[rewop]

I am having the same need. In my case I want to log the error, an report it to bugsnag. However, since in the logger we print request ids that are unique per request, we inject the logger in the context.

Since formatError doesn't have the context, I cannot log the request_id to not managed errors.

[twelve17]

Similar to @rewop, we used this option to log the error, including the request id, query variables, etc.

With Apollo 1, we were able to get access to the request params because the options were inside the scope of the function with it. Alas, once it was moved into the context option, formatError no longer has access to it.

[disintegrator]

This would be super handy for my use case. I want to log the request id alongside errors like so:

{
  formatError(err, ctx) {
    const id = ctx.requestId

    logger.error({ err, id })

    return { message, id, locations, path  };
  }
}

Currently, the context is only available in resolvers. This means devs have to ensure the request id is capture at error creation time. It's easy to miss that...

const someFieldResolver = async (_, __, ctx) => {
  try {
    await doSomething();
  } catch (err) {
    // This can be tedious and easy to forgot
    throw new ServiceError({ message: err.message, id: ctx.requestId })
  }  
}

[kennethaasan]

Would be great having access to req and res as well in formatError so you can log the request context as well as change the res.statusCode.

[thanpolas]

Would be great having access to req and res as well in formatError so you can log the request context as well as change the res.statusCode.

@kennethaasan you can attach req and res on the context

[Whoaa512]

I would love to have this feature because we attach some AWS X-Ray logging utilities on the context object that we use to capture the error with other data from the context.

I could see this being implemented as an extension but after looking through the code I didn't see that the context was passed to any of the methods. Personally I think this makes more sense.

@evans, would you be interested in a PR for something like this? If so would you prefer the functionality happen within formatError or the extensions api?

[maimike]

so any way to do so. i checked extension PR but still do not know how to apply