Skip to content

feat(node): Use diagnostics channel for Fastify v5 error handling #16715

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

Merged
merged 4 commits into from
Jun 26, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ const app = fastify();
const port = 3030;
const port2 = 3040;

Sentry.setupFastifyErrorHandler(app);

app.get('/test-success', function (_req, res) {
res.send({ version: 'v1' });
});
Expand Down
38 changes: 30 additions & 8 deletions packages/node/src/integrations/tracing/fastify/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ export const instrumentFastifyV3 = generateInstrumentOnce(INTEGRATION_NAME_V3, (
export const instrumentFastify = generateInstrumentOnce(INTEGRATION_NAME, () => {
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 => {
Expand All @@ -78,8 +80,22 @@ export const instrumentFastify = generateInstrumentOnce(INTEGRATION_NAME, () =>
});
});

// This diagnostics channel only works on Fastify version 5
// For versions 3 and 4, we use `setupFastifyErrorHandler` instead
diagnosticsChannel.subscribe('tracing:fastify.request.handler:error', message => {
const { error, request, reply } = message as {
error: Error;
request: FastifyRequest & { opentelemetry?: () => { span?: Span } };
reply: FastifyReply;
};

if (shouldHandleError(error, request, reply)) {
captureException(error);
}
});

// Returning this as unknown not to deal with the internal types of the FastifyOtelInstrumentation
return fastifyOtelInstrumentationInstance as Instrumentation<InstrumentationConfig>;
return fastifyOtelInstrumentationInstance as Instrumentation<InstrumentationConfig & FastifyHandlerOptions>;
});

const _fastifyIntegration = (() => {
Expand Down Expand Up @@ -143,15 +159,21 @@ function defaultShouldHandleError(_error: Error, _request: FastifyRequest, reply
*/
export function setupFastifyErrorHandler(fastify: FastifyInstance, options?: Partial<FastifyHandlerOptions>): void {
const shouldHandleError = options?.shouldHandleError || defaultShouldHandleError;

const plugin = Object.assign(
function (fastify: FastifyInstance, _options: unknown, done: () => void): void {
fastify.addHook('onError', async (request, reply, error) => {
if (shouldHandleError(error, request, reply)) {
captureException(error);
}
});

if (fastify.version?.startsWith('5.')) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break for > v5!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was intentional, just in case Fastify v6 uses a different diagnostics channel hook for errors when published. It won't break if 6.x is used here, in the worst scenario, the error will be captured twice, and deduped eventually.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather we check if our tracing:fastify.request.handler:error diagnostics channel listener was registered, that feels like a more robust check.

I guess this is hard because others can also register on tracing:fastify.request.handler:error. Perhaps we can do this by setting a field on the integration? Then you can grab the current client fastify integration instance and check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just tried that. Looks like it's a bit tricky because dc.subscribe does not return anything (we don't know if it subscribed or not at instrumentation time. If the hook is not found, it fails silently). We can set a flag in dc hook when an error happens, but the error handler plugin will be registered already, then we can maybe unregister it. Wdyt?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can set a flag in dc hook when an error happens, but the error handler plugin will be registered already, then we can maybe unregister it

yeah lets give this a try

// Fastify 5.x uses diagnostics channel for error handling
DEBUG_BUILD &&
logger.warn(
'Fastify 5.x detected, using diagnostics channel for error handling.\nYou can safely remove `setupFastifyErrorHandler` call.',
);
} else {
fastify.addHook('onError', async (request, reply, error) => {
if (shouldHandleError(error, request, reply)) {
captureException(error);
}
});
}
done();
},
{
Expand Down
Loading