Skip to content

Commit efb2e29

Browse files
committed
fix: use onFinished replace patch res.end, close getsentry#8848
1 parent 437d20a commit efb2e29

File tree

4 files changed

+32
-41
lines changed

4 files changed

+32
-41
lines changed

packages/node/src/handlers.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -143,20 +143,6 @@ export function requestHandler(
143143
res: http.ServerResponse,
144144
next: (error?: any) => void,
145145
): void {
146-
if (options && options.flushTimeout && options.flushTimeout > 0) {
147-
// eslint-disable-next-line @typescript-eslint/unbound-method
148-
const _end = res.end;
149-
res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): void {
150-
void flush(options.flushTimeout)
151-
.then(() => {
152-
_end.call(this, chunk, encoding, cb);
153-
})
154-
.then(null, e => {
155-
DEBUG_BUILD && logger.error(e);
156-
_end.call(this, chunk, encoding, cb);
157-
});
158-
};
159-
}
160146
return withIsolationScope(isolationScope => {
161147
isolationScope.setSDKProcessingMetadata({
162148
request: req,
@@ -181,6 +167,12 @@ export function requestHandler(
181167
}
182168
});
183169
}
170+
171+
if (options && options.flushTimeout && options.flushTimeout > 0) {
172+
void flush(options.flushTimeout).then(null, e => {
173+
DEBUG_BUILD && logger.error(e);
174+
});
175+
}
184176
});
185177
next();
186178
});

packages/node/test/handlers.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ describe('requestHandler', () => {
148148
const sentryRequestMiddleware = requestHandler({ flushTimeout: 1337 });
149149
sentryRequestMiddleware(req, res, next);
150150
res.end('ok');
151+
res.emit('finish');
151152

152153
setImmediate(() => {
153154
expect(flush).toHaveBeenCalledWith(1337);
@@ -162,6 +163,7 @@ describe('requestHandler', () => {
162163
const sentryRequestMiddleware = requestHandler({ flushTimeout: 1337 });
163164
sentryRequestMiddleware(req, res, next);
164165
res.end('ok');
166+
res.emit('finish');
165167

166168
setImmediate(() => {
167169
expect(res.finished).toBe(true);

packages/serverless/src/gcpfunction/http.ts

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -68,25 +68,15 @@ function _wrapHttpFunction(fn: HttpFunction, options: Partial<WrapperOptions>):
6868
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
6969
(res as any).__sentry_transaction = span;
7070
}
71-
72-
// eslint-disable-next-line @typescript-eslint/unbound-method
73-
const _end = res.end;
74-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
75-
res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): any {
71+
res.once('finish', (): void => {
7672
if (span) {
7773
setHttpStatus(span, res.statusCode);
7874
span.end();
7975
}
80-
81-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
82-
flush(flushTimeout)
83-
.then(null, e => {
84-
DEBUG_BUILD && logger.error(e);
85-
})
86-
.then(() => {
87-
_end.call(this, chunk, encoding, cb);
88-
});
89-
};
76+
void flush(options.flushTimeout).then(null, (e: unknown) => {
77+
DEBUG_BUILD && logger.error(e);
78+
});
79+
});
9080

9181
return handleCallbackErrors(
9282
() => fn(req, res),

packages/serverless/test/gcpfunction.test.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as domain from 'domain';
2+
import * as http from 'http';
23

34
import type { Event, Integration } from '@sentry/types';
45

@@ -89,9 +90,23 @@ describe('GCPFunction', () => {
8990
headers: headers,
9091
body: { foo: 'bar' },
9192
} as Request;
92-
const res = { end: resolve } as Response;
93-
d.on('error', () => res.end());
94-
d.run(() => process.nextTick(fn, req, res));
93+
const res = new http.ServerResponse(req);
94+
d.on('error', () => {
95+
res.end();
96+
res.emit('finish');
97+
resolve();
98+
});
99+
d.run(() =>
100+
process.nextTick(
101+
(req: Request, res: Response) =>
102+
Promise.resolve(fn(req, res)).then(() => {
103+
res.emit('finish');
104+
resolve();
105+
}),
106+
req,
107+
res as Response,
108+
),
109+
);
95110
});
96111
}
97112

@@ -203,21 +218,13 @@ describe('GCPFunction', () => {
203218

204219
const wrappedHandler = wrapHttpFunction(handler);
205220

206-
const request = {
207-
method: 'POST',
208-
url: '/path?q=query',
209-
headers: { host: 'hostname', 'content-type': 'application/json' },
210-
body: { foo: 'bar' },
211-
} as Request;
212-
213221
const mockEnd = jest.fn();
214-
const response = { end: mockEnd } as unknown as Response;
215222

216223
mockFlush.mockImplementationOnce(async () => {
217224
throw new Error();
218225
});
219226

220-
await expect(wrappedHandler(request, response)).resolves.toBeUndefined();
227+
await expect(handleHttp(wrappedHandler).then(mockEnd)).resolves.toBeUndefined();
221228
expect(mockEnd).toHaveBeenCalledTimes(1);
222229
});
223230
});

0 commit comments

Comments
 (0)