Merge pull request #53 from mrjones2014/main

fix race condition between catch and finally in do-try.ts

Author: brandongregoryscott

src/utilities/core-utils.ts

@@ -92,6 +92,18 @@ const _sleep = (milliseconds: number, debug: boolean = false) => {
     );
 };
 
+/**
+ * Block execution for specified number of milliseconds, synchronously.
+ * @param milliseconds the delay
+ */
+const _sleepSync = (milliseconds: number) => {
+    let now = Date.now();
+    const start = now;
+    while (now - start < milliseconds) {
+        now = Date.now();
+    }
+};
+
 /**
  * Creates a timer instance that when stopped will supply elapsed time in milliseconds.
  * Useful for benchmarking or providing counters
@@ -134,6 +146,7 @@ export const CoreUtils = {
     objectToArray: _objectToArray,
     range: _.range,
     sleep: _sleep,
+    sleepSync: _sleepSync,
     throttle: _.throttle,
     timer: _timer,
     times: _.times,

src/utilities/do-try.test.ts

@@ -7,11 +7,33 @@ import {
 import { Do, DoSync } from "../utilities/do-try";
 import { PolyfillUtils } from "../utilities/polyfill-utils";
 import { StubResourceRecord } from "../tests/stubs/stub-resource-record";
+import { CoreUtils } from "../utilities/core-utils";
 
 PolyfillUtils.registerPromiseFinallyPolyfill();
 
 describe("do-try.ts", () => {
     describe("Do.try", () => {
+        it("When a catch handler exists, finally is called after catch resolves", async () => {
+            // Arrange
+            let catchRanAtTimestamp: number;
+            let finallyRanAtTimestamp: number;
+            const workload = async () => {
+                throw Error();
+            };
+
+            // Act
+            await Do.try(workload)
+                .catch(() => {
+                    catchRanAtTimestamp = Date.now();
+                    CoreUtils.sleepSync(1000);
+                })
+                .finally(() => (finallyRanAtTimestamp = Date.now()))
+                .getAwaiter();
+
+            // Assert
+            expect(catchRanAtTimestamp).toBeLessThan(finallyRanAtTimestamp);
+        });
+
         it("When validation errors occur (i.e. error is a ResultRecord), then passes typed errors to catch handler", async () => {
             // Arrange
             const workload = async () => {
@@ -54,7 +76,7 @@ describe("do-try.ts", () => {
         it("When no errors occur, then catch handler is never called", async () => {
             // Arrange
             const catchHandler = jest.fn();
-            const workload = jest.fn();
+            const workload = async () => {};
 
             // Act
             await Do.try(workload)
@@ -68,7 +90,7 @@ describe("do-try.ts", () => {
         it("When no errors occur, then finally handler is still called", async () => {
             // Arrange
             const finallyHandler = jest.fn();
-            const workload = jest.fn();
+            const workload = async () => {};
 
             // Act
             await Do.try(workload)

src/utilities/do-try.ts

@@ -17,10 +17,10 @@ import {
  * - error?: any -- if it's a Javascript error, will not be null
  */
 class Do<TResourceType, TReturnVal = void> {
-    private readonly promise: Promise<TReturnVal>;
+    private promise: Promise<TReturnVal>;
 
     private constructor(workload: AsyncWorkload<TReturnVal>) {
-        this.promise = Promise.resolve(workload());
+        this.promise = workload();
     }
 
     /**
@@ -35,14 +35,14 @@ class Do<TResourceType, TReturnVal = void> {
     public catch(
         errorHandler: CatchHandler<TResourceType>
     ): Do<TResourceType, TReturnVal> {
-        this.promise.catch((err: any) => {
+        this.promise = this.promise.catch((err: any) => {
             if (err instanceof ResultRecord) {
                 errorHandler(err, undefined);
                 return;
             }
 
             errorHandler(undefined, err);
-        });
+        }) as Promise<TReturnVal>;
 
         return this;
     }
@@ -56,7 +56,7 @@ class Do<TResourceType, TReturnVal = void> {
     public finally(
         finallyHandler: FinallyHandler
     ): Do<TResourceType, TReturnVal> {
-        this.promise.finally(finallyHandler);
+        this.promise = this.promise.finally(finallyHandler);
         return this;
     }