Skip to content

Commit e1e5160

Browse files
committed
fix: use onFinished replace patch res.end, close getsentry#8848
1 parent 2d80b4b commit e1e5160

File tree

7 files changed

+34
-24
lines changed

7 files changed

+34
-24
lines changed

packages/node/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@
3030
"cookie": "^0.4.1",
3131
"https-proxy-agent": "^5.0.0",
3232
"lru_map": "^0.3.3",
33+
"on-finished": "^2.4.1",
3334
"tslib": "^2.4.1 || ^1.9.3"
3435
},
3536
"devDependencies": {
3637
"@types/cookie": "0.3.2",
3738
"@types/express": "^4.17.14",
3839
"@types/lru-cache": "^5.1.0",
3940
"@types/node": "~10.17.0",
41+
"@types/on-finished": "^2.3.1",
4042
"express": "^4.17.1",
4143
"nock": "^13.0.5",
4244
"undici": "^5.21.0"

packages/node/src/handlers.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
tracingContextFromHeaders,
2222
} from '@sentry/utils';
2323
import type * as http from 'http';
24+
import * as onFinished from 'on-finished';
2425

2526
import type { NodeClient } from './client';
2627
import { extractRequestData } from './requestdata';
@@ -172,18 +173,11 @@ export function requestHandler(
172173
next: (error?: any) => void,
173174
): void {
174175
if (options && options.flushTimeout && options.flushTimeout > 0) {
175-
// eslint-disable-next-line @typescript-eslint/unbound-method
176-
const _end = res.end;
177-
res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): void {
178-
void flush(options.flushTimeout)
179-
.then(() => {
180-
_end.call(this, chunk, encoding, cb);
181-
})
182-
.then(null, e => {
183-
__DEBUG_BUILD__ && logger.error(e);
184-
_end.call(this, chunk, encoding, cb);
185-
});
186-
};
176+
onFinished(res, () => {
177+
void flush(options.flushTimeout).then(null, e => {
178+
__DEBUG_BUILD__ && logger.error(e);
179+
});
180+
});
187181
}
188182
runWithAsyncContext(() => {
189183
const currentHub = getCurrentHub();

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/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
"@sentry/utils": "7.64.0",
3030
"@types/aws-lambda": "^8.10.62",
3131
"@types/express": "^4.17.14",
32+
"on-finished": "^2.4.1",
3233
"tslib": "^2.4.1 || ^1.9.3"
3334
},
3435
"devDependencies": {
@@ -37,6 +38,7 @@
3738
"@google-cloud/functions-framework": "^1.7.1",
3839
"@google-cloud/pubsub": "^2.5.0",
3940
"@types/node": "^14.6.4",
41+
"@types/on-finished": "^2.3.1",
4042
"aws-sdk": "^2.765.0",
4143
"find-up": "^5.0.0",
4244
"google-gax": "^2.9.0",

packages/serverless/src/gcpfunction/http.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { AddRequestDataToEventOptions } from '@sentry/node';
22
import { captureException, flush, getCurrentHub } from '@sentry/node';
33
import { isString, isThenable, logger, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils';
4+
import * as onFinished from 'on-finished';
45

56
import { domainify, proxyFunction } from './../utils';
67
import type { HttpFunction, WrapperOptions } from './general';
@@ -101,21 +102,14 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
101102
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
102103
(res as any).__sentry_transaction = transaction;
103104

104-
// eslint-disable-next-line @typescript-eslint/unbound-method
105-
const _end = res.end;
106-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
107-
res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): any {
105+
onFinished(res, (err, res) => {
108106
transaction?.setHttpStatus(res.statusCode);
109107
transaction?.finish();
110-
111108
void flush(options.flushTimeout)
112-
.then(null, e => {
109+
.then(null, (e: unknown) => {
113110
__DEBUG_BUILD__ && logger.error(e);
114-
})
115-
.then(() => {
116-
_end.call(this, chunk, encoding, cb);
117111
});
118-
};
112+
});
119113

120114
let fnResult;
121115
try {

packages/serverless/test/gcpfunction.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,16 @@ describe('GCPFunction', () => {
3939
headers: headers,
4040
body: { foo: 'bar' },
4141
} as Request;
42-
const res = { end: resolve } as Response;
42+
// make res extends EventEmitter
43+
// eslint-disable-next-line @typescript-eslint/no-var-requires
44+
const res = Object.create(require('events').EventEmitter.prototype);
45+
// const res = { end: resolve } as Response;
46+
res.end = function () {
47+
this.emit('finish');
48+
// onFinished use setImmediate to defer listener, same as here.
49+
setImmediate(resolve)
50+
}
51+
4352
d.on('error', () => res.end());
4453
d.run(() => process.nextTick(fn, req, res));
4554
});

yarn.lock

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5369,6 +5369,13 @@
53695369
resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.0.tgz#e486d0d97396d79beedd0a6e33f4534ff6b4973e"
53705370
integrity sha512-f5j5b/Gf71L+dbqxIpQ4Z2WlmI/mPJ0fOkGGmFgtb6sAu97EPczzbS3/tJKxmcYDj55OX6ssqwDAWOHIYDRDGA==
53715371

5372+
"@types/on-finished@^2.3.1":
5373+
version "2.3.1"
5374+
resolved "https://registry.yarnpkg.com/@types/on-finished/-/on-finished-2.3.1.tgz#4537f9f2b47b3ba0b92c14a4bcc0f755aeda3484"
5375+
integrity sha512-mzVYaYcFs5Jd2n/O6uYIRUsFRR1cHyZLRvkLCU0E7+G5WhY0qBDAR5fUCeZbvecYOSh9ikhlesyi2UfI8B9ckQ==
5376+
dependencies:
5377+
"@types/node" "*"
5378+
53725379
"@types/pako@^2.0.0":
53735380
version "2.0.0"
53745381
resolved "https://registry.yarnpkg.com/@types/pako/-/pako-2.0.0.tgz#12ab4c19107528452e73ac99132c875ccd43bdfb"
@@ -20772,7 +20779,7 @@ obuf@^1.0.0, obuf@^1.1.2, obuf@~1.1.2:
2077220779
resolved "https://registry.yarnpkg.com/obuf/-/obuf-1.1.2.tgz#09bea3343d41859ebd446292d11c9d4db619084e"
2077320780
integrity sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg==
2077420781

20775-
[email protected], on-finished@^2.3.0:
20782+
[email protected], on-finished@^2.3.0, on-finished@^2.4.1:
2077620783
version "2.4.1"
2077720784
resolved "https://registry.yarnpkg.com/on-finished/-/on-finished-2.4.1.tgz#58c8c44116e54845ad57f14ab10b03533184ac3f"
2077820785
integrity sha512-oVlzkg3ENAhCk2zdv7IJwd/QUD4z2RxRwpkcGY8psCVcCYZNq4wYnVWALHM+brtuJjePWiYF/ClmuDr8Ch5+kg==

0 commit comments

Comments
 (0)