fix: Fix an issue where failed http requests could cause an unhandled promise rejection. (#371)

Should fix #370

Author: kinyoklion

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<TestRequestData>) => void;
   let promise: Promise<TestRequestData>;
   let server: http.Server;
+  let resetResolve: () => void;
+  let resetPromise: Promise<void>;
 
   beforeEach(() => {
+    resetPromise = new Promise((res) => {
+      resetResolve = res;
+    });
+
     promise = new Promise<TestRequestData>((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();
+  });
 });

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<string> {
+  private async wrappedWait(): Promise<string> {
+    this.listened = true;
+    if (this.rejection) {
+      throw this.rejection;
+    }
     return this.promise;
   }
 
+  text(): Promise<string> {
+    return this.wrappedWait();
+  }
+
   async json(): Promise<any> {
-    const stringValue = await this.promise;
+    const stringValue = await this.wrappedWait();
     return JSON.parse(stringValue);
   }
 }