Skip to content

Commit df0bff8

Browse files
authored
test(fastify): Verify if upstream error is fixed and won't regress (#18838)
closes #18418 closes [JS-1260](https://linear.app/getsentry/issue/JS-1260/fastify-5-integration-replystatuscode-is-always-200-in) A test that verifies that the upstream issue got fixed. There was also [one tiny config for playwright](https://github.com/getsentry/sentry-javascript/pull/16845/files#diff-b3f13ce31676390f5b27a03da28c135f8412f211e90735bf6c2cf25c287a202c) which was missed, so the wrong application was running.
1 parent cef02b8 commit df0bff8

File tree

5 files changed

+98
-10
lines changed

5 files changed

+98
-10
lines changed

dev-packages/e2e-tests/test-applications/node-fastify-5/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"dependencies": {
1616
"@sentry/node": "latest || *",
1717
"@types/node": "^18.19.1",
18-
"fastify": "5.3.2",
18+
"fastify": "^5.7.0",
1919
"typescript": "5.6.3",
2020
"ts-node": "10.9.2"
2121
},
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { getPlaywrightConfig } from '@sentry-internal/test-utils';
22

33
const config = getPlaywrightConfig({
4-
startCommand: `pnpm start`,
4+
startCommand: `pnpm start:override`,
55
});
66

77
export default config;

dev-packages/e2e-tests/test-applications/node-fastify-5/src/app-handle-error-override.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ Sentry.setupFastifyErrorHandler(app, {
4848
return false;
4949
}
5050

51+
// @ts-ignore // Fastify V5 is not typed correctly
52+
if (_request.routeOptions?.url?.includes('/test-error-ignored') && _reply.statusCode === 500) {
53+
return false;
54+
}
55+
5156
return true;
5257
},
5358
});
@@ -130,10 +135,30 @@ app.get('/test-outgoing-http-external-disallowed', async function (req, res) {
130135
res.send(data);
131136
});
132137

138+
// Regression test for https://github.com/fastify/fastify/issues/6409
139+
// The error diagnostic channel was always sending 200 unless explicitly changed.
140+
// This was fixed in Fastify 5.7.0
141+
app.register((childApp: F.FastifyInstance, _options: F.FastifyPluginOptions, next: (err?: Error) => void) => {
142+
childApp.setErrorHandler((error: Error, _request: F.FastifyRequest, reply: F.FastifyReply) => {
143+
reply.send({ ok: false });
144+
});
145+
146+
childApp.get('/test-error-ignored', async function () {
147+
throw new Error('This is an error that will not be captured');
148+
});
149+
150+
next();
151+
});
152+
133153
app.post('/test-post', function (req, res) {
134154
res.send({ status: 'ok', body: req.body });
135155
});
136156

157+
app.get('/flush', async function (_req, res) {
158+
await Sentry.flush();
159+
res.send({ ok: true });
160+
});
161+
137162
app.listen({ port: port });
138163

139164
// A second app so we can test header propagation between external URLs

dev-packages/e2e-tests/test-applications/node-fastify-5/src/app.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ Sentry.init({
2525
return false;
2626
}
2727

28+
// @ts-ignore // Fastify V5 is not typed correctly
29+
if (_request.routeOptions?.url?.includes('/test-error-ignored') && _reply.statusCode === 500) {
30+
return false;
31+
}
32+
2833
return true;
2934
},
3035
}),
@@ -90,6 +95,21 @@ app.get('/test-error', async function (req, res) {
9095
res.send({ exceptionId });
9196
});
9297

98+
// Regression test for https://github.com/fastify/fastify/issues/6409
99+
// The error diagnostic channel was always sending 200 unless explicitly changed.
100+
// This was fixed in Fastify 5.7.0
101+
app.register((childApp: F.FastifyInstance, _options: F.FastifyPluginOptions, next: (err?: Error) => void) => {
102+
childApp.setErrorHandler((error: Error, _request: F.FastifyRequest, reply: F.FastifyReply) => {
103+
reply.send({ ok: false });
104+
});
105+
106+
childApp.get('/test-error-ignored', async function () {
107+
throw new Error('This is an error that will not be captured');
108+
});
109+
110+
next();
111+
});
112+
93113
app.get('/test-error-not-captured', async function () {
94114
// This error will not be captured by Sentry
95115
throw new Error('This is an error that will not be captured');
@@ -127,6 +147,11 @@ app.post('/test-post', function (req, res) {
127147
res.send({ status: 'ok', body: req.body });
128148
});
129149

150+
app.get('/flush', async function (_req, res) {
151+
await Sentry.flush();
152+
res.send({ ok: true });
153+
});
154+
130155
app.listen({ port: port });
131156

132157
// A second app so we can test header propagation between external URLs

dev-packages/e2e-tests/test-applications/node-fastify-5/tests/errors.test.ts

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect, test } from '@playwright/test';
2-
import { waitForError } from '@sentry-internal/test-utils';
2+
import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';
33

44
test('Sends correct error event', async ({ baseURL }) => {
55
const errorEventPromise = waitForError('node-fastify-5', event => {
@@ -35,16 +35,54 @@ test('Sends correct error event', async ({ baseURL }) => {
3535
});
3636

3737
test('Does not send error when shouldHandleError returns false', async ({ baseURL }) => {
38-
const errorEventPromise = waitForError('node-fastify-5', event => {
39-
return !event.type && event.exception?.values?.[0]?.value === 'This is an error that will not be captured';
38+
let errorEventOccurred = false;
39+
40+
waitForError('node-fastify-5', event => {
41+
if (!event.type && event.exception?.values?.[0]?.value === 'This is an error that will not be captured') {
42+
errorEventOccurred = true;
43+
}
44+
return event?.transaction === 'GET /test-error-not-captured';
45+
});
46+
47+
const transactionEventPromise = waitForTransaction('node-fastify-5', transactionEvent => {
48+
return transactionEvent?.transaction === 'GET /test-error-not-captured';
49+
});
50+
51+
const response = await fetch(`${baseURL}/test-error-not-captured`);
52+
53+
await transactionEventPromise;
54+
55+
const flushResponse = await fetch(`${baseURL}/flush`);
56+
57+
expect(response.status).toBe(500);
58+
expect(flushResponse.status).toBe(200);
59+
expect(errorEventOccurred).toBe(false);
60+
});
61+
62+
// Regression test for https://github.com/fastify/fastify/issues/6409
63+
// The error diagnostic channel was always sending 200 unless explicitly changed.
64+
// This was fixed in Fastify 5.7.0
65+
test('Error in child plugin with rethrown error handler reports correct 500 status', async ({ baseURL }) => {
66+
let errorEventOccurred = false;
67+
68+
waitForError('node-fastify-5', event => {
69+
if (!event.type && event.exception?.values?.[0]?.value === 'This is an error that will not be captured') {
70+
errorEventOccurred = true;
71+
}
72+
return event?.transaction === 'GET /test-error-ignored';
4073
});
4174

42-
errorEventPromise.then(() => {
43-
throw new Error('This error should not be captured');
75+
const transactionEventPromise = waitForTransaction('node-fastify-5', transactionEvent => {
76+
return transactionEvent?.transaction === 'GET /test-error-ignored';
4477
});
4578

46-
await fetch(`${baseURL}/test-error-not-captured`);
79+
const response = await fetch(`${baseURL}/test-error-ignored`);
80+
81+
await transactionEventPromise;
82+
83+
const flushResponse = await fetch(`${baseURL}/flush`);
4784

48-
// wait for a short time to ensure the error is not captured
49-
await new Promise(resolve => setTimeout(resolve, 1000));
85+
expect(response.status).toBe(500);
86+
expect(flushResponse.status).toBe(200);
87+
expect(errorEventOccurred).toBe(false);
5088
});

0 commit comments

Comments
 (0)