GraphQL plug-in prevents Sentry middleware from catching errors

[jorisw]

Bug report

Describe the bug

Because the Sentry plug-in's middleware comes after that of the GraphQL plug-in, and the GraphQL plug-in catches fatal errors for reporting in the response, fatal errors as part of GraphQL operations are not reported to Sentry.

Other middlewares are likely to suffer from the GraphQL plug-in catching the errors, too.

Steps to reproduce the behavior

1. Enable the GraphQL plug-in
2. Enable the Sentry plug-in
3. Put a sendError() call somewhere and trigger the call
4. Notice the error is reported to Sentry
5. Put some nonsensical code in a controller or model such as hi()
6. Do a GraphQL query that reaches this code
7. Notice the error is part of the GraphQL response
8. Notice the error is not caught by the Sentry plug-in/middleware, therefore not reported to Sentry

Expected behavior

I expect fatal errors to be both reported by the GraphQL response, as well as reported by the Sentry plug-in to Sentry

Screenshots

These console logs, added to node_modules/strapi-sentry-plugin/middlewares/sentry/index.js, are not logged when calling a GraphQL query that triggers an error:

I also tried to put the Sentry middleware before that of GraphQL, to no avail, probably because we're talking about middlewares as part of plug-ins:

// config/middleware.js

module.exports = () => ({
  load: { order: ['sentry', 'graphql'] },
})

System

• Node.js version: v14.15.0
• Yarn version: 1.22.5
• Strapi version: 3.6.2
• Database: MySQL
• Operating system: macOS 11.3

[jorisw]

FWIW, I think this issue represents a broader problem in the design of the middlewares-in-plugins queuing system rather than a bug in the Sentry plug-in.

[derrickmehaffy]

FWIW, I think this issue represents a broader problem in the design of the middlewares-in-plugins queuing system rather than a bug in the Sentry plug-in.

Agreed, we are discussing it internally but we should summarize that discussion here @remidej

[IvanCoronado]

@jorisw I am facing exactly this problem. Did you find some workaround?

My best temporal solution by now is wrapping my controllers with a try/catch and manually send the error to Sentry but it's too much extra code...

[jorisw]

@IvanCoronado Indeed that's what we're doing.

@derrickmehaffy Any updates on this?

[ralphsomeday]

@jorisw I am facing exactly this problem. Did you find some workaround?

My best temporal solution by now is wrapping my controllers with a try/catch and manually send the error to Sentry but it's too much extra code...

@alexandrebodin any idea how we can solve this as the Sentry plugin is worthless when using Strapi with Graphql? thanks

[derrickmehaffy]

@IvanCoronado Indeed that's what we're doing.

@derrickmehaffy Any updates on this?

@remidej is currently out of the office for a while, let me poke the team and see if it's going to be reassigned to someone else.

[ralphsomeday]

@derrickmehaffy any update on this one?

[derrickmehaffy]

@remidej any information you can offer here?

[remidej]

Not yet sorry 😕

[ralphsomeday]

@remidej when do you think you will have time to look into it?

[derrickmehaffy]

This issue has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/how-can-i-persist-logging-beyond-application-running-scope/8595/3

[ralphsomeday]

@derrickmehaffy in current Strapi version 3.6.x Sentry plugin is unusable... Is anyone from the core team looking into it? thanks

[jorisw]

@jorisw I am facing exactly this problem. Did you find some workaround?

My best temporal solution by now is wrapping my controllers with a try/catch and manually send the error to Sentry but it's too much extra code...

We were able to get rid of try/catch blocks all over the controllers, by adding the following to extensions/graphql/config/settings.js.

There may be a better place to put this than in extensions/, we haven't looked into that yet.

module.exports = {
  apolloServer: {
    plugins: [
      {
        requestDidStart() {
          return {
            didEncounterErrors(apolloContext) {
              const koaContext = apolloContext.context.context
              apolloContext.errors.forEach(error => {
                strapi.log.error(error)
                strapi.sentry.captureRequestError(error, koaContext)
              })
            },
          }
        },
      },
    ],
  },
}

Perhaps the Strapi team could use such an approach to fix their Sentry plug-in.

[derrickmehaffy]

Fixed in v4