Skip to content

Commit 939b65d

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

File tree

7 files changed

+349
-29
lines changed

7 files changed

+349
-29
lines changed

packages/node/src/handlers.ts

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { extractRequestData } from './requestdata';
2727
// TODO (v8 / XXX) Remove this import
2828
import type { ParseRequestOptions } from './requestDataDeprecated';
2929
import { isAutoSessionTrackingEnabled } from './sdk';
30+
import { onFinished } from './vendor/on-finished';
3031

3132
/**
3233
* Express-compatible tracing handler.
@@ -173,18 +174,11 @@ export function requestHandler(
173174
next: (error?: any) => void,
174175
): void {
175176
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-
};
177+
onFinished(res, () => {
178+
void flush(options.flushTimeout).then(null, e => {
179+
__DEBUG_BUILD__ && logger.error(e);
180+
});
181+
});
188182
}
189183
runWithAsyncContext(() => {
190184
const currentHub = getCurrentHub();
@@ -364,6 +358,6 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) {
364358

365359
// TODO (v8 / #5257): Remove this
366360
// eslint-disable-next-line deprecation/deprecation
367-
export type { ParseRequestOptions, ExpressRequest } from './requestDataDeprecated';
361+
export type { ExpressRequest, ParseRequestOptions } from './requestDataDeprecated';
368362
// eslint-disable-next-line deprecation/deprecation
369-
export { parseRequest, extractRequestData } from './requestDataDeprecated';
363+
export { extractRequestData, parseRequest } from './requestDataDeprecated';

packages/node/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,5 @@ const INTEGRATIONS = {
8686
};
8787

8888
export { INTEGRATIONS as Integrations, Handlers };
89+
90+
export { onFinished } from './vendor/on-finished';

packages/node/src/vendor/ee-first.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/* eslint-disable */
2+
// @ts-nocheck
3+
/* !
4+
* ee-first
5+
* Copyright(c) 2014 Jonathan Ong
6+
* MIT Licensed
7+
* https://github.com/jonathanong/ee-first/blob/master/index.js
8+
*/
9+
10+
/**
11+
* Get the first event in a set of event emitters and event pairs.
12+
*
13+
* @param {array} stuff
14+
* @param {function} done
15+
* @public
16+
*/
17+
18+
export function first(stuff, done) {
19+
if (!Array.isArray(stuff)) {
20+
throw new TypeError('arg must be an array of [ee, events...] arrays');
21+
}
22+
23+
var cleanups = [];
24+
25+
for (var i = 0; i < stuff.length; i++) {
26+
var arr = stuff[i];
27+
28+
if (!Array.isArray(arr) || arr.length < 2) {
29+
throw new TypeError('each array member must be [ee, events...]');
30+
}
31+
32+
var ee = arr[0];
33+
34+
for (var j = 1; j < arr.length; j++) {
35+
var event = arr[j];
36+
var fn = listener(event, callback);
37+
38+
// listen to the event
39+
ee.on(event, fn);
40+
// push this listener to the list of cleanups
41+
cleanups.push({
42+
ee: ee,
43+
event: event,
44+
fn: fn,
45+
});
46+
}
47+
}
48+
49+
function callback() {
50+
cleanup();
51+
done.apply(null, arguments);
52+
}
53+
54+
function cleanup() {
55+
var x;
56+
for (var i = 0; i < cleanups.length; i++) {
57+
x = cleanups[i];
58+
x.ee.removeListener(x.event, x.fn);
59+
}
60+
}
61+
62+
function thunk(fn) {
63+
done = fn;
64+
}
65+
66+
thunk.cancel = cleanup;
67+
68+
return thunk;
69+
}
70+
71+
/**
72+
* Create the event listener.
73+
* @private
74+
*/
75+
76+
function listener(event, done) {
77+
return function onevent(arg1) {
78+
var args = new Array(arguments.length);
79+
var ee = this;
80+
var err = event === 'error' ? arg1 : null;
81+
82+
// copy args to prevent arguments escaping scope
83+
for (var i = 0; i < args.length; i++) {
84+
args[i] = arguments[i];
85+
}
86+
87+
done(err, ee, event, args);
88+
};
89+
}
Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
/* eslint-disable */
2+
// @ts-nocheck
3+
/* !
4+
* on-finished
5+
* Copyright(c) 2013 Jonathan Ong
6+
* Copyright(c) 2014 Douglas Christopher Wilson
7+
* MIT Licensed
8+
* https://github.com/jshttp/on-finished/blob/master/index.js
9+
*/
10+
11+
import type EventEmitter from 'events';
12+
import { first } from './ee-first';
13+
14+
/**
15+
* Module dependencies.
16+
* @private
17+
*/
18+
19+
var asyncHooks = tryRequireAsyncHooks();
20+
21+
/**
22+
* Variables.
23+
* @private
24+
*/
25+
26+
/* istanbul ignore next */
27+
var defer =
28+
typeof setImmediate === 'function'
29+
? setImmediate
30+
: function (fn) {
31+
process.nextTick(fn.bind.apply(fn, arguments));
32+
};
33+
34+
/**
35+
* Invoke callback when the response has finished, useful for
36+
* cleaning up resources afterwards.
37+
*
38+
* @param {object} msg
39+
* @param {function} listener
40+
* @return {object}
41+
* @public
42+
*/
43+
44+
export function onFinished<T extends EventEmitter>(msg: T, listener: (err: unknown, msg: T) => void) {
45+
if (isFinished(msg) !== false) {
46+
defer(listener, null, msg);
47+
return msg;
48+
}
49+
50+
// attach the listener to the message
51+
attachListener(msg, wrap(listener));
52+
53+
return msg;
54+
}
55+
56+
/**
57+
* Determine if message is already finished.
58+
*
59+
* @param {object} msg
60+
* @return {boolean}
61+
* @public
62+
*/
63+
64+
function isFinished(msg) {
65+
var socket = msg.socket;
66+
67+
if (typeof msg.finished === 'boolean') {
68+
// OutgoingMessage
69+
return Boolean(msg.finished || (socket && !socket.writable));
70+
}
71+
72+
if (typeof msg.complete === 'boolean') {
73+
// IncomingMessage
74+
return Boolean(msg.upgrade || !socket || !socket.readable || (msg.complete && !msg.readable));
75+
}
76+
77+
// don't know
78+
return undefined;
79+
}
80+
81+
/**
82+
* Attach a finished listener to the message.
83+
*
84+
* @param {object} msg
85+
* @param {function} callback
86+
* @private
87+
*/
88+
89+
function attachFinishedListener(msg, callback) {
90+
var eeMsg;
91+
var eeSocket;
92+
var finished = false;
93+
94+
function onFinish(error) {
95+
eeMsg.cancel();
96+
eeSocket.cancel();
97+
98+
finished = true;
99+
callback(error);
100+
}
101+
102+
// finished on first message event
103+
eeMsg = eeSocket = first([[msg, 'end', 'finish']], onFinish);
104+
105+
function onSocket(socket) {
106+
// remove listener
107+
msg.removeListener('socket', onSocket);
108+
109+
if (finished) return;
110+
if (eeMsg !== eeSocket) return;
111+
112+
// finished on first socket event
113+
eeSocket = first([[socket, 'error', 'close']], onFinish);
114+
}
115+
116+
if (msg.socket) {
117+
// socket already assigned
118+
onSocket(msg.socket);
119+
return;
120+
}
121+
122+
// wait for socket to be assigned
123+
msg.on('socket', onSocket);
124+
125+
if (msg.socket === undefined) {
126+
// istanbul ignore next: node.js 0.8 patch
127+
patchAssignSocket(msg, onSocket);
128+
}
129+
}
130+
131+
/**
132+
* Attach the listener to the message.
133+
*
134+
* @param {object} msg
135+
* @return {function}
136+
* @private
137+
*/
138+
139+
function attachListener(msg, listener) {
140+
var attached = msg.__onFinished;
141+
142+
// create a private single listener with queue
143+
if (!attached || !attached.queue) {
144+
attached = msg.__onFinished = createListener(msg);
145+
attachFinishedListener(msg, attached);
146+
}
147+
148+
attached.queue.push(listener);
149+
}
150+
151+
/**
152+
* Create listener on message.
153+
*
154+
* @param {object} msg
155+
* @return {function}
156+
* @private
157+
*/
158+
159+
function createListener(msg) {
160+
function listener(err) {
161+
if (msg.__onFinished === listener) msg.__onFinished = null;
162+
if (!listener.queue) return;
163+
164+
var queue = listener.queue;
165+
listener.queue = null;
166+
167+
for (var i = 0; i < queue.length; i++) {
168+
queue[i](err, msg);
169+
}
170+
}
171+
172+
listener.queue = [];
173+
174+
return listener;
175+
}
176+
177+
/**
178+
* Patch ServerResponse.prototype.assignSocket for node.js 0.8.
179+
*
180+
* @param {ServerResponse} res
181+
* @param {function} callback
182+
* @private
183+
*/
184+
185+
// istanbul ignore next: node.js 0.8 patch
186+
function patchAssignSocket(res, callback) {
187+
var assignSocket = res.assignSocket;
188+
189+
if (typeof assignSocket !== 'function') return;
190+
191+
// res.on('socket', callback) is broken in 0.8
192+
res.assignSocket = function _assignSocket(socket) {
193+
assignSocket.call(this, socket);
194+
callback(socket);
195+
};
196+
}
197+
198+
/**
199+
* Try to require async_hooks
200+
* @private
201+
*/
202+
203+
function tryRequireAsyncHooks() {
204+
try {
205+
return require('async_hooks');
206+
} catch (e) {
207+
return {};
208+
}
209+
}
210+
211+
/**
212+
* Wrap function with async resource, if possible.
213+
* AsyncResource.bind static method backported.
214+
* @private
215+
*/
216+
217+
function wrap(fn) {
218+
var res;
219+
220+
// create anonymous resource
221+
if (asyncHooks.AsyncResource) {
222+
res = new asyncHooks.AsyncResource(fn.name || 'bound-anonymous-fn');
223+
}
224+
225+
// incompatible node.js
226+
if (!res || !res.runInAsyncScope) {
227+
return fn;
228+
}
229+
230+
// return bound function
231+
return res.runInAsyncScope.bind(res, fn, null);
232+
}

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);

0 commit comments

Comments
 (0)