From 130ab89ba049bf16c759747d6563b06193f73503 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Fri, 28 Jun 2024 14:46:46 +0200 Subject: [PATCH 1/9] Filter 4xx errors in nest --- packages/node/src/integrations/tracing/nest.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/node/src/integrations/tracing/nest.ts b/packages/node/src/integrations/tracing/nest.ts index bbb658318946..327a25a61fd2 100644 --- a/packages/node/src/integrations/tracing/nest.ts +++ b/packages/node/src/integrations/tracing/nest.ts @@ -100,6 +100,12 @@ export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsE const originalCatch = Reflect.get(target, prop, receiver); return (exception: unknown, host: unknown) => { + const status_code = (exception as { status?: number }).status; + + if (status_code !== undefined && status_code >= 400 && status_code < 500) { // don't report expected errors + return originalCatch.apply(target, [exception, host]); + } + captureException(exception); return originalCatch.apply(target, [exception, host]); }; From 3ac6824b8a3c042705702bdc5b14d66eab19370b Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Fri, 28 Jun 2024 15:30:32 +0200 Subject: [PATCH 2/9] Lint --- packages/node/src/integrations/tracing/nest.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/nest.ts b/packages/node/src/integrations/tracing/nest.ts index 327a25a61fd2..ab6a66fdb895 100644 --- a/packages/node/src/integrations/tracing/nest.ts +++ b/packages/node/src/integrations/tracing/nest.ts @@ -102,7 +102,8 @@ export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsE return (exception: unknown, host: unknown) => { const status_code = (exception as { status?: number }).status; - if (status_code !== undefined && status_code >= 400 && status_code < 500) { // don't report expected errors + // don't report expected errors + if (status_code !== undefined && status_code >= 400 && status_code < 500) { return originalCatch.apply(target, [exception, host]); } From 0a500a20291739f15bb62404155c0defb4458ec7 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 1 Jul 2024 11:18:47 +0200 Subject: [PATCH 3/9] Add baseline for e2e test --- .../nestjs/src/app.controller.ts | 5 +++++ .../nestjs/src/app.service.ts | 6 +++++- .../nestjs/tests/errors.test.ts | 19 +++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs/src/app.controller.ts index 6350cb49f1c5..d79bcb2ecab5 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/src/app.controller.ts @@ -45,6 +45,11 @@ export class AppController1 { return this.appService.testException(id); } + @Get("test-expected-exception/:id") + async testExpectedException(@Param("id") id: string) { + return this.appService.testExpectedException(id); + } + @Get('test-outgoing-fetch-external-allowed') async testOutgoingFetchExternalAllowed() { return this.appService.testOutgoingFetchExternalAllowed(); diff --git a/dev-packages/e2e-tests/test-applications/nestjs/src/app.service.ts b/dev-packages/e2e-tests/test-applications/nestjs/src/app.service.ts index 01a96549546b..1103c65941a1 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/src/app.service.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/src/app.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; import * as Sentry from '@sentry/nestjs'; import { makeHttpRequest } from './utils'; @@ -52,6 +52,10 @@ export class AppService1 { throw new Error(`This is an exception with id ${id}`); } + testExpectedException(id: string) { + throw new HttpException(`This is an expected exception with id ${id}`, HttpStatus.FORBIDDEN); + } + async testOutgoingFetchExternalAllowed() { const fetchResponse = await fetch('http://localhost:3040/external-allowed'); diff --git a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts index aa46f77815d4..a7990256967a 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts @@ -11,6 +11,9 @@ test('Sends exception to Sentry', async ({ baseURL }) => { const errorEvent = await errorEventPromise; + console.log("UNEXPECTED EXCEPTION"); + console.log(errorEvent); + expect(errorEvent.exception?.values).toHaveLength(1); expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123'); @@ -28,3 +31,19 @@ test('Sends exception to Sentry', async ({ baseURL }) => { span_id: expect.any(String), }); }); + +test('Does not send expected exception to Sentry', async ({ baseURL }) => { + const errorEventPromise = waitForError('nestjs', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an unexpected exception with id 123'; + }); + + const response = await fetch(`${baseURL}/test-expected-exception/123`); + expect(response.status).toBe(403); + + const errorEvent = await errorEventPromise; + + console.log("EXPECTED EXCEPTION"); + console.log(errorEvent); + + expect(errorEvent.exception?.values).toHaveLength(1); +}); From 5cc987f6b2e327198a3a5fc981de1810cb487bbc Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 1 Jul 2024 12:04:51 +0200 Subject: [PATCH 4/9] Finish e2e test# --- .../nestjs/tests/errors.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts index a7990256967a..04d4eaa046d4 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts @@ -11,9 +11,6 @@ test('Sends exception to Sentry', async ({ baseURL }) => { const errorEvent = await errorEventPromise; - console.log("UNEXPECTED EXCEPTION"); - console.log(errorEvent); - expect(errorEvent.exception?.values).toHaveLength(1); expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123'); @@ -33,17 +30,20 @@ test('Sends exception to Sentry', async ({ baseURL }) => { }); test('Does not send expected exception to Sentry', async ({ baseURL }) => { + let errorEventOccurred = false; + const errorEventPromise = waitForError('nestjs', event => { - return !event.type && event.exception?.values?.[0]?.value === 'This is an unexpected exception with id 123'; + if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected exception with id 123') { + errorEventOccurred = true; + } + return false; }); const response = await fetch(`${baseURL}/test-expected-exception/123`); expect(response.status).toBe(403); - const errorEvent = await errorEventPromise; + const timeoutPromise = new Promise((resolve) => setTimeout(resolve, 2000)); + await Promise.race([errorEventPromise, timeoutPromise]); - console.log("EXPECTED EXCEPTION"); - console.log(errorEvent); - - expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEventOccurred).toBe(false); }); From 06b9dd478d5e5e8abebe7290960d8770d2731203 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 1 Jul 2024 14:39:40 +0200 Subject: [PATCH 5/9] Change e2e to use transaction end as cutoff --- .../nestjs/tests/errors.test.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts index 04d4eaa046d4..1b4c84ea5f0a 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts @@ -1,5 +1,5 @@ import { expect, test } from '@playwright/test'; -import { waitForError } from '@sentry-internal/test-utils'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; test('Sends exception to Sentry', async ({ baseURL }) => { const errorEventPromise = waitForError('nestjs', event => { @@ -32,18 +32,29 @@ test('Sends exception to Sentry', async ({ baseURL }) => { test('Does not send expected exception to Sentry', async ({ baseURL }) => { let errorEventOccurred = false; - const errorEventPromise = waitForError('nestjs', event => { + waitForError('nestjs', event => { if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected exception with id 123') { errorEventOccurred = true; } return false; }); + const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => { + return ( + transactionEvent?.transaction === 'GET /test-expected-exception/123' + ); + }); + + console.log("fetch"); + const response = await fetch(`${baseURL}/test-expected-exception/123`); expect(response.status).toBe(403); - const timeoutPromise = new Promise((resolve) => setTimeout(resolve, 2000)); - await Promise.race([errorEventPromise, timeoutPromise]); + console.log("fetch done"); + + await transactionEventPromise; + + console.log("resolved promise"); expect(errorEventOccurred).toBe(false); }); From 26ade2f9e855641f6b4fc22c572c64b82c1ffa34 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 1 Jul 2024 14:43:00 +0200 Subject: [PATCH 6/9] Lint --- .../test-applications/nestjs/src/app.controller.ts | 4 ++-- .../test-applications/nestjs/tests/errors.test.ts | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs/src/app.controller.ts b/dev-packages/e2e-tests/test-applications/nestjs/src/app.controller.ts index d79bcb2ecab5..154f62ada912 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/src/app.controller.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/src/app.controller.ts @@ -45,8 +45,8 @@ export class AppController1 { return this.appService.testException(id); } - @Get("test-expected-exception/:id") - async testExpectedException(@Param("id") id: string) { + @Get('test-expected-exception/:id') + async testExpectedException(@Param('id') id: string) { return this.appService.testExpectedException(id); } diff --git a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts index 1b4c84ea5f0a..eb668f5a6d5a 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts @@ -40,21 +40,19 @@ test('Does not send expected exception to Sentry', async ({ baseURL }) => { }); const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => { - return ( - transactionEvent?.transaction === 'GET /test-expected-exception/123' - ); + return transactionEvent?.transaction === 'GET /test-expected-exception/123'; }); - console.log("fetch"); + console.log('fetch'); const response = await fetch(`${baseURL}/test-expected-exception/123`); expect(response.status).toBe(403); - console.log("fetch done"); + console.log('fetch done'); await transactionEventPromise; - console.log("resolved promise"); + console.log('resolved promise'); expect(errorEventOccurred).toBe(false); }); From 8d5560fc05339fa672ec6d7505b31ef7f6031dea Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 1 Jul 2024 15:27:48 +0200 Subject: [PATCH 7/9] Fix e2e --- .../test-applications/nestjs/tests/errors.test.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts index eb668f5a6d5a..c6f20f11a95c 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts @@ -40,19 +40,13 @@ test('Does not send expected exception to Sentry', async ({ baseURL }) => { }); const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => { - return transactionEvent?.transaction === 'GET /test-expected-exception/123'; + return transactionEvent?.transaction === 'GET /test-expected-exception/:id'; }); - console.log('fetch'); - const response = await fetch(`${baseURL}/test-expected-exception/123`); expect(response.status).toBe(403); - console.log('fetch done'); - await transactionEventPromise; - console.log('resolved promise'); - expect(errorEventOccurred).toBe(false); }); From 9e480adc50e8d730c1cf2bbccab17dc6072a5717 Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 1 Jul 2024 16:39:01 +0200 Subject: [PATCH 8/9] Fix e2e --- .../e2e-tests/test-applications/nestjs/tests/errors.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts index c6f20f11a95c..ed31909d28c5 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts @@ -36,7 +36,8 @@ test('Does not send expected exception to Sentry', async ({ baseURL }) => { if (!event.type && event.exception?.values?.[0]?.value === 'This is an expected exception with id 123') { errorEventOccurred = true; } - return false; + + return event?.transaction === 'GET /test-expected-exception/:id'; }); const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => { @@ -48,5 +49,7 @@ test('Does not send expected exception to Sentry', async ({ baseURL }) => { await transactionEventPromise; + await new Promise((resolve) => setTimeout(resolve, 10000)); + expect(errorEventOccurred).toBe(false); }); From a78a01396c8b92d1f881af57fc41870f498c9aac Mon Sep 17 00:00:00 2001 From: nicohrubec Date: Mon, 1 Jul 2024 16:41:04 +0200 Subject: [PATCH 9/9] Lint --- .../e2e-tests/test-applications/nestjs/tests/errors.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts index ed31909d28c5..349b25b0eee9 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs/tests/errors.test.ts @@ -49,7 +49,7 @@ test('Does not send expected exception to Sentry', async ({ baseURL }) => { await transactionEventPromise; - await new Promise((resolve) => setTimeout(resolve, 10000)); + await new Promise(resolve => setTimeout(resolve, 10000)); expect(errorEventOccurred).toBe(false); });