From 04064fe2aaa268f7a80557bcb7e8b87d7416b50c Mon Sep 17 00:00:00 2001
From: Diana Savatina <diana.savvatina@trili.tech>
Date: Mon, 16 Dec 2024 14:49:34 +0000
Subject: [PATCH] fix: avoid double toast by handleAsyncActionUnsafe

---
 .../src/hooks/useAsyncActionHandler.test.ts   | 39 ++++++++++++++++++-
 .../state/src/hooks/useAsyncActionHandler.ts  | 35 ++++++++++-------
 2 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/packages/state/src/hooks/useAsyncActionHandler.test.ts b/packages/state/src/hooks/useAsyncActionHandler.test.ts
index bf5ebb8548..ee7038b37e 100644
--- a/packages/state/src/hooks/useAsyncActionHandler.test.ts
+++ b/packages/state/src/hooks/useAsyncActionHandler.test.ts
@@ -1,3 +1,5 @@
+import { CustomError } from "@umami/utils";
+
 import { useAsyncActionHandler } from "./useAsyncActionHandler";
 import { act, mockToast, renderHook, waitFor } from "../testUtils";
 
@@ -144,12 +146,12 @@ describe("useAsyncActionHandler", () => {
   describe("handleAsyncActionUnsafe", () => {
     it("returns the result of the computation", async () => {
       const view = fixture();
-
       const result = await act(() =>
         view.result.current.handleAsyncActionUnsafe(() => Promise.resolve(42))
       );
 
       expect(result).toBe(42);
+      expect(mockToast).toHaveBeenCalledTimes(0);
     });
 
     it("throws when the computation fails", async () => {
@@ -160,6 +162,41 @@ describe("useAsyncActionHandler", () => {
           view.result.current.handleAsyncActionUnsafe(() => Promise.reject(new Error("test error")))
         )
       ).rejects.toThrow("test error");
+      expect(mockToast).toHaveBeenCalledTimes(2);
+    });
+
+    it("Unsafe propagates the error and shows the toast once on first handling", async () => {
+      const view = fixture();
+
+      expect(mockToast).toHaveBeenCalledTimes(0);
+
+      const error: any = new CustomError("test nested error handling");
+      await expect(
+        act(() => view.result.current.handleAsyncActionUnsafe(() => Promise.reject(error)))
+      ).rejects.toThrow("test nested error handling");
+      // check that error.processed is set to true
+      expect(error.processed).toBe(true);
+      expect(mockToast).toHaveBeenCalledWith({
+        description: "test nested error handling",
+        status: "error",
+        isClosable: true,
+      });
+      expect(mockToast).toHaveBeenCalledTimes(2);
+    });
+
+    it("Unsafe propagates the error and shows no toast on second handling", async () => {
+      const view = fixture();
+
+      expect(mockToast).toHaveBeenCalledTimes(0);
+
+      const error: any = new CustomError("test nested error handling");
+      error.processed = true;
+      await expect(
+        act(() => view.result.current.handleAsyncActionUnsafe(() => Promise.reject(error)))
+      ).rejects.toThrow("test nested error handling");
+      // check that error.processed is still true
+      expect(error.processed).toBe(true);
+      expect(mockToast).toHaveBeenCalledTimes(0);
     });
   });
 });
diff --git a/packages/state/src/hooks/useAsyncActionHandler.ts b/packages/state/src/hooks/useAsyncActionHandler.ts
index e450349c3b..86add7f533 100644
--- a/packages/state/src/hooks/useAsyncActionHandler.ts
+++ b/packages/state/src/hooks/useAsyncActionHandler.ts
@@ -40,22 +40,29 @@ export const useAsyncActionHandler = () => {
       } catch (error: any) {
         const errorContext = getErrorContext(error);
 
-        toast({
-          description: errorContext.description,
-          status: "error",
-          isClosable: true,
-          ...(typeof toastOptions === "function" ? toastOptions(error) : toastOptions),
-        });
+        // Prevent double toast and record of the same error if case of nested handleAsyncActionUnsafe calls.
+        // Still we need to re-throw the error to propagate it to the caller.
+        // There is no problem with handleAsyncAction calls as they stop the propagation of the error.
+        if (!error.processed) {
+          error.processed = true;
 
-        // TODO: fix this dirty hack
-        mockToast({
-          description: errorContext.description,
-          status: "error",
-          isClosable: true,
-          ...(typeof toastOptions === "function" ? toastOptions(error) : toastOptions),
-        });
+          toast({
+            description: errorContext.description,
+            status: "error",
+            isClosable: true,
+            ...(typeof toastOptions === "function" ? toastOptions(error) : toastOptions),
+          });
 
-        dispatch(errorsActions.add(errorContext));
+          // TODO: fix this dirty hack
+          mockToast({
+            description: errorContext.description,
+            status: "error",
+            isClosable: true,
+            ...(typeof toastOptions === "function" ? toastOptions(error) : toastOptions),
+          });
+
+          dispatch(errorsActions.add(errorContext));
+        }
         throw error;
       } finally {
         isLoadingRef.current = false;