Skip to content

Commit b74f300

Browse files
mydeamydea
authored
fix(node): Fix vercel flushing logic & add test for it (#16208)
As pre-work for #16178, actually add a test for this (kind of). This showed that there was actually a fundamental flaw here, as we looked as the `req` not the `res`, oops.
1 parent 38316fd commit b74f300

File tree

4 files changed

+129
-44
lines changed

4 files changed

+129
-44
lines changed

‎dev-packages/node-integration-tests/suites/vercel/instrument.mjs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
process.env.VERCEL = 'true';
5+
6+
Sentry.init({
7+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
8+
release: '1.0',
9+
tracesSampleRate: 1.0,
10+
transport: loggingTransport,
11+
// We look at debug logs in this test
12+
debug: true,
13+
});

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as Sentry from '@sentry/node';
2+
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
3+
import express from 'express';
4+
5+
const app = express();
6+
7+
app.get('/test/express', (_req, res) => {
8+
res.send({ response: 'response 1' });
9+
});
10+
11+
Sentry.setupExpressErrorHandler(app);
12+
13+
startExpressServerAndSendPortToRunner(app);

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { afterAll, describe, expect } from 'vitest';
2+
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../utils/runner';
3+
4+
describe('vercel xxx', () => {
5+
afterAll(() => {
6+
cleanupChildProcesses();
7+
});
8+
9+
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
10+
test('should flush events correctly on Vercel', async () => {
11+
const runner = createRunner()
12+
.expect({
13+
transaction: {
14+
transaction: 'GET /test/express',
15+
},
16+
})
17+
.start();
18+
runner.makeRequest('get', '/test/express');
19+
await runner.completed();
20+
21+
const actualLogs = runner.getLogs();
22+
23+
// We want to test that the following logs are present in this order
24+
// other logs may be in between
25+
const expectedLogs = [
26+
'Sentry Logger [log]: @sentry/instrumentation-http Patching server.emit',
27+
'Sentry Logger [log]: @sentry/instrumentation-http Handling incoming request',
28+
'Sentry Logger [log]: @sentry/instrumentation-http Patching request.on',
29+
'Sentry Logger [debug]: @opentelemetry_sentry-patched/instrumentation-http http instrumentation incomingRequest',
30+
'Sentry Logger [log]: [Tracing] Starting sampled root span',
31+
// later...
32+
'Sentry Logger [log]: Patching response to flush on Vercel',
33+
'Sentry Logger [log]: Patching res.end()',
34+
// later...
35+
'Sentry Logger [log]: Flushing events before Vercel Lambda freeze',
36+
'Sentry Logger [log]: SpanExporter exported 4 spans, 0 spans are waiting for their parent spans to finish',
37+
];
38+
39+
// Test that the order of logs is correct
40+
for (const log of actualLogs) {
41+
if (expectedLogs.length === 0) {
42+
break;
43+
}
44+
45+
if (log === expectedLogs[0]) {
46+
expectedLogs.shift();
47+
}
48+
}
49+
50+
expect(expectedLogs).toEqual([]);
51+
});
52+
});
53+
});

‎packages/node/src/integrations/http/SentryHttpInstrumentationBeforeOtel.ts‎

Lines changed: 50 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ export class SentryHttpInstrumentationBeforeOtel extends InstrumentationBase {
6868
return original.apply(this, args);
6969
}
7070

71-
const response = args[1] as http.OutgoingMessage;
72-
71+
const response = args[2] as http.OutgoingMessage;
7372
patchResponseToFlushOnServerlessPlatforms(response);
7473

7574
return original.apply(this, args);
@@ -81,50 +80,57 @@ export class SentryHttpInstrumentationBeforeOtel extends InstrumentationBase {
8180
function patchResponseToFlushOnServerlessPlatforms(res: http.OutgoingMessage): void {
8281
// Freely extend this function with other platforms if necessary
8382
if (process.env.VERCEL) {
83+
DEBUG_BUILD && logger.log('Patching response to flush on Vercel');
84+
8485
// In some cases res.end does not seem to be defined leading to errors if passed to Proxy
8586
// https://github.com/getsentry/sentry-javascript/issues/15759
86-
if (typeof res.end === 'function') {
87-
let markOnEndDone = (): void => undefined;
88-
const onEndDonePromise = new Promise<void>(res => {
89-
markOnEndDone = res;
90-
});
91-
92-
res.on('close', () => {
93-
markOnEndDone();
94-
});
95-
96-
// eslint-disable-next-line @typescript-eslint/unbound-method
97-
res.end = new Proxy(res.end, {
98-
apply(target, thisArg, argArray) {
99-
vercelWaitUntil(
100-
new Promise<void>(finishWaitUntil => {
101-
// Define a timeout that unblocks the lambda just to be safe so we're not indefinitely keeping it alive, exploding server bills
102-
const timeout = setTimeout(() => {
103-
finishWaitUntil();
104-
}, 2000);
105-
106-
onEndDonePromise
107-
.then(() => {
108-
DEBUG_BUILD && logger.log('Flushing events before Vercel Lambda freeze');
109-
return flush(2000);
110-
})
111-
.then(
112-
() => {
113-
clearTimeout(timeout);
114-
finishWaitUntil();
115-
},
116-
e => {
117-
clearTimeout(timeout);
118-
DEBUG_BUILD && logger.log('Error while flushing events for Vercel:\n', e);
119-
finishWaitUntil();
120-
},
121-
);
122-
}),
123-
);
124-
125-
return target.apply(thisArg, argArray);
126-
},
127-
});
87+
if (typeof res.end !== 'function') {
88+
DEBUG_BUILD && logger.warn('res.end is not a function, skipping patch...');
89+
return;
12890
}
91+
92+
let markOnEndDone = (): void => undefined;
93+
const onEndDonePromise = new Promise<void>(res => {
94+
markOnEndDone = res;
95+
});
96+
97+
res.on('close', () => {
98+
markOnEndDone();
99+
});
100+
101+
logger.log('Patching res.end()');
102+
103+
// eslint-disable-next-line @typescript-eslint/unbound-method
104+
res.end = new Proxy(res.end, {
105+
apply(target, thisArg, argArray) {
106+
vercelWaitUntil(
107+
new Promise<void>(finishWaitUntil => {
108+
// Define a timeout that unblocks the lambda just to be safe so we're not indefinitely keeping it alive, exploding server bills
109+
const timeout = setTimeout(() => {
110+
finishWaitUntil();
111+
}, 2000);
112+
113+
onEndDonePromise
114+
.then(() => {
115+
DEBUG_BUILD && logger.log('Flushing events before Vercel Lambda freeze');
116+
return flush(2000);
117+
})
118+
.then(
119+
() => {
120+
clearTimeout(timeout);
121+
finishWaitUntil();
122+
},
123+
e => {
124+
clearTimeout(timeout);
125+
DEBUG_BUILD && logger.log('Error while flushing events for Vercel:\n', e);
126+
finishWaitUntil();
127+
},
128+
);
129+
}),
130+
);
131+
132+
return target.apply(thisArg, argArray);
133+
},
134+
});
129135
}
130136
}

0 commit comments

Comments
 (0)