From 85b1b6fc843e24fa95ab9997c5a4d1173bc0a2f2 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 22 Jul 2025 14:25:59 +0100 Subject: [PATCH 1/2] feat(node): Add `shouldHandleError` option to `fastifyIntegration` (#16845) Resolves: https://github.com/getsentry/sentry-javascript/issues/16819 This requires the Fastify integration to be manually added while initialising Sentry. ```typescript Sentry.init({ integrations: [ Sentry.fastifyIntegration({ shouldHandleError(_error, _request, reply) { return reply.statusCode >= 500; }, }); }, }); ``` If `shouldHandleError` is provided in `setupFastifyErrorHandler`, it overrides `shouldHandleError` provided from `fastifyIntegration`. So this should not break any existing (pre-DC) usage. In summary: - When `setupFastifyErrorHandler` is not used, `shouldHandleError` at `Sentry.init` will be used. - When `setupFastifyErrorHandler` is used but without `shouldHandleError`, `shouldHandleError` at `Sentry.init` will be used. - When in both `setupFastifyErrorHandler` and `Sentry.init` are used with `shouldHandleError`, the one in `setupFastifyErrorHandler` is used. Works for all Fastify versions supported (v3, v4, v5) Note: E2E tests for this now requires to test overrides, so they need to be run twice with both overriden and non-overriden applications. Not the prettiest solution, but I could not figure out an alternative. --- .../node-fastify-3/package.json | 4 +- .../playwright.override.config.mjs | 7 + .../src/app-handle-error-override.ts | 178 +++++++++++++++++ .../node-fastify-3/src/app.ts | 19 +- .../node-fastify-3/tests/errors.test.ts | 15 ++ .../node-fastify-4/package.json | 4 +- .../playwright.override.config.mjs | 7 + .../src/app-handle-error-override.ts | 187 ++++++++++++++++++ .../node-fastify-4/src/app.ts | 18 +- .../node-fastify-4/tests/errors.test.ts | 15 ++ .../node-fastify-5/package.json | 4 +- .../playwright.override.config.mjs | 7 + .../src/app-handle-error-override.ts | 178 +++++++++++++++++ .../node-fastify-5/src/app.ts | 18 +- .../node-fastify-5/tests/errors.test.ts | 15 ++ .../src/integrations/tracing/fastify/index.ts | 82 ++++++-- 16 files changed, 741 insertions(+), 17 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-3/playwright.override.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-3/src/app-handle-error-override.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-4/playwright.override.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-4/src/app-handle-error-override.ts create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/playwright.override.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/node-fastify-5/src/app-handle-error-override.ts diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-3/package.json b/dev-packages/e2e-tests/test-applications/node-fastify-3/package.json index 3beb69e26e59..006a33585fd3 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-3/package.json +++ b/dev-packages/e2e-tests/test-applications/node-fastify-3/package.json @@ -4,11 +4,13 @@ "private": true, "scripts": { "start": "ts-node src/app.ts", + "start:override": "ts-node src/app-handle-error-override.ts", "test": "playwright test", + "test:override": "playwright test --config playwright.override.config.mjs", "clean": "npx rimraf node_modules pnpm-lock.yaml", "typecheck": "tsc", "test:build": "pnpm install && pnpm run typecheck", - "test:assert": "pnpm test" + "test:assert": "pnpm test && pnpm test:override" }, "dependencies": { "@sentry/node": "latest || *", diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-3/playwright.override.config.mjs b/dev-packages/e2e-tests/test-applications/node-fastify-3/playwright.override.config.mjs new file mode 100644 index 000000000000..ee1f461c57c8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-3/playwright.override.config.mjs @@ -0,0 +1,7 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: `pnpm start:override`, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-3/src/app-handle-error-override.ts b/dev-packages/e2e-tests/test-applications/node-fastify-3/src/app-handle-error-override.ts new file mode 100644 index 000000000000..378ef99fa309 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-3/src/app-handle-error-override.ts @@ -0,0 +1,178 @@ +import type * as S from '@sentry/node'; +const Sentry = require('@sentry/node') as typeof S; + +// We wrap console.warn to find out if a warning is incorrectly logged +console.warn = new Proxy(console.warn, { + apply: function (target, thisArg, argumentsList) { + const msg = argumentsList[0]; + if (typeof msg === 'string' && msg.startsWith('[Sentry]')) { + console.error(`Sentry warning was triggered: ${msg}`); + process.exit(1); + } + + return target.apply(thisArg, argumentsList); + }, +}); + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.E2E_TEST_DSN, + integrations: [ + Sentry.fastifyIntegration({ + shouldHandleError: (error, _request, _reply) => { + return true; + }, + }), + ], + tracesSampleRate: 1, + tunnel: 'http://localhost:3031/', // proxy server + tracePropagationTargets: ['http://localhost:3030', '/external-allowed'], +}); + +import type * as H from 'http'; +import type * as F from 'fastify'; + +// Make sure fastify is imported after Sentry is initialized +const { fastify } = require('fastify') as typeof F; +const http = require('http') as typeof H; + +const app = fastify(); +const port = 3030; +const port2 = 3040; + +Sentry.setupFastifyErrorHandler(app, { + shouldHandleError: (error, _request, _reply) => { + // @ts-ignore // Fastify V3 is not typed correctly + if (_request.url?.includes('/test-error-not-captured')) { + // Errors from this path will not be captured by Sentry + return false; + } + + return true; + }, +}); + +app.get('/test-success', function (_req, res) { + res.send({ version: 'v1' }); +}); + +app.get<{ Params: { param: string } }>('/test-param/:param', function (req, res) { + res.send({ paramWas: req.params.param }); +}); + +app.get<{ Params: { id: string } }>('/test-inbound-headers/:id', function (req, res) { + const headers = req.headers; + + res.send({ headers, id: req.params.id }); +}); + +app.get<{ Params: { id: string } }>('/test-outgoing-http/:id', async function (req, res) { + const id = req.params.id; + const data = await makeHttpRequest(`http://localhost:3030/test-inbound-headers/${id}`); + + res.send(data); +}); + +app.get<{ Params: { id: string } }>('/test-outgoing-fetch/:id', async function (req, res) { + const id = req.params.id; + const response = await fetch(`http://localhost:3030/test-inbound-headers/${id}`); + const data = await response.json(); + + res.send(data); +}); + +app.get('/test-transaction', async function (req, res) { + Sentry.startSpan({ name: 'test-span' }, () => { + Sentry.startSpan({ name: 'child-span' }, () => {}); + }); + + res.send({}); +}); + +app.get('/test-error', async function (req, res) { + const exceptionId = Sentry.captureException(new Error('This is an error')); + + await Sentry.flush(2000); + + res.send({ exceptionId }); +}); + +app.get('/test-error-not-captured', async function () { + // This error will not be captured by Sentry + throw new Error('This is an error that will not be captured'); +}); + +app.get<{ Params: { id: string } }>('/test-exception/:id', async function (req, res) { + throw new Error(`This is an exception with id ${req.params.id}`); +}); + +app.get('/test-outgoing-fetch-external-allowed', async function (req, res) { + const fetchResponse = await fetch(`http://localhost:${port2}/external-allowed`); + const data = await fetchResponse.json(); + + res.send(data); +}); + +app.get('/test-outgoing-fetch-external-disallowed', async function (req, res) { + const fetchResponse = await fetch(`http://localhost:${port2}/external-disallowed`); + const data = await fetchResponse.json(); + + res.send(data); +}); + +app.get('/test-outgoing-http-external-allowed', async function (req, res) { + const data = await makeHttpRequest(`http://localhost:${port2}/external-allowed`); + res.send(data); +}); + +app.get('/test-outgoing-http-external-disallowed', async function (req, res) { + const data = await makeHttpRequest(`http://localhost:${port2}/external-disallowed`); + res.send(data); +}); + +app.post('/test-post', function (req, res) { + res.send({ status: 'ok', body: req.body }); +}); + +app.listen({ port: port }); + +// A second app so we can test header propagation between external URLs +const app2 = fastify(); +app2.get('/external-allowed', function (req, res) { + const headers = req.headers; + + res.send({ headers, route: '/external-allowed' }); +}); + +app2.get('/external-disallowed', function (req, res) { + const headers = req.headers; + + res.send({ headers, route: '/external-disallowed' }); +}); + +app2.listen({ port: port2 }); + +function makeHttpRequest(url: string) { + return new Promise(resolve => { + const data: any[] = []; + + http + .request(url, httpRes => { + httpRes.on('data', chunk => { + data.push(chunk); + }); + httpRes.on('error', error => { + resolve({ error: error.message, url }); + }); + httpRes.on('end', () => { + try { + const json = JSON.parse(Buffer.concat(data).toString()); + resolve(json); + } catch { + resolve({ data: Buffer.concat(data).toString(), url }); + } + }); + }) + .end(); + }); +} diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-3/src/app.ts b/dev-packages/e2e-tests/test-applications/node-fastify-3/src/app.ts index 73ffafcfd04d..5b4a2f0d16ac 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-3/src/app.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify-3/src/app.ts @@ -17,7 +17,19 @@ console.warn = new Proxy(console.warn, { Sentry.init({ environment: 'qa', // dynamic sampling bias to keep transactions dsn: process.env.E2E_TEST_DSN, - integrations: [], + integrations: [ + Sentry.fastifyIntegration({ + shouldHandleError: (error, _request, _reply) => { + // @ts-ignore // Fastify V3 is not typed correctly + if (_request.url?.includes('/test-error-not-captured')) { + // Errors from this path will not be captured by Sentry + return false; + } + + return true; + }, + }), + ], tracesSampleRate: 1, tunnel: 'http://localhost:3031/', // proxy server tracePropagationTargets: ['http://localhost:3030', '/external-allowed'], @@ -81,6 +93,11 @@ app.get('/test-error', async function (req, res) { res.send({ exceptionId }); }); +app.get('/test-error-not-captured', async function () { + // This error will not be captured by Sentry + throw new Error('This is an error that will not be captured'); +}); + app.get<{ Params: { id: string } }>('/test-exception/:id', async function (req, res) { throw new Error(`This is an exception with id ${req.params.id}`); }); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-3/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify-3/tests/errors.test.ts index 1a37fc244413..c0be1b0292a3 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-3/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify-3/tests/errors.test.ts @@ -28,3 +28,18 @@ test('Sends correct error event', async ({ baseURL }) => { parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), }); }); + +test('Does not send error when shouldHandleError returns false', async ({ baseURL }) => { + const errorEventPromise = waitForError('node-fastify-3', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an error that will not be captured'; + }); + + errorEventPromise.then(() => { + throw new Error('This error should not be captured'); + }); + + await fetch(`${baseURL}/test-error-not-captured`); + + // wait for a short time to ensure the error is not captured + await new Promise(resolve => setTimeout(resolve, 1000)); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-4/package.json b/dev-packages/e2e-tests/test-applications/node-fastify-4/package.json index 7441e4335fb4..01f4879d5a68 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-4/package.json +++ b/dev-packages/e2e-tests/test-applications/node-fastify-4/package.json @@ -4,11 +4,13 @@ "private": true, "scripts": { "start": "ts-node src/app.ts", + "start:override": "ts-node src/app-handle-error-override.ts", "test": "playwright test", + "test:override": "playwright test --config playwright.override.config.mjs", "clean": "npx rimraf node_modules pnpm-lock.yaml", "typecheck": "tsc", "test:build": "pnpm install && pnpm run typecheck", - "test:assert": "pnpm test" + "test:assert": "pnpm test && pnpm test:override" }, "dependencies": { "@sentry/node": "latest || *", diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-4/playwright.override.config.mjs b/dev-packages/e2e-tests/test-applications/node-fastify-4/playwright.override.config.mjs new file mode 100644 index 000000000000..ee1f461c57c8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-4/playwright.override.config.mjs @@ -0,0 +1,7 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: `pnpm start:override`, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-4/src/app-handle-error-override.ts b/dev-packages/e2e-tests/test-applications/node-fastify-4/src/app-handle-error-override.ts new file mode 100644 index 000000000000..72270efc05de --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-4/src/app-handle-error-override.ts @@ -0,0 +1,187 @@ +import type * as S from '@sentry/node'; +const Sentry = require('@sentry/node') as typeof S; + +// We wrap console.warn to find out if a warning is incorrectly logged +console.warn = new Proxy(console.warn, { + apply: function (target, thisArg, argumentsList) { + const msg = argumentsList[0]; + if (typeof msg === 'string' && msg.startsWith('[Sentry]')) { + console.error(`Sentry warning was triggered: ${msg}`); + process.exit(1); + } + + return target.apply(thisArg, argumentsList); + }, +}); + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.E2E_TEST_DSN, + integrations: [ + Sentry.fastifyIntegration({ + shouldHandleError: (error, _request, _reply) => { + return true; + }, + }), + ], + tracesSampleRate: 1, + tunnel: 'http://localhost:3031/', // proxy server + tracePropagationTargets: ['http://localhost:3030', '/external-allowed'], +}); + +import type * as H from 'http'; +import type * as F from 'fastify'; + +// Make sure fastify is imported after Sentry is initialized +const { fastify } = require('fastify') as typeof F; +const http = require('http') as typeof H; + +const app = fastify(); +const port = 3030; +const port2 = 3040; + +Sentry.setupFastifyErrorHandler(app, { + shouldHandleError: (error, _request, _reply) => { + if (_request.routeOptions?.url?.includes('/test-error-not-captured')) { + // Errors from this path will not be captured by Sentry + return false; + } + + return true; + }, +}); + +app.get('/test-success', function (_req, res) { + res.send({ version: 'v1' }); +}); + +app.get<{ Params: { param: string } }>('/test-param/:param', function (req, res) { + res.send({ paramWas: req.params.param }); +}); + +app.get<{ Params: { id: string } }>('/test-inbound-headers/:id', function (req, res) { + const headers = req.headers; + + res.send({ headers, id: req.params.id }); +}); + +app.get<{ Params: { id: string } }>('/test-outgoing-http/:id', async function (req, res) { + const id = req.params.id; + const data = await makeHttpRequest(`http://localhost:3030/test-inbound-headers/${id}`); + + res.send(data); +}); + +app.get<{ Params: { id: string } }>('/test-outgoing-fetch/:id', async function (req, res) { + const id = req.params.id; + const response = await fetch(`http://localhost:3030/test-inbound-headers/${id}`); + const data = await response.json(); + + res.send(data); +}); + +app.get('/test-transaction', async function (req, res) { + Sentry.startSpan({ name: 'test-span' }, () => { + Sentry.startSpan({ name: 'child-span' }, () => {}); + }); + + res.send({}); +}); + +app.get('/test-error', async function (req, res) { + const exceptionId = Sentry.captureException(new Error('This is an error')); + + await Sentry.flush(2000); + + res.send({ exceptionId }); +}); + +app.get('/test-error-not-captured', async function () { + // This error will not be captured by Sentry + throw new Error('This is an error that will not be captured'); +}); + +app.get('/test-4xx-error', async function (req, res) { + res.code(400); + throw new Error('This is a 4xx error'); +}); + +app.get('/test-5xx-error', async function (req, res) { + res.code(500); + throw new Error('This is a 5xx error'); +}); + +app.get<{ Params: { id: string } }>('/test-exception/:id', async function (req, res) { + throw new Error(`This is an exception with id ${req.params.id}`); +}); + +app.get('/test-outgoing-fetch-external-allowed', async function (req, res) { + const fetchResponse = await fetch(`http://localhost:${port2}/external-allowed`); + const data = await fetchResponse.json(); + + res.send(data); +}); + +app.get('/test-outgoing-fetch-external-disallowed', async function (req, res) { + const fetchResponse = await fetch(`http://localhost:${port2}/external-disallowed`); + const data = await fetchResponse.json(); + + res.send(data); +}); + +app.get('/test-outgoing-http-external-allowed', async function (req, res) { + const data = await makeHttpRequest(`http://localhost:${port2}/external-allowed`); + res.send(data); +}); + +app.get('/test-outgoing-http-external-disallowed', async function (req, res) { + const data = await makeHttpRequest(`http://localhost:${port2}/external-disallowed`); + res.send(data); +}); + +app.post('/test-post', function (req, res) { + res.send({ status: 'ok', body: req.body }); +}); + +app.listen({ port: port }); + +// A second app so we can test header propagation between external URLs +const app2 = fastify(); +app2.get('/external-allowed', function (req, res) { + const headers = req.headers; + + res.send({ headers, route: '/external-allowed' }); +}); + +app2.get('/external-disallowed', function (req, res) { + const headers = req.headers; + + res.send({ headers, route: '/external-disallowed' }); +}); + +app2.listen({ port: port2 }); + +function makeHttpRequest(url: string) { + return new Promise(resolve => { + const data: any[] = []; + + http + .request(url, httpRes => { + httpRes.on('data', chunk => { + data.push(chunk); + }); + httpRes.on('error', error => { + resolve({ error: error.message, url }); + }); + httpRes.on('end', () => { + try { + const json = JSON.parse(Buffer.concat(data).toString()); + resolve(json); + } catch { + resolve({ data: Buffer.concat(data).toString(), url }); + } + }); + }) + .end(); + }); +} diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-4/src/app.ts b/dev-packages/e2e-tests/test-applications/node-fastify-4/src/app.ts index 7f7ac390b4b3..1c428c0486f9 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-4/src/app.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify-4/src/app.ts @@ -17,7 +17,18 @@ console.warn = new Proxy(console.warn, { Sentry.init({ environment: 'qa', // dynamic sampling bias to keep transactions dsn: process.env.E2E_TEST_DSN, - integrations: [], + integrations: [ + Sentry.fastifyIntegration({ + shouldHandleError: (error, _request, _reply) => { + if (_request.routeOptions?.url?.includes('/test-error-not-captured')) { + // Errors from this path will not be captured by Sentry + return false; + } + + return true; + }, + }), + ], tracesSampleRate: 1, tunnel: 'http://localhost:3031/', // proxy server tracePropagationTargets: ['http://localhost:3030', '/external-allowed'], @@ -81,6 +92,11 @@ app.get('/test-error', async function (req, res) { res.send({ exceptionId }); }); +app.get('/test-error-not-captured', async function () { + // This error will not be captured by Sentry + throw new Error('This is an error that will not be captured'); +}); + app.get('/test-4xx-error', async function (req, res) { res.code(400); throw new Error('This is a 4xx error'); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-4/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify-4/tests/errors.test.ts index 8ecdc8975778..46453e4749e0 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-4/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify-4/tests/errors.test.ts @@ -50,3 +50,18 @@ test('Does not send 4xx errors by default', async ({ baseURL }) => { const errorEvent = await serverErrorPromise; expect(errorEvent.exception?.values?.[0]?.value).toContain('This is a 5xx error'); }); + +test('Does not send error when shouldHandleError returns false', async ({ baseURL }) => { + const errorEventPromise = waitForError('node-fastify-4', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an error that will not be captured'; + }); + + errorEventPromise.then(() => { + throw new Error('This error should not be captured'); + }); + + await fetch(`${baseURL}/test-error-not-captured`); + + // wait for a short time to ensure the error is not captured + await new Promise(resolve => setTimeout(resolve, 1000)); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json b/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json index fba10d4d2a90..0b03a26eca47 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json @@ -4,11 +4,13 @@ "private": true, "scripts": { "start": "ts-node src/app.ts", + "start:override": "ts-node src/app-handle-error-override.ts", "test": "playwright test", + "test:override": "playwright test --config playwright.override.config.mjs", "clean": "npx rimraf node_modules pnpm-lock.yaml", "typecheck": "tsc", "test:build": "pnpm install && pnpm run typecheck", - "test:assert": "pnpm test" + "test:assert": "pnpm test && pnpm test:override" }, "dependencies": { "@sentry/node": "latest || *", diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/playwright.override.config.mjs b/dev-packages/e2e-tests/test-applications/node-fastify-5/playwright.override.config.mjs new file mode 100644 index 000000000000..31f2b913b58b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/playwright.override.config.mjs @@ -0,0 +1,7 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: `pnpm start`, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app-handle-error-override.ts b/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app-handle-error-override.ts new file mode 100644 index 000000000000..217201332e17 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app-handle-error-override.ts @@ -0,0 +1,178 @@ +import type * as S from '@sentry/node'; +const Sentry = require('@sentry/node') as typeof S; + +// We wrap console.warn to find out if a warning is incorrectly logged +console.warn = new Proxy(console.warn, { + apply: function (target, thisArg, argumentsList) { + const msg = argumentsList[0]; + if (typeof msg === 'string' && msg.startsWith('[Sentry]')) { + console.error(`Sentry warning was triggered: ${msg}`); + process.exit(1); + } + + return target.apply(thisArg, argumentsList); + }, +}); + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.E2E_TEST_DSN, + integrations: [ + Sentry.fastifyIntegration({ + shouldHandleError: (error, _request, _reply) => { + return true; + }, + }), + ], + tracesSampleRate: 1, + tunnel: 'http://localhost:3031/', // proxy server + tracePropagationTargets: ['http://localhost:3030', '/external-allowed'], +}); + +import type * as H from 'http'; +import type * as F from 'fastify'; + +// Make sure fastify is imported after Sentry is initialized +const { fastify } = require('fastify') as typeof F; +const http = require('http') as typeof H; + +const app = fastify(); +const port = 3030; +const port2 = 3040; + +Sentry.setupFastifyErrorHandler(app, { + shouldHandleError: (error, _request, _reply) => { + // @ts-ignore // Fastify V5 is not typed correctly + if (_request.routeOptions?.url?.includes('/test-error-not-captured')) { + // Errors from this path will not be captured by Sentry + return false; + } + + return true; + }, +}); + +app.get('/test-success', function (_req, res) { + res.send({ version: 'v1' }); +}); + +app.get<{ Params: { param: string } }>('/test-param/:param', function (req, res) { + res.send({ paramWas: req.params.param }); +}); + +app.get<{ Params: { id: string } }>('/test-inbound-headers/:id', function (req, res) { + const headers = req.headers; + + res.send({ headers, id: req.params.id }); +}); + +app.get<{ Params: { id: string } }>('/test-outgoing-http/:id', async function (req, res) { + const id = req.params.id; + const data = await makeHttpRequest(`http://localhost:3030/test-inbound-headers/${id}`); + + res.send(data); +}); + +app.get<{ Params: { id: string } }>('/test-outgoing-fetch/:id', async function (req, res) { + const id = req.params.id; + const response = await fetch(`http://localhost:3030/test-inbound-headers/${id}`); + const data = await response.json(); + + res.send(data); +}); + +app.get('/test-transaction', async function (req, res) { + Sentry.startSpan({ name: 'test-span' }, () => { + Sentry.startSpan({ name: 'child-span' }, () => {}); + }); + + res.send({}); +}); + +app.get('/test-error', async function (req, res) { + const exceptionId = Sentry.captureException(new Error('This is an error')); + + await Sentry.flush(2000); + + res.send({ exceptionId }); +}); + +app.get('/test-error-not-captured', async function () { + // This error will not be captured by Sentry + throw new Error('This is an error that will not be captured'); +}); + +app.get<{ Params: { id: string } }>('/test-exception/:id', async function (req, res) { + throw new Error(`This is an exception with id ${req.params.id}`); +}); + +app.get('/test-outgoing-fetch-external-allowed', async function (req, res) { + const fetchResponse = await fetch(`http://localhost:${port2}/external-allowed`); + const data = await fetchResponse.json(); + + res.send(data); +}); + +app.get('/test-outgoing-fetch-external-disallowed', async function (req, res) { + const fetchResponse = await fetch(`http://localhost:${port2}/external-disallowed`); + const data = await fetchResponse.json(); + + res.send(data); +}); + +app.get('/test-outgoing-http-external-allowed', async function (req, res) { + const data = await makeHttpRequest(`http://localhost:${port2}/external-allowed`); + res.send(data); +}); + +app.get('/test-outgoing-http-external-disallowed', async function (req, res) { + const data = await makeHttpRequest(`http://localhost:${port2}/external-disallowed`); + res.send(data); +}); + +app.post('/test-post', function (req, res) { + res.send({ status: 'ok', body: req.body }); +}); + +app.listen({ port: port }); + +// A second app so we can test header propagation between external URLs +const app2 = fastify(); +app2.get('/external-allowed', function (req, res) { + const headers = req.headers; + + res.send({ headers, route: '/external-allowed' }); +}); + +app2.get('/external-disallowed', function (req, res) { + const headers = req.headers; + + res.send({ headers, route: '/external-disallowed' }); +}); + +app2.listen({ port: port2 }); + +function makeHttpRequest(url: string) { + return new Promise(resolve => { + const data: any[] = []; + + http + .request(url, httpRes => { + httpRes.on('data', chunk => { + data.push(chunk); + }); + httpRes.on('error', error => { + resolve({ error: error.message, url }); + }); + httpRes.on('end', () => { + try { + const json = JSON.parse(Buffer.concat(data).toString()); + resolve(json); + } catch { + resolve({ data: Buffer.concat(data).toString(), url }); + } + }); + }) + .end(); + }); +} diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app.ts b/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app.ts index db2e9bf9cc5f..83f7e53a45ce 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/src/app.ts @@ -17,7 +17,18 @@ console.warn = new Proxy(console.warn, { Sentry.init({ environment: 'qa', // dynamic sampling bias to keep transactions dsn: process.env.E2E_TEST_DSN, - integrations: [], + integrations: [ + Sentry.fastifyIntegration({ + shouldHandleError: (error, _request, _reply) => { + if (_request.routeOptions?.url?.includes('/test-error-not-captured')) { + // Errors from this path will not be captured by Sentry + return false; + } + + return true; + }, + }), + ], tracesSampleRate: 1, tunnel: 'http://localhost:3031/', // proxy server tracePropagationTargets: ['http://localhost:3030', '/external-allowed'], @@ -79,6 +90,11 @@ app.get('/test-error', async function (req, res) { res.send({ exceptionId }); }); +app.get('/test-error-not-captured', async function () { + // This error will not be captured by Sentry + throw new Error('This is an error that will not be captured'); +}); + app.get<{ Params: { id: string } }>('/test-exception/:id', async function (req, res) { throw new Error(`This is an exception with id ${req.params.id}`); }); diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts index f79eb30e9b4c..cf1428b7a34a 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts @@ -28,3 +28,18 @@ test('Sends correct error event', async ({ baseURL }) => { parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), }); }); + +test('Does not send error when shouldHandleError returns false', async ({ baseURL }) => { + const errorEventPromise = waitForError('node-fastify-5', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an error that will not be captured'; + }); + + errorEventPromise.then(() => { + throw new Error('This error should not be captured'); + }); + + await fetch(`${baseURL}/test-error-not-captured`); + + // wait for a short time to ensure the error is not captured + await new Promise(resolve => setTimeout(resolve, 1000)); +}); diff --git a/packages/node/src/integrations/tracing/fastify/index.ts b/packages/node/src/integrations/tracing/fastify/index.ts index 0aaf7814e9e3..bb8333e73cf0 100644 --- a/packages/node/src/integrations/tracing/fastify/index.ts +++ b/packages/node/src/integrations/tracing/fastify/index.ts @@ -17,6 +17,41 @@ import { FastifyOtelInstrumentation } from './fastify-otel/index'; import type { FastifyInstance, FastifyReply, FastifyRequest } from './types'; import { FastifyInstrumentationV3 } from './v3/instrumentation'; +/** + * Options for the Fastify integration. + * + * `shouldHandleError` - Callback method deciding whether error should be captured and sent to Sentry + * This is used on Fastify v5 where Sentry handles errors in the diagnostics channel. + * Fastify v3 and v4 use `setupFastifyErrorHandler` instead. + * + * @example + * + * ```javascript + * Sentry.init({ + * integrations: [ + * Sentry.fastifyIntegration({ + * shouldHandleError(_error, _request, reply) { + * return reply.statusCode >= 500; + * }, + * }); + * }, + * }); + * ``` + * + */ +interface FastifyIntegrationOptions { + /** + * Callback method deciding whether error should be captured and sent to Sentry + * This is used on Fastify v5 where Sentry handles errors in the diagnostics channel. + * Fastify v3 and v4 use `setupFastifyErrorHandler` instead. + * + * @param error Captured Fastify error + * @param request Fastify request (or any object containing at least method, routeOptions.url, and routerPath) + * @param reply Fastify reply (or any object containing at least statusCode) + */ + shouldHandleError: (error: Error, request: FastifyRequest, reply: FastifyReply) => boolean; +} + interface FastifyHandlerOptions { /** * Callback method deciding whether error should be captured and sent to Sentry @@ -27,6 +62,7 @@ interface FastifyHandlerOptions { * * @example * + * * ```javascript * setupFastifyErrorHandler(app, { * shouldHandleError(_error, _request, reply) { @@ -35,6 +71,7 @@ interface FastifyHandlerOptions { * }); * ``` * + * * If using TypeScript, you can cast the request and reply to get full type safety. * * ```typescript @@ -53,10 +90,20 @@ interface FastifyHandlerOptions { } const INTEGRATION_NAME = 'Fastify'; +const INTEGRATION_NAME_V5 = 'Fastify-V5'; const INTEGRATION_NAME_V3 = 'Fastify-V3'; export const instrumentFastifyV3 = generateInstrumentOnce(INTEGRATION_NAME_V3, () => new FastifyInstrumentationV3()); +function getFastifyIntegration(): ReturnType | undefined { + const client = getClient(); + if (!client) { + return undefined; + } else { + return client.getIntegrationByName(INTEGRATION_NAME) as ReturnType | undefined; + } +} + function handleFastifyError( this: { diagnosticsChannelExists?: boolean; @@ -64,9 +111,9 @@ function handleFastifyError( error: Error, request: FastifyRequest & { opentelemetry?: () => { span?: Span } }, reply: FastifyReply, - shouldHandleError: (error: Error, request: FastifyRequest, reply: FastifyReply) => boolean, handlerOrigin: 'diagnostics-channel' | 'onError-hook', ): void { + const shouldHandleError = getFastifyIntegration()?.getShouldHandleError() || defaultShouldHandleError; // Diagnostics channel runs before the onError hook, so we can use it to check if the handler was already registered if (handlerOrigin === 'diagnostics-channel') { this.diagnosticsChannelExists = true; @@ -76,7 +123,7 @@ function handleFastifyError( DEBUG_BUILD && debug.warn( 'Fastify error handler was already registered via diagnostics channel.', - 'You can safely remove `setupFastifyErrorHandler` call.', + 'You can safely remove `setupFastifyErrorHandler` call and set `shouldHandleError` on the integration options.', ); // If the diagnostics channel already exists, we don't need to handle the error again @@ -88,11 +135,9 @@ function handleFastifyError( } } -export const instrumentFastify = generateInstrumentOnce(INTEGRATION_NAME, () => { +export const instrumentFastify = generateInstrumentOnce(INTEGRATION_NAME_V5, () => { const fastifyOtelInstrumentationInstance = new FastifyOtelInstrumentation(); const plugin = fastifyOtelInstrumentationInstance.plugin(); - const options = fastifyOtelInstrumentationInstance.getConfig(); - const shouldHandleError = (options as FastifyHandlerOptions)?.shouldHandleError || defaultShouldHandleError; // This message handler works for Fastify versions 3, 4 and 5 diagnosticsChannel.subscribe('fastify.initialization', message => { @@ -120,20 +165,30 @@ export const instrumentFastify = generateInstrumentOnce(INTEGRATION_NAME, () => reply: FastifyReply; }; - handleFastifyError.call(handleFastifyError, error, request, reply, shouldHandleError, 'diagnostics-channel'); + handleFastifyError.call(handleFastifyError, error, request, reply, 'diagnostics-channel'); }); // Returning this as unknown not to deal with the internal types of the FastifyOtelInstrumentation - return fastifyOtelInstrumentationInstance as Instrumentation; + return fastifyOtelInstrumentationInstance as Instrumentation; }); -const _fastifyIntegration = (() => { +const _fastifyIntegration = (({ shouldHandleError }: Partial) => { + let _shouldHandleError: (error: Error, request: FastifyRequest, reply: FastifyReply) => boolean; + return { name: INTEGRATION_NAME, setupOnce() { + _shouldHandleError = shouldHandleError || defaultShouldHandleError; + instrumentFastifyV3(); instrumentFastify(); }, + getShouldHandleError() { + return _shouldHandleError; + }, + setShouldHandleError(fn: (error: Error, request: FastifyRequest, reply: FastifyReply) => boolean): void { + _shouldHandleError = fn; + }, }; }) satisfies IntegrationFn; @@ -153,7 +208,9 @@ const _fastifyIntegration = (() => { * }) * ``` */ -export const fastifyIntegration = defineIntegration(_fastifyIntegration); +export const fastifyIntegration = defineIntegration((options: Partial = {}) => + _fastifyIntegration(options), +); /** * Default function to determine if an error should be sent to Sentry @@ -187,11 +244,14 @@ function defaultShouldHandleError(_error: Error, _request: FastifyRequest, reply * ``` */ export function setupFastifyErrorHandler(fastify: FastifyInstance, options?: Partial): void { - const shouldHandleError = options?.shouldHandleError || defaultShouldHandleError; + if (options?.shouldHandleError) { + getFastifyIntegration()?.setShouldHandleError(options.shouldHandleError); + } + const plugin = Object.assign( function (fastify: FastifyInstance, _options: unknown, done: () => void): void { fastify.addHook('onError', async (request, reply, error) => { - handleFastifyError.call(handleFastifyError, error, request, reply, shouldHandleError, 'onError-hook'); + handleFastifyError.call(handleFastifyError, error, request, reply, 'onError-hook'); }); done(); }, From c6deb74b219c17bf021f7a2376f87e09944c48b7 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 23 Jul 2025 10:30:54 +0200 Subject: [PATCH 2/2] start ci please?