Skip to content

Commit ecfd2db

Browse files
kinyoklionyusinto
andauthored
fix: Fix an issue where failed http requests could cause an unhandled promise rejection. (#374)
This is a cherry-pick from the 9.x release. --------- Co-authored-by: Yusinto Ngadiman <[email protected]>
1 parent 1b7fe3c commit ecfd2db

File tree

2 files changed

+47
-3
lines changed

2 files changed

+47
-3
lines changed

packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,14 @@ describe('given a default instance of NodeRequests', () => {
1616
let resolve: (value: TestRequestData | PromiseLike<TestRequestData>) => void;
1717
let promise: Promise<TestRequestData>;
1818
let server: http.Server;
19+
let resetResolve: () => void;
20+
let resetPromise: Promise<void>;
1921

2022
beforeEach(() => {
23+
resetPromise = new Promise((res) => {
24+
resetResolve = res;
25+
});
26+
2127
promise = new Promise<TestRequestData>((res) => {
2228
resolve = res;
2329
});
@@ -43,6 +49,14 @@ describe('given a default instance of NodeRequests', () => {
4349
} else if ((req.url?.indexOf('404') || -1) >= 0) {
4450
res.statusCode = 404;
4551
res.end();
52+
} else if (req.url?.includes('reset')) {
53+
res.statusCode = 200;
54+
res.flushHeaders();
55+
res.write('potato');
56+
setTimeout(() => {
57+
res.destroy();
58+
resetResolve();
59+
}, 0);
4660
} else {
4761
res.end(TEXT_RESPONSE);
4862
}
@@ -115,4 +129,19 @@ describe('given a default instance of NodeRequests', () => {
115129
expect(serverResult.body).toEqual('BODY TEXT');
116130
expect(serverResult.headers['sample-header']).toEqual('Some header value');
117131
});
132+
133+
it('rejection is handled for response even if not awaited', async () => {
134+
const res = await requests.fetch(`http://localhost:${PORT}/reset`);
135+
expect(res.status).toEqual(200);
136+
await resetPromise;
137+
});
138+
139+
it('rejection is propagated with json promise', async () => {
140+
const res = await requests.fetch(`http://localhost:${PORT}/reset`);
141+
expect(res.status).toEqual(200);
142+
143+
await expect(async () => {
144+
await res.json();
145+
}).rejects.toThrow();
146+
});
118147
});

packages/sdk/server-node/src/platform/NodeResponse.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ export default class NodeResponse implements platform.Response {
1515

1616
status: number;
1717

18+
listened: boolean = false;
19+
20+
rejection?: Error;
21+
1822
constructor(res: http.IncomingMessage) {
1923
this.headers = new HeaderWrapper(res.headers);
2024
// Status code is optionally typed, but will always be present for this
@@ -28,7 +32,10 @@ export default class NodeResponse implements platform.Response {
2832
});
2933

3034
res.on('error', (err) => {
31-
reject(err);
35+
this.rejection = err;
36+
if (this.listened) {
37+
reject(err);
38+
}
3239
});
3340

3441
res.on('end', () => {
@@ -37,12 +44,20 @@ export default class NodeResponse implements platform.Response {
3744
});
3845
}
3946

40-
text(): Promise<string> {
47+
private async wrappedWait(): Promise<string> {
48+
this.listened = true;
49+
if (this.rejection) {
50+
throw this.rejection;
51+
}
4152
return this.promise;
4253
}
4354

55+
text(): Promise<string> {
56+
return this.wrappedWait();
57+
}
58+
4459
async json(): Promise<any> {
45-
const stringValue = await this.promise;
60+
const stringValue = await this.wrappedWait();
4661
return JSON.parse(stringValue);
4762
}
4863
}

0 commit comments

Comments
 (0)