From 4bfa3bfdbe1257786331404685debe5e1321a5aa Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 8 Feb 2024 13:25:20 -0800 Subject: [PATCH 1/3] fix: Fix an issue where failed http requests could cause an unhandled promise rejection. (#371) Should fix #370 --- .../__tests__/platform/NodeRequests.test.ts | 29 +++++++++++++++++++ .../server-node/src/platform/NodeResponse.ts | 20 +++++++++++-- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts b/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts index d7ea0ff6c2..fda2c3c669 100644 --- a/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts +++ b/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts @@ -16,8 +16,14 @@ describe('given a default instance of NodeRequests', () => { let resolve: (value: TestRequestData | PromiseLike) => void; let promise: Promise; let server: http.Server; + let resetResolve: () => void; + let resetPromise: Promise; beforeEach(() => { + resetPromise = new Promise((res) => { + resetResolve = res; + }); + promise = new Promise((res) => { resolve = res; }); @@ -43,6 +49,14 @@ describe('given a default instance of NodeRequests', () => { } else if ((req.url?.indexOf('404') || -1) >= 0) { res.statusCode = 404; res.end(); + } else if ((req.url?.indexOf('reset') || -1) >= 0) { + res.statusCode = 200; + res.flushHeaders(); + res.write('potato'); + setTimeout(() => { + res.destroy(); + resetResolve(); + }, 0); } else { res.end(TEXT_RESPONSE); } @@ -115,4 +129,19 @@ describe('given a default instance of NodeRequests', () => { expect(serverResult.body).toEqual('BODY TEXT'); expect(serverResult.headers['sample-header']).toEqual('Some header value'); }); + + it('rejection is handled for response even if not awaited', async () => { + const res = await requests.fetch(`http://localhost:${PORT}/reset`); + expect(res.status).toEqual(200); + await resetPromise; + }); + + it('rejection is propagated with json promise', async () => { + const res = await requests.fetch(`http://localhost:${PORT}/reset`); + expect(res.status).toEqual(200); + + await expect(async () => { + await res.json(); + }).rejects.toThrow(); + }); }); diff --git a/packages/sdk/server-node/src/platform/NodeResponse.ts b/packages/sdk/server-node/src/platform/NodeResponse.ts index 7cbc0e573c..70c8861c0b 100644 --- a/packages/sdk/server-node/src/platform/NodeResponse.ts +++ b/packages/sdk/server-node/src/platform/NodeResponse.ts @@ -15,6 +15,9 @@ export default class NodeResponse implements platform.Response { status: number; + listened: boolean = false; + rejection?: Error; + constructor(res: http.IncomingMessage) { this.headers = new HeaderWrapper(res.headers); // Status code is optionally typed, but will always be present for this @@ -28,7 +31,10 @@ export default class NodeResponse implements platform.Response { }); res.on('error', (err) => { - reject(err); + this.rejection = err; + if (this.listened) { + reject(err); + } }); res.on('end', () => { @@ -37,12 +43,20 @@ export default class NodeResponse implements platform.Response { }); } - text(): Promise { + private async wrappedWait(): Promise { + this.listened = true; + if (this.rejection) { + throw this.rejection; + } return this.promise; } + text(): Promise { + return this.wrappedWait(); + } + async json(): Promise { - const stringValue = await this.promise; + const stringValue = await this.wrappedWait(); return JSON.parse(stringValue); } } From a20b1a9b9e67bd7e3f32050f5769f745a81d8cb7 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 8 Feb 2024 14:58:48 -0800 Subject: [PATCH 2/3] Lint --- packages/sdk/server-node/src/platform/NodeResponse.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/sdk/server-node/src/platform/NodeResponse.ts b/packages/sdk/server-node/src/platform/NodeResponse.ts index 70c8861c0b..b1e8d240e1 100644 --- a/packages/sdk/server-node/src/platform/NodeResponse.ts +++ b/packages/sdk/server-node/src/platform/NodeResponse.ts @@ -16,6 +16,7 @@ export default class NodeResponse implements platform.Response { status: number; listened: boolean = false; + rejection?: Error; constructor(res: http.IncomingMessage) { From dbc5279163077725e203c6685f56cea0d0036382 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 12 Feb 2024 12:49:27 -0800 Subject: [PATCH 3/3] Update packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts Co-authored-by: Yusinto Ngadiman --- .../sdk/server-node/__tests__/platform/NodeRequests.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts b/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts index fda2c3c669..5b6574a03a 100644 --- a/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts +++ b/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts @@ -49,7 +49,7 @@ describe('given a default instance of NodeRequests', () => { } else if ((req.url?.indexOf('404') || -1) >= 0) { res.statusCode = 404; res.end(); - } else if ((req.url?.indexOf('reset') || -1) >= 0) { + } else if (req.url?.includes('reset')) { res.statusCode = 200; res.flushHeaders(); res.write('potato');