fix: substantially clarify the awaitWithTimeout interface

Author: feywind

src/subscriber.ts

@@ -991,14 +991,18 @@ export class Subscriber extends EventEmitter {
         this._inventory.on('empty', r);
       });
 
-      try {
-        await awaitWithTimeout(emptyPromise, waitTimeout);
-      } catch (e) {
+      const resultCompletion = await awaitWithTimeout(
+        emptyPromise,
+        waitTimeout,
+      );
+      if (resultCompletion.exception || resultCompletion.timedOut) {
         // Don't try to deal with errors at this point, just warn-log.
-        const err = e as [unknown, boolean];
-        if (err[1] === false) {
+        if (resultCompletion.timedOut === false) {
           // This wasn't a timeout.
-          logs.debug.warn('Error during Subscriber.close(): %j', err[0]);
+          logs.debug.warn(
+            'Error during Subscriber.close(): %j',
+            resultCompletion.exception,
+          );
         }
       }
     }
@@ -1019,14 +1023,15 @@ export class Subscriber extends EventEmitter {
 
     // Wait for user callbacks to complete.
     const flushCompleted = this._waitForFlush();
-    try {
-      await awaitWithTimeout(flushCompleted, timeout);
-    } catch (e) {
+    const flushResult = await awaitWithTimeout(flushCompleted, timeout);
+    if (flushResult.exception || flushResult.timedOut) {
       // Don't try to deal with errors at this point, just warn-log.
-      const err = e as [unknown, boolean];
-      if (err[1] === false) {
+      if (flushResult.timedOut === false) {
         // This wasn't a timeout.
-        logs.debug.warn('Error during Subscriber.close(): %j', err[0]);
+        logs.debug.warn(
+          'Error during Subscriber.close(): %j',
+          flushResult.exception,
+        );
       }
     }
 

src/util.ts

@@ -88,29 +88,48 @@ export function addToBucket<T, U>(map: Map<T, U[]>, bucket: T, item: U) {
 const timeoutToken: unique symbol = Symbol('pubsub promise timeout');
 
 /**
- * Awaits on Promise with a timeout. Resolves on success, rejects on timeout.
+ * Return value from `awaitWithTimeout`. There are several variations on what
+ * might happen here, so this bundles it all up into a "report".
+ */
+export interface TimeoutReturn<T> {
+  returnedValue?: T;
+  exception?: Error;
+  timedOut: boolean;
+}
+
+/**
+ * Awaits on Promise with a timeout. Resolves on the passed promise resolving or
+ * rejecting, or on timeout.
  *
  * @param promise An existing Promise returning type T.
  * @param timeout A timeout value as a Duration; if the timeout is exceeded while
- *   waiting for the promise, the Promise this function returns will reject.
- * @returns In any case, a tuple with the first item being the T value or an
- *   error value, and the second item being true if the timeout was exceeded.
+ *   waiting for the promise, the Promise this function returns will resolve, but
+ *   with `timedOut` set.
+ * @returns A TimeoutReturn with the returned value, if applicable, an exception if
+ *   the promise rejected, or the timedOut set to true if it timed out.
  */
 export async function awaitWithTimeout<T>(
   promise: Promise<T>,
   timeout: Duration,
-): Promise<[T, boolean]> {
+): Promise<TimeoutReturn<T>> {
   let timeoutId: NodeJS.Timeout | undefined;
   const timeoutPromise = new Promise<T>((_, rej) => {
     timeoutId = setTimeout(() => rej(timeoutToken), timeout.milliseconds);
   });
   try {
     const value = await Promise.race([timeoutPromise, promise]);
     clearTimeout(timeoutId);
-    return [value, false];
+    return {
+      returnedValue: value,
+      timedOut: false,
+    };
   } catch (e) {
-    // The timeout passed.
+    const err: Error | symbol = e as unknown as Error | symbol;
+    // The timeout passed or the promise rejected.
     clearTimeout(timeoutId);
-    throw [e, e === timeoutToken];
+    return {
+      exception: (err !== timeoutToken ? err : undefined) as Error | undefined,
+      timedOut: err === timeoutToken,
+    };
   }
 }

test/util.ts

@@ -83,12 +83,15 @@ describe('utils', () => {
         Duration.from({seconds: 1}),
       );
       fakeTimers.tick(500);
-      try {
-        const result = await awaitPromise;
-        assert.deepStrictEqual(result, [testString, false]);
-      } catch (e) {
-        assert.strictEqual(e, null, 'timeout was triggered, improperly');
-      }
+
+      const result = await awaitPromise;
+      assert.strictEqual(result.returnedValue, testString);
+      assert.strictEqual(result.exception, undefined);
+      assert.strictEqual(
+        result.timedOut,
+        false,
+        'timeout was triggered, improperly',
+      );
     });
 
     it('handles non-timeout errors properly', async () => {
@@ -104,12 +107,14 @@ describe('utils', () => {
         Duration.from({seconds: 1}),
       );
       fakeTimers.tick(500);
-      try {
-        const result = await awaitPromise;
-        assert.strictEqual(result, null, 'non-error was triggered, improperly');
-      } catch (e) {
-        assert.deepStrictEqual(e, [testString, false]);
-      }
+
+      const result = await awaitPromise;
+      assert.strictEqual(
+        result.exception,
+        testString,
+        'non-error was triggered, improperly',
+      );
+      assert.strictEqual(result.timedOut, false);
     });
 
     it('handles timeout properly', async () => {
@@ -125,17 +130,15 @@ describe('utils', () => {
         Duration.from({seconds: 1}),
       );
       fakeTimers.tick(1500);
-      try {
-        const result = await awaitPromise;
-        assert.strictEqual(
-          result,
-          null,
-          'timeout was not triggered, improperly',
-        );
-      } catch (e) {
-        const err: unknown[] = e as unknown[];
-        assert.strictEqual(err[1], true);
-      }
+
+      const result = await awaitPromise;
+      assert.strictEqual(
+        result.timedOut,
+        true,
+        'timeout was not triggered, improperly',
+      );
+
+      assert.strictEqual(result.timedOut, true);
     });
   });
 });