From 7b342c0eb0f5e5b0ec48705e0a67397baf32e517 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:46:39 -0800 Subject: [PATCH] Fix an issue where failed http requests would cause an unhandled promise rejection. --- .../__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); } }