Skip to content

setupNestErrorHandler does not work properly within a NestJS + GraphQL application #12128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3 tasks done
Blargel opened this issue May 21, 2024 · 6 comments
Closed
3 tasks done
Assignees
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@Blargel
Copy link

Blargel commented May 21, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.2.1

Framework Version

@nestjs/core@10.3.8, @nestjs/graphql@12.1.1

Link to Sentry event

No response

SDK Setup

Sentry.init({
  dsn: [redacted],
  integrations: [nodeProfilingIntegration()],
  tracesSampleRate: 1.0,
  profilesSampleRate: 1.0,
})

async function bootstrap(): Promise<void> {
  const app = await NestFactory.create(AppModule)
  Sentry.setupNestErrorHandler(
    app,
    new BaseExceptionFilter(app.getHttpAdapter()),
  )
  await app.listen(config.port)
}

Steps to Reproduce

  1. Clone this repo: https://github.com/Blargel/sentry-nestjs-graphql-test
  2. npm install
  3. Open src/main.ts and change the DSN const to something valid.
  4. npm start
  5. In a web browser, go to http://localhost:3000/
  6. Observe that an error is raised and reported to Sentry correctly. This is a regular HTTP endpoint with an error and works as expected.
  7. In a web browser, go to http://localhost:3000/graphql
  8. This will open a GraphQL explorer. I believe it defaults to the correct query for testing but if not, the correct test query is query {hello}.
  9. Execute the query by pressing the play button in the top middle of the GraphQL explorer.
  10. Observe that a strange TypeError is raised and the original error is not sent to Sentry correctly.

Expected Result

The GraphQL error should also be reported to Sentry without raising the strange TypeError.

Actual Result

No error reported in Sentry and the following error is thrown:

TypeError: response.status is not a function
    at ExpressAdapter.reply (~/platform-api/node_modules/@nestjs/platform-express/adapters/express-adapter.js:28:22)
    at BaseExceptionFilter.handleUnknownError (~/platform-api/node_modules/@nestjs/core/exceptions/base-exception-filter.js:46:28)
    at BaseExceptionFilter.catch (~/platform-api/node_modules/@nestjs/core/exceptions/base-exception-filter.js:17:25)
    at Proxy.<anonymous> (~/platform-api/node_modules/@sentry/src/integrations/tracing/nest.ts:80:51)
    at ExternalExceptionsHandler.invokeCustomFilters (~/platform-api/node_modules/@nestjs/core/exceptions/external-exceptions-handler.js:31:32)
    at ExternalExceptionsHandler.next (~/platform-api/node_modules/@nestjs/core/exceptions/external-exceptions-handler.js:14:29)
    at ~/platform-api/node_modules/@nestjs/core/helpers/external-proxy.js:14:42
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 21, 2024
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label May 21, 2024
@onurtemizkan
Copy link
Collaborator

Thanks for reporting this @Blargel!

It looks like GraphQL does not work well with BaseExceptionFilter. (Ref: nestjs/nest#5958 (comment))

I have tried to update your setup as,

async function bootstrap(): Promise<void> {
  const app = await NestFactory.create(AppModule)
  Sentry.setupNestErrorHandler(
    app,
    new ExternalExceptionFilter(),
  )
  await app.listen(config.port)
}

And it seemed to work.

Could you check if that resolves the issue?

If so we may need to add a note in the docs about this.

@Blargel
Copy link
Author

Blargel commented May 21, 2024

Hmmmm, that works in the test repo I put in my bug report, but it's not working on my actual application. The same TypeError as before is still being thrown. Let me get back to you on this when I figure out what's different.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 21, 2024
@Blargel
Copy link
Author

Blargel commented May 21, 2024

Oops, nevermind it works. Maybe something weird got cached. Anyways, that does fix my issue then. Thanks! Should I leave this open to track documentation updates?

@mydea
Copy link
Member

mydea commented May 22, 2024

Great that we found a solution - I'll close the issue, users can (hopefully) still find it if they search! 🙏

@GustavoContreiras
Copy link

GustavoContreiras commented May 23, 2024

It stopped throwring the response.status error but now I can't access the GraphQL Playground...

If I comment Sentry.setupNestErrorHandler(app, new ExternalExceptionFilter()) I can access again.

C:\dev\xxxxx\nestjs\node_modules\@nestjs\core\router\routes-resolver.js:77
            throw new common_1.NotFoundException(`Cannot ${method} ${url}`);
                  ^
NotFoundException: Cannot GET /favicon.ico
    at callback (C:\dev\xxxxx\nestjs\node_modules\@nestjs\core\router\routes-resolver.js:77:19)
    at C:\dev\xxxxx\nestjs\node_modules\@nestjs\core\router\router-proxy.js:9:23
    at Layer.handle [as handle_request] (C:\dev\xxxxx\nestjs\node_modules\express\lib\router\layer.js:95:5)
    at trim_prefix (C:\dev\xxxxx\nestjs\node_modules\express\lib\router\index.js:328:13)
    at C:\dev\xxxxx\nestjs\node_modules\express\lib\router\index.js:286:9
    at Function.process_params (C:\dev\xxxxx\nestjs\node_modules\express\lib\router\index.js:346:12)     
    at next (C:\dev\xxxxx\nestjs\node_modules\express\lib\router\index.js:280:10)
    at urlencodedParser (C:\dev\xxxxx\nestjs\node_modules\body-parser\lib\types\urlencoded.js:91:7)      
    at Layer.handle [as handle_request] (C:\dev\xxxxx\nestjs\node_modules\express\lib\router\layer.js:95:5)
    at trim_prefix (C:\dev\xxxxx\nestjs\node_modules\express\lib\router\index.js:328:13)
plugins: [
    ApolloServerPluginLandingPageLocalDefault(),
    ApolloServerPluginCacheControl({ defaultMaxAge: 60 }) // seconds
]

@martinvysnovsky
Copy link

I have solved the issue by creating custom exception filter:

@Catch()
export class SentryFilter extends BaseExceptionFilter {
  catch(exception: Error, host: ArgumentsHost) {
    if (host.getType<GqlContextType>() === 'graphql') {
      new ExternalExceptionFilter().catch(exception, host);
    } else {
      super.catch(exception, host);
    }
  }
}

Then you can use this filter when initializing Sentry:

const { httpAdapter } = app.get(HttpAdapterHost);
Sentry.setupNestErrorHandler(app, new SentryFilter(httpAdapter));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: node Issues related to the Sentry Node SDK
Projects
Archived in project
Development

No branches or pull requests

5 participants