Skip to content

Commit 54e6cae

Browse files
committed
fix: use onFinished replace patch res.end, close getsentry#8848
1 parent 618e992 commit 54e6cae

File tree

4 files changed

+32
-39
lines changed

4 files changed

+32
-39
lines changed

packages/node/src/handlers.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -172,20 +172,6 @@ export function requestHandler(
172172
res: http.ServerResponse,
173173
next: (error?: any) => void,
174174
): void {
175-
if (options && options.flushTimeout && options.flushTimeout > 0) {
176-
// eslint-disable-next-line @typescript-eslint/unbound-method
177-
const _end = res.end;
178-
res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): void {
179-
void flush(options.flushTimeout)
180-
.then(() => {
181-
_end.call(this, chunk, encoding, cb);
182-
})
183-
.then(null, e => {
184-
__DEBUG_BUILD__ && logger.error(e);
185-
_end.call(this, chunk, encoding, cb);
186-
});
187-
};
188-
}
189175
runWithAsyncContext(() => {
190176
const currentHub = getCurrentHub();
191177
currentHub.configureScope(scope => {
@@ -216,6 +202,12 @@ export function requestHandler(
216202
}
217203
});
218204
}
205+
206+
if (options && options.flushTimeout && options.flushTimeout > 0) {
207+
void flush(options.flushTimeout).then(null, e => {
208+
__DEBUG_BUILD__ && logger.error(e);
209+
});
210+
}
219211
});
220212
next();
221213
});

packages/node/test/handlers.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ describe('requestHandler', () => {
132132
const sentryRequestMiddleware = requestHandler({ flushTimeout: 1337 });
133133
sentryRequestMiddleware(req, res, next);
134134
res.end('ok');
135+
res.emit('finish');
135136

136137
setImmediate(() => {
137138
expect(flush).toHaveBeenCalledWith(1337);
@@ -146,6 +147,7 @@ describe('requestHandler', () => {
146147
const sentryRequestMiddleware = requestHandler({ flushTimeout: 1337 });
147148
sentryRequestMiddleware(req, res, next);
148149
res.end('ok');
150+
res.emit('finish');
149151

150152
setImmediate(() => {
151153
expect(res.finished).toBe(true);

packages/serverless/src/gcpfunction/http.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -102,21 +102,13 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
102102
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
103103
(res as any).__sentry_transaction = transaction;
104104

105-
// eslint-disable-next-line @typescript-eslint/unbound-method
106-
const _end = res.end;
107-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
108-
res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): any {
105+
res.once('finish', (): void => {
109106
transaction?.setHttpStatus(res.statusCode);
110107
transaction?.finish();
111-
112-
void flush(options.flushTimeout)
113-
.then(null, e => {
114-
__DEBUG_BUILD__ && logger.error(e);
115-
})
116-
.then(() => {
117-
_end.call(this, chunk, encoding, cb);
118-
});
119-
};
108+
void flush(options.flushTimeout).then(null, (e: unknown) => {
109+
__DEBUG_BUILD__ && logger.error(e);
110+
});
111+
});
120112

121113
let fnResult;
122114
try {

packages/serverless/test/gcpfunction.test.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as SentryNode from '@sentry/node';
22
import type { Event } from '@sentry/types';
33
import * as domain from 'domain';
4+
import * as http from 'http';
45

56
import * as Sentry from '../src';
67
import { wrapCloudEventFunction, wrapEventFunction, wrapHttpFunction } from '../src/gcpfunction';
@@ -39,9 +40,23 @@ describe('GCPFunction', () => {
3940
headers: headers,
4041
body: { foo: 'bar' },
4142
} as Request;
42-
const res = { end: resolve } as Response;
43-
d.on('error', () => res.end());
44-
d.run(() => process.nextTick(fn, req, res));
43+
const res = new http.ServerResponse(req);
44+
d.on('error', () => {
45+
res.end();
46+
res.emit('finish');
47+
resolve();
48+
});
49+
d.run(() =>
50+
process.nextTick(
51+
(req: Request, res: Response) =>
52+
Promise.resolve(fn(req, res)).then(() => {
53+
res.emit('finish');
54+
resolve();
55+
}),
56+
req,
57+
res as Response,
58+
),
59+
);
4560
});
4661
}
4762

@@ -211,21 +226,13 @@ describe('GCPFunction', () => {
211226

212227
const wrappedHandler = wrapHttpFunction(handler);
213228

214-
const request = {
215-
method: 'POST',
216-
url: '/path?q=query',
217-
headers: { host: 'hostname', 'content-type': 'application/json' },
218-
body: { foo: 'bar' },
219-
} as Request;
220-
221229
const mockEnd = jest.fn();
222-
const response = { end: mockEnd } as unknown as Response;
223230

224231
jest.spyOn(Sentry, 'flush').mockImplementationOnce(async () => {
225232
throw new Error();
226233
});
227234

228-
await expect(wrappedHandler(request, response)).resolves.toBeUndefined();
235+
await expect(handleHttp(wrappedHandler).then(mockEnd)).resolves.toBeUndefined();
229236
expect(mockEnd).toHaveBeenCalledTimes(1);
230237
});
231238
});

0 commit comments

Comments
 (0)