Skip to content

Commit

Permalink
fix: use onFinished replace patch res.end, close getsentry#8848
Browse files Browse the repository at this point in the history
  • Loading branch information
jiangbo0216 committed Sep 26, 2023
1 parent 618e992 commit d6379dd
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 24 deletions.
2 changes: 2 additions & 0 deletions packages/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@
"cookie": "^0.5.0",
"https-proxy-agent": "^5.0.0",
"lru_map": "^0.3.3",
"on-finished": "^2.4.1",
"tslib": "^2.4.1 || ^1.9.3"
},
"devDependencies": {
"@types/cookie": "0.5.2",
"@types/express": "^4.17.14",
"@types/lru-cache": "^5.1.0",
"@types/node": "~10.17.0",
"@types/on-finished": "^2.3.1",
"express": "^4.17.1",
"nock": "^13.0.5",
"undici": "^5.21.0"
Expand Down
18 changes: 6 additions & 12 deletions packages/node/src/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
tracingContextFromHeaders,
} from '@sentry/utils';
import type * as http from 'http';
import * as onFinished from 'on-finished';

import type { NodeClient } from './client';
import { extractRequestData } from './requestdata';
Expand Down Expand Up @@ -173,18 +174,11 @@ export function requestHandler(
next: (error?: any) => void,
): void {
if (options && options.flushTimeout && options.flushTimeout > 0) {
// eslint-disable-next-line @typescript-eslint/unbound-method
const _end = res.end;
res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): void {
void flush(options.flushTimeout)
.then(() => {
_end.call(this, chunk, encoding, cb);
})
.then(null, e => {
__DEBUG_BUILD__ && logger.error(e);
_end.call(this, chunk, encoding, cb);
});
};
onFinished(res, () => {
void flush(options.flushTimeout).then(null, e => {
__DEBUG_BUILD__ && logger.error(e);
});
});
}
runWithAsyncContext(() => {
const currentHub = getCurrentHub();
Expand Down
2 changes: 2 additions & 0 deletions packages/node/test/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ describe('requestHandler', () => {
const sentryRequestMiddleware = requestHandler({ flushTimeout: 1337 });
sentryRequestMiddleware(req, res, next);
res.end('ok');
res.emit('finish');

setImmediate(() => {
expect(flush).toHaveBeenCalledWith(1337);
Expand All @@ -146,6 +147,7 @@ describe('requestHandler', () => {
const sentryRequestMiddleware = requestHandler({ flushTimeout: 1337 });
sentryRequestMiddleware(req, res, next);
res.end('ok');
res.emit('finish');

setImmediate(() => {
expect(res.finished).toBe(true);
Expand Down
2 changes: 2 additions & 0 deletions packages/serverless/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"@sentry/utils": "7.71.0",
"@types/aws-lambda": "^8.10.62",
"@types/express": "^4.17.14",
"on-finished": "^2.4.1",
"tslib": "^2.4.1 || ^1.9.3"
},
"devDependencies": {
Expand All @@ -37,6 +38,7 @@
"@google-cloud/functions-framework": "^1.7.1",
"@google-cloud/pubsub": "^2.5.0",
"@types/node": "^14.6.4",
"@types/on-finished": "^2.3.1",
"aws-sdk": "^2.765.0",
"find-up": "^5.0.0",
"google-gax": "^2.9.0",
Expand Down
14 changes: 4 additions & 10 deletions packages/serverless/src/gcpfunction/http.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { AddRequestDataToEventOptions } from '@sentry/node';
import { captureException, flush, getCurrentHub } from '@sentry/node';
import { isString, isThenable, logger, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils';
import * as onFinished from 'on-finished';

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

// eslint-disable-next-line @typescript-eslint/unbound-method
const _end = res.end;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): any {
onFinished(res, (err, res) => {
transaction?.setHttpStatus(res.statusCode);
transaction?.finish();

void flush(options.flushTimeout)
.then(null, e => {
.then(null, (e: unknown) => {
__DEBUG_BUILD__ && logger.error(e);
})
.then(() => {
_end.call(this, chunk, encoding, cb);
});
};
});

let fnResult;
try {
Expand Down
11 changes: 10 additions & 1 deletion packages/serverless/test/gcpfunction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,16 @@ describe('GCPFunction', () => {
headers: headers,
body: { foo: 'bar' },
} as Request;
const res = { end: resolve } as Response;
// make res extends EventEmitter
// eslint-disable-next-line @typescript-eslint/no-var-requires
const res = Object.create(require('events').EventEmitter.prototype);
// const res = { end: resolve } as Response;
res.end = function () {
this.emit('finish');
// onFinished use setImmediate to defer listener, same as here.
setImmediate(resolve)
}

d.on('error', () => res.end());
d.run(() => process.nextTick(fn, req, res));
});
Expand Down
9 changes: 8 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5425,6 +5425,13 @@
resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.0.tgz#e486d0d97396d79beedd0a6e33f4534ff6b4973e"
integrity sha512-f5j5b/Gf71L+dbqxIpQ4Z2WlmI/mPJ0fOkGGmFgtb6sAu97EPczzbS3/tJKxmcYDj55OX6ssqwDAWOHIYDRDGA==

"@types/on-finished@^2.3.1":
version "2.3.1"
resolved "https://registry.yarnpkg.com/@types/on-finished/-/on-finished-2.3.1.tgz#4537f9f2b47b3ba0b92c14a4bcc0f755aeda3484"
integrity sha512-mzVYaYcFs5Jd2n/O6uYIRUsFRR1cHyZLRvkLCU0E7+G5WhY0qBDAR5fUCeZbvecYOSh9ikhlesyi2UfI8B9ckQ==
dependencies:
"@types/node" "*"

"@types/pako@^2.0.0":
version "2.0.0"
resolved "https://registry.yarnpkg.com/@types/pako/-/pako-2.0.0.tgz#12ab4c19107528452e73ac99132c875ccd43bdfb"
Expand Down Expand Up @@ -20921,7 +20928,7 @@ obuf@^1.0.0, obuf@^1.1.2, obuf@~1.1.2:
resolved "https://registry.yarnpkg.com/obuf/-/obuf-1.1.2.tgz#09bea3343d41859ebd446292d11c9d4db619084e"
integrity sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg==

on-finished@2.4.1, on-finished@^2.3.0:
on-finished@2.4.1, on-finished@^2.3.0, on-finished@^2.4.1:
version "2.4.1"
resolved "https://registry.yarnpkg.com/on-finished/-/on-finished-2.4.1.tgz#58c8c44116e54845ad57f14ab10b03533184ac3f"
integrity sha512-oVlzkg3ENAhCk2zdv7IJwd/QUD4z2RxRwpkcGY8psCVcCYZNq4wYnVWALHM+brtuJjePWiYF/ClmuDr8Ch5+kg==
Expand Down

0 comments on commit d6379dd

Please sign in to comment.