fix(sdk): static question should cancel requests on component unmount (#48808)

* cancel request on unmount in static question

* use deferred promise

* remove logs

* alias cancelled to deferred.promise

* extract useLoadStaticQuestion hook

* add tests for request cancellation

* change updateQuestion method

* add sanity check with fetchMock

enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.tsx

 import cx from "classnames";
-import { useEffect, useState } from "react";
 import { t } from "ttag";
 
 import {
@@ -7,11 +6,10 @@ import {
   SdkLoader,
   withPublicComponentWrapper,
 } from "embedding-sdk/components/private/PublicComponentWrapper";
+import { useLoadStaticQuestion } from "embedding-sdk/hooks/private/use-load-static-question";
 import { getDefaultVizHeight } from "embedding-sdk/lib/default-height";
-import { loadStaticQuestion } from "embedding-sdk/lib/load-static-question";
 import CS from "metabase/css/core/index.css";
 import { useValidatedEntityId } from "metabase/lib/entity-id/hooks/use-validated-entity-id";
-import type { GenericErrorResponse } from "metabase/lib/errors";
 import { getResponseErrorMessage } from "metabase/lib/errors";
 import { useSelector } from "metabase/lib/redux";
 import QueryVisualization from "metabase/query_builder/components/QueryVisualization";
@@ -23,7 +21,7 @@ import { getMetadata } from "metabase/selectors/metadata";
 import { Box, Group } from "metabase/ui";
 import { PublicMode } from "metabase/visualizations/click-actions/modes/PublicMode";
 import Question from "metabase-lib/v1/Question";
-import type { Card, CardEntityId, CardId, Dataset } from "metabase-types/api";
+import type { CardEntityId, CardId, Dataset } from "metabase-types/api";
 
 export type StaticQuestionProps = {
   questionId: CardId | CardEntityId;
@@ -32,13 +30,6 @@ export type StaticQuestionProps = {
   parameterValues?: Record<string, string | number>;
 };
 
-type State = {
-  loading: boolean;
-  card: Card | null;
-  result: Dataset | null;
-  error: GenericErrorResponse | null;
-};
-
 type StaticQuestionVisualizationSelectorProps = {
   question: Question;
   result: Dataset | null;
@@ -87,63 +78,8 @@ const StaticQuestionInner = ({
 
   const metadata = useSelector(getMetadata);
 
-  const [{ loading, card, result, error }, setState] = useState<State>({
-    loading: false,
-    card: null,
-    result: null,
-    error: null,
-  });
-
-  useEffect(() => {
-    async function loadCardData() {
-      setState(prevState => ({
-        ...prevState,
-        loading: true,
-      }));
-
-      if (!questionId) {
-        return;
-      }
-
-      try {
-        const { card, result } = await loadStaticQuestion({
-          questionId,
-          parameterValues,
-        });
-
-        setState(prevState => ({
-          ...prevState,
-          card,
-          result,
-          loading: false,
-          error: null,
-        }));
-      } catch (error) {
-        if (typeof error === "object") {
-          setState(prevState => ({
-            ...prevState,
-            result: null,
-            card: null,
-            loading: false,
-            error,
-          }));
-        } else {
-          console.error("error loading static question", error);
-        }
-      }
-    }
-
-    loadCardData();
-  }, [questionId, parameterValues]);
-
-  const changeVisualization = (newQuestion: Question) => {
-    setState({
-      card: newQuestion.card(),
-      result: result,
-      loading: false,
-      error: null,
-    });
-  };
+  const { card, loading, result, error, updateQuestion } =
+    useLoadStaticQuestion(questionId, parameterValues);
 
   const isLoading = loading || (!result && !error) || isValidatingEntityId;
 
@@ -173,7 +109,7 @@ const StaticQuestionInner = ({
           <StaticQuestionVisualizationSelector
             question={question}
             result={result}
-            onUpdateQuestion={changeVisualization}
+            onUpdateQuestion={updateQuestion}
           />
         )}
         <QueryVisualization

enterprise/frontend/src/embedding-sdk/components/public/StaticQuestion/StaticQuestion.unit.spec.tsx

+import { act } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import fetchMock from "fetch-mock";
 
@@ -85,7 +86,7 @@ const setup = ({
 
   setupCardQueryEndpoints(card, TEST_DATASET);
 
-  renderWithProviders(
+  return renderWithProviders(
     <StaticQuestion
       questionId={TEST_QUESTION_ID}
       showVisualizationSelector={showVisualizationSelector}
@@ -174,4 +175,19 @@ describe("StaticQuestion", () => {
       value: 1024,
     });
   });
+
+  it("should cancel the request when the component unmounts", async () => {
+    const abortSpy = jest.spyOn(AbortController.prototype, "abort");
+
+    const { unmount } = setup();
+
+    await act(async () => unmount());
+
+    expect(abortSpy).toHaveBeenCalled();
+    abortSpy.mockRestore();
+
+    // sanity check that the two requests were made initially
+    expect(fetchMock.calls(`path:/api/card/1`).length).toBe(1);
+    expect(fetchMock.calls(`path:/api/card/1/query`).length).toBe(1);
+  });
 });

enterprise/frontend/src/embedding-sdk/hooks/private/use-load-static-question.ts

+import { useEffect, useState } from "react";
+
+import { loadStaticQuestion } from "embedding-sdk/lib/load-static-question";
+import type { GenericErrorResponse } from "metabase/lib/errors";
+import { defer } from "metabase/lib/promise";
+import type Question from "metabase-lib/v1/Question";
+import type { Card, Dataset } from "metabase-types/api";
+
+type QuestionState = {
+  loading: boolean;
+  card: Card | null;
+  result: Dataset | null;
+  error: GenericErrorResponse | null;
+};
+
+export function useLoadStaticQuestion(
+  questionId: number | null,
+  parameterValues?: Record<string, string | number>,
+) {
+  const [questionState, setQuestionState] = useState<QuestionState>({
+    loading: false,
+    card: null,
+    result: null,
+    error: null,
+  });
+
+  const updateQuestion = (newQuestion: Question) =>
+    setQuestionState(state => ({
+      ...state,
+      card: newQuestion.card(),
+      loading: false,
+      error: null,
+    }));
+
+  useEffect(() => {
+    const cancelDeferred = defer();
+
+    async function loadCardData() {
+      setQuestionState(state => ({ ...state, loading: true }));
+
+      if (!questionId) {
+        return;
+      }
+
+      try {
+        const { card, result } = await loadStaticQuestion({
+          questionId,
+          parameterValues,
+          cancelDeferred,
+        });
+
+        setQuestionState({
+          card,
+          result,
+          loading: false,
+          error: null,
+        });
+      } catch (error) {
+        if (typeof error === "object") {
+          setQuestionState({
+            result: null,
+            card: null,
+            loading: false,
+            error,
+          });
+        } else {
+          console.error("error loading static question", error);
+        }
+      }
+    }
+
+    loadCardData();
+
+    return () => {
+      // cancel pending requests upon unmount
+      cancelDeferred.resolve();
+    };
+  }, [questionId, parameterValues]);
+
+  return { ...questionState, updateQuestion };
+}

enterprise/frontend/src/embedding-sdk/lib/load-static-question.ts

+import type { Deferred } from "metabase/lib/promise";
 import { CardApi } from "metabase/services";
 import type { Card, Dataset, ParameterQueryObject } from "metabase-types/api";
 
 interface Options {
   questionId: number;
   parameterValues?: Record<string, string | number>;
+  cancelDeferred?: Deferred;
 }
 
 type ParameterQueryInput = { id: string } & ParameterQueryObject;
 
 export async function loadStaticQuestion(options: Options) {
-  const { questionId, parameterValues } = options;
+  const { questionId, parameterValues, cancelDeferred } = options;
 
   let card: Card | null;
   let result: Dataset | null;
 
+  const cancelled = cancelDeferred?.promise;
+
   [card, result] = await Promise.all([
-    CardApi.get({ cardId: questionId }),
+    CardApi.get({ cardId: questionId }, { cancelled }),
 
     // Query the card in parallel when no parameters are provided.
-    !parameterValues && CardApi.query({ cardId: questionId }),
+    !parameterValues && CardApi.query({ cardId: questionId }, { cancelled }),
   ]);
 
   if (parameterValues && card?.parameters) {
@@ -31,10 +35,10 @@ export async function loadStaticQuestion(options: Options) {
         value: parameterValues[parameter.slug],
       }));
 
-    result = await CardApi.query({
-      cardId: questionId,
-      parameters,
-    });
+    result = await CardApi.query(
+      { cardId: questionId, parameters },
+      { cancelled },
+    );
   }
 
   return { card, result };