fix(property-provider): stop memoizing rejected provider (#2680)

aws · Aug 17, 2021 · fac5f27 · fac5f27
1 parent cbdcd9c
commit fac5f27
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 17 deletions.
diff --git a/packages/property-provider/src/memoize.spec.ts b/packages/property-provider/src/memoize.spec.ts
@@ -20,22 +20,28 @@ describe("memoize", () => {
       const memoized = memoize(provider);
       expect(provider).toHaveBeenCalledTimes(0);
       // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      for (const index in [...Array(repeatTimes).keys()]) {
+      for (const _ in [...Array(repeatTimes).keys()]) {
         expect(await memoized()).toStrictEqual(mockReturn);
         expect(provider).toHaveBeenCalledTimes(1);
       }
     });
 
-    it("should always return the same promise", () => {
-      expect.assertions(repeatTimes * 2);
-
+    it("should retry provider if previous provider is failed", async () => {
+      provider
+        .mockReset()
+        .mockRejectedValueOnce("Error")
+        .mockResolvedValueOnce("Retry")
+        .mockRejectedValueOnce("Should not call 3rd time");
       const memoized = memoize(provider);
-      const result = memoized();
-      // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      for (const index in [...Array(repeatTimes).keys()]) {
-        expect(memoized()).toStrictEqual(result);
-        expect(provider).toHaveBeenCalledTimes(1);
+      try {
+        await memoized();
+        fail();
+      } catch (e) {
+        expect(e).toBe("Error");
       }
+      expect(await memoized()).toBe("Retry");
+      expect(await memoized()).toBe("Retry");
+      expect(provider).toBeCalledTimes(2);
     });
   });
 
@@ -124,5 +130,24 @@ describe("memoize", () => {
         return requiresRefreshFalseTest();
       });
     });
+
+    it("should retry provider if previous provider is failed", async () => {
+      provider
+        .mockReset()
+        .mockRejectedValueOnce("Error")
+        .mockResolvedValueOnce("Retry")
+        .mockRejectedValueOnce("Should not call 3rd time");
+      isExpired.mockReset().mockReturnValue(false);
+      const memoized = memoize(provider, isExpired);
+      try {
+        await memoized();
+        fail();
+      } catch (e) {
+        expect(e).toBe("Error");
+      }
+      expect(await memoized()).toBe("Retry");
+      expect(await memoized()).toBe("Retry");
+      expect(provider).toBeCalledTimes(2);
+    });
   });
 });
diff --git a/packages/property-provider/src/memoize.ts b/packages/property-provider/src/memoize.ts
@@ -45,37 +45,36 @@ export const memoize: MemoizeOverload = <T>(
   isExpired?: (resolved: T) => boolean,
   requiresRefresh?: (resolved: T) => boolean
 ): Provider<T> => {
-  let result: any;
+  let resolved: any;
   let hasResult: boolean;
   if (isExpired === undefined) {
     // This is a static memoization; no need to incorporate refreshing
-    return () => {
+    return async () => {
       if (!hasResult) {
-        result = provider();
+        resolved = await provider();
         hasResult = true;
       }
-      return result;
+      return resolved;
     };
   }
 
   let isConstant = false;
 
   return async () => {
     if (!hasResult) {
-      result = provider();
+      resolved = await provider();
       hasResult = true;
     }
     if (isConstant) {
-      return result;
+      return resolved;
     }
 
-    const resolved = await result;
     if (requiresRefresh && !requiresRefresh(resolved)) {
       isConstant = true;
       return resolved;
     }
     if (isExpired(resolved)) {
-      return (result = provider());
+      return (resolved = await provider());
     }
     return resolved;
   };