[NestJS] Sentry Error filter doesn't work in GraphQL context

[lforst]

Hey @nicohrubec - we are using GraphQL/NestJS and it looks like the newest version of the Sentry SDK for NestJS doesn't work for this. Specifically - I think it is somewhat related to this change regressing from #12128, or this comment #12128 (comment)

It doesn't look like there is a way to specify a different base filter in the new version of the NestJS/Sentry SDK like in setupNestErrorHandler - which btw, should that be considered deprecated?

I'm wondering if SentryGlobalFilter needs to allow configuring a custom base filter here https://github.com/getsentry/sentry-javascript/blob/develop/packages/nestjs/src/setup.ts#L63 as you are able to do here

sentry-javascript/packages/node/src/integrations/tracing/nest/nest.ts

Line 55 in 615c670

export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsErrorFilter): void {

With the current impl - we are consistently getting

{
      "query": "query {\n  triggerError\n}",
      "variables": {},
      "errors": [
        {
          "message": "response.status is not a function",
          "locations": [
            {
              "line": 2,
              "column": 3
            }
          ],
          "path": [
            "triggerError"
          ]
        }
      ]
    }

Let me know if you'd like me to open a new issue for this!

Edit - I confirmed monkey-patching the BaseExceptionFilter to ExternalExceptionFilter fixed the issue on GraphQL requests for us

diff --git a/build/cjs/setup.js b/build/cjs/setup.js
index 119735398bb49a01b1c941045a809edd45b32fdc..b06228c7d71288ef53f140de1890e9e4fefe7477 100644
--- a/build/cjs/setup.js
+++ b/build/cjs/setup.js
@@ -6,6 +6,7 @@ Object.defineProperty(exports, '__esModule', { value: true });
 
 const common = require('@nestjs/common');
 const core = require('@nestjs/core');
+const filter = require('@nestjs/core/exceptions/external-exception-filter');
 const microservices = require('@nestjs/microservices');
 const core$1 = require('@sentry/core');
 const utils = require('@sentry/utils');
@@ -47,7 +48,7 @@ common.Injectable()(SentryTracingInterceptor);
 /**
  * Global filter to handle exceptions and report them to Sentry.
  */
-class SentryGlobalFilter extends core.BaseExceptionFilter {
+class SentryGlobalFilter extends filter.ExternalExceptionFilter {
    static  __initStatic2() {this.__SENTRY_INTERNAL__ = true;}
 
   /**

Originally posted by @ysuhaas in #12351

[nicohrubec]

For now I think the solution here should be to just add an additional variation of the SentryGlobalFilter that can handle GraphQL contexts (extending ExternalExceptionFilter). However, with this setup users might still run into issues in certain scenarios as was already discussed before. ref In these cases custom filters are needed. Currently the way to work around this is to use the WithSentry() decorator.

In the future, we can think about providing a factory-method that takes an arbitrary filter and proxies it with some logic to report errors. Instead of useClass developers can then use useFactory to add the proxied filter to their application.

[ralexandr]

I've met same issue, but withSentry could not resolve it
When i add withSentry decorator to my custom AllExceptionsFilter, "exception" and "host" variables are undefined.

 async catch(exception, host) {
    console.log(exception, host);  // This will return "undefined undefined" to stdout
}

[nicohrubec]

@ralexandr Did you put the decorator on the catch directly or on top of the AllExceptionsFilter?

[nicohrubec]

Hey everyone, we added a new SentryGlobalGraphQLFilter, which is essentially a reimplementation of the NestJS ExternalExceptionFilter. This filter should be used in NestJS + GraphQL applications.

In our e2e sample applications, this seems to resolve this issue, but let us know if you run into any further troubles.

[hernandezka]

hi @nicohrubec, have a question, is there any Filter that can work with graphql + rest? I did some test with SentryGlobalGraphQLFilter and with SentryGlobalFilter, but looks like each of them have issues.
For example, when I used SentryGlobalGraphQLFilter, when we throw an exception from REST modules, the response seems to be blocked, and don't resolve/rejects to the requester.
In the other hand, when I used SentryGlobalFilter, the errors thowed by graphql module, lost the sent error messages.

Also, I tried with SentryGlobalGenericFilter, seems to work OK, but seems that it is logging all the "expected" exceptions in sentry console, which is not what we need.
any thoughs?
Best regards 😄

[lforst]

@hernandezka what version of the SDK are you on? The current recommended way to set up error filters is outlined here https://docs.sentry.io/platforms/javascript/guides/nestjs/#use - it somewhat recently changed.

If you have already followed these docs, would you mind giving a specific example with code that doesn't work as expected? Ideally in a new issue so that we can properly track. (also so that Nico the intern can go back to doing university assignments without being bugged on github 😂)

[hernandezka]

@lforst thanks for the quick response!
Will check the docs and open a new ticket 😄
Best regards