Skip to content

Commit 3f555ca

Browse files
fix(server-utils): Skip Fastify 4xx errors raised before the reply status is set
Fastify runs `onError` hooks before it applies the error's status to the reply, so for errors like an unparseable request body `reply.statusCode` is still the default 200 in `defaultShouldHandleError`, and they were captured. Resolve the status the same way Fastify does when the reply has not been set yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 06dd51e commit 3f555ca

4 files changed

Lines changed: 104 additions & 2 deletions

File tree

‎dev-packages/node-integration-tests/suites/tracing/fastify/scenario.mjs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ app.get('/test-exception/:id', async request => {
2626
throw new Error(`This is an exception with id ${request.params.id}`);
2727
});
2828

29+
app.post('/test-body', async () => {
30+
return {};
31+
});
32+
2933
app.get('/test-inbound-headers/:id', async request => {
3034
return { headers: request.headers, id: request.params.id };
3135
});

‎dev-packages/node-integration-tests/suites/tracing/fastify/test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,5 +182,29 @@ describe('fastify v5 auto-instrumentation', () => {
182182
runner.makeRequest('get', '/test-exception/456', { expectError: true });
183183
await runner.completed();
184184
});
185+
186+
test('does not capture 4xx errors raised before the reply status is set', async () => {
187+
const runner = createRunner()
188+
.expect({
189+
event: {
190+
exception: {
191+
values: [
192+
{
193+
type: 'Error',
194+
value: 'This is an exception with id 789',
195+
},
196+
],
197+
},
198+
},
199+
})
200+
.start();
201+
// Fastify rejects an empty JSON body with a 400 and runs `onError` hooks before setting that status.
202+
await runner.makeRequest('post', '/test-body', {
203+
headers: { 'content-type': 'application/json' },
204+
expectError: true,
205+
});
206+
await runner.makeRequest('get', '/test-exception/789', { expectError: true });
207+
await runner.completed();
208+
});
185209
});
186210
});

‎packages/server-utils/src/integrations/fastify/utils.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,23 @@ export const INTEGRATION_NAME = 'Fastify' as const;
77
*
88
* 3xx and 4xx errors are not sent by default.
99
*/
10-
export function defaultShouldHandleError(_error: Error, _request: FastifyRequest, reply: FastifyReply): boolean {
11-
const statusCode = reply.statusCode;
10+
export function defaultShouldHandleError(error: Error, _request: FastifyRequest, reply: FastifyReply): boolean {
11+
const statusCode = getReplyStatusCode(error, reply);
1212
// 3xx and 4xx errors are not sent by default.
1313
return statusCode >= 500 || statusCode <= 299;
1414
}
15+
16+
/**
17+
* Fastify runs `onError` hooks before it applies the error's status to the reply
18+
* (e.g. for a request body that fails to parse), so `reply.statusCode` can still be
19+
* the default 200. In that case, resolve the status the same way Fastify will.
20+
*/
21+
function getReplyStatusCode(error: Error, reply: FastifyReply): number {
22+
if (reply.statusCode !== 200) {
23+
return reply.statusCode;
24+
}
25+
26+
const { statusCode, status } = error as { statusCode?: unknown; status?: unknown };
27+
const errorStatusCode = Number(statusCode || status);
28+
return errorStatusCode >= 400 ? errorStatusCode : 500;
29+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { describe, expect, it } from 'vitest';
2+
import type { FastifyReply, FastifyRequest } from '../../../src/integrations/fastify/types';
3+
import { defaultShouldHandleError } from '../../../src/integrations/fastify/utils';
4+
5+
const request: FastifyRequest = { method: 'POST', routeOptions: { url: '/' } };
6+
7+
function reply(statusCode: number): FastifyReply {
8+
return { statusCode } as FastifyReply;
9+
}
10+
11+
function errorWith(props: { statusCode?: number; status?: number }): Error {
12+
return Object.assign(new Error('test error'), props);
13+
}
14+
15+
describe('defaultShouldHandleError', () => {
16+
// Fastify runs `onError` hooks before it applies the error's status to the reply,
17+
// e.g. for a body that fails to parse, so `reply.statusCode` is still the default 200 there.
18+
describe('when the reply status has not been set yet', () => {
19+
it('skips errors carrying a 4xx `statusCode`', () => {
20+
expect(defaultShouldHandleError(errorWith({ statusCode: 400 }), request, reply(200))).toBe(false);
21+
});
22+
23+
it('skips errors carrying a 4xx `status`', () => {
24+
expect(defaultShouldHandleError(errorWith({ status: 404 }), request, reply(200))).toBe(false);
25+
});
26+
27+
it('skips errors carrying a 4xx status as a string', () => {
28+
expect(
29+
defaultShouldHandleError(Object.assign(new Error('test error'), { statusCode: '404' }), request, reply(200)),
30+
).toBe(false);
31+
});
32+
33+
it('captures errors carrying a 5xx status', () => {
34+
expect(defaultShouldHandleError(errorWith({ statusCode: 503 }), request, reply(200))).toBe(true);
35+
});
36+
37+
it('captures errors without a status', () => {
38+
expect(defaultShouldHandleError(new Error('test error'), request, reply(200))).toBe(true);
39+
});
40+
41+
it('captures errors carrying a status below 400, which Fastify sends as a 500', () => {
42+
expect(defaultShouldHandleError(errorWith({ statusCode: 302 }), request, reply(200))).toBe(true);
43+
});
44+
});
45+
46+
describe('when the reply status has been set', () => {
47+
it('skips errors on a 4xx reply', () => {
48+
expect(defaultShouldHandleError(new Error('test error'), request, reply(404))).toBe(false);
49+
});
50+
51+
it('skips errors on a 3xx reply', () => {
52+
expect(defaultShouldHandleError(new Error('test error'), request, reply(302))).toBe(false);
53+
});
54+
55+
it('captures errors on a 5xx reply', () => {
56+
expect(defaultShouldHandleError(errorWith({ statusCode: 400 }), request, reply(500))).toBe(true);
57+
});
58+
});
59+
});

0 commit comments

Comments
 (0)