feat: disable frontend retries for client errors (#12890)

* checkout fe disable retries files from fe-retries branch

* refactor(frontend): tighten retry helpers and broaden their tests
Address review feedback on the retry rework in UseRequestProcessor:
- Use axios.isAxiosError as a type guard instead of an `as AxiosError`
  cast, so non-axios errors (TypeErrors thrown by queryFn, etc.) are
  classified by structure rather than by an unsafe shape assertion.
- Hoist makeRetry(5) / makeRetry(3) to module scope as queryRetry /
  mutationRetry. They're pure factories and don't need to be rebuilt on
  every render of the hook; matches how retryDelay was already scoped.
- Honor a caller-provided options.retryDelay on the mutate path
  (`options.retryDelay ?? retryDelay`). Previously the explicit
  retryDelay was set after `...options`, silently clobbering any
  override — inconsistent with the query path where `...options` wins.
Tests:
- Rename capture vars to mockCapturedQueryOptions /
  mockCapturedMutationOptions so they comply with jest's mock-prefix
  hoisting rule for variables referenced from a jest.mock factory.
- Have setup() throw a clear error if the wrapped hook was never called,
  instead of NPE-ing on `const { retry } = null`.
- Mark axios fixtures with `isAxiosError: true` so they actually pass
  the new type guard. Add an explicit axiosNetworkError() helper and a
  nonAxiosError() case to lock in the "non-axios = transient = retry"
  branch.
- Add coverage for caller-provided options.retryDelay on both query and
  mutate paths to prevent the previous regression from coming back.

src/frontend/src/controllers/API/queries/deployment-provider-accounts/use-delete-provider-account.ts

@@ -19,10 +19,8 @@ export const useDeleteProviderAccount: useMutationFunctionType<
     );
   };
 
-  // TODO: Add retries for transient server-side errors (5xx, timeouts).
   return mutate(["useDeleteProviderAccount"], fn, {
     ...options,
-    retry: false,
     onSuccess: (...args) => {
       queryClient.refetchQueries({ queryKey: ["useGetProviderAccounts"] });
       options?.onSuccess?.(...args);

src/frontend/src/controllers/API/queries/deployments/use-delete-deployment.ts

@@ -17,10 +17,8 @@ export const useDeleteDeployment: useMutationFunctionType<
     await api.delete(`${getURL("DEPLOYMENTS")}/${deployment_id}`);
   };
 
-  // TODO: Add retries for transient server-side errors (5xx, timeouts).
   return mutate(["useDeleteDeployment"], fn, {
     ...options,
-    retry: false,
     onSuccess: (...args) => {
       queryClient.refetchQueries({ queryKey: ["useGetDeployments"] });
       options?.onSuccess?.(...args);

src/frontend/src/controllers/API/queries/deployments/use-patch-deployment.ts

@@ -50,10 +50,8 @@ export const usePatchDeployment: useMutationFunctionType<
     return res.data;
   };
 
-  // TODO: Add retries for transient server-side errors (5xx, timeouts).
   return mutate(["usePatchDeployment"], fn, {
     ...options,
-    retry: false,
     onSuccess: (...args) => {
       queryClient.refetchQueries({ queryKey: ["useGetDeployments"] });
       queryClient.removeQueries({

src/frontend/src/controllers/API/queries/deployments/use-patch-snapshot.ts

@@ -26,10 +26,8 @@ export const usePatchSnapshot: useMutationFunctionType<
     return data;
   };
 
-  // TODO: Add retries for transient server-side errors (5xx, timeouts).
   return mutate(["usePatchSnapshot"], fn, {
     ...options,
-    retry: false,
     onSuccess: (...args) => {
       queryClient.refetchQueries({ queryKey: ["useGetDeployments"] });
       options?.onSuccess?.(...args);

src/frontend/src/controllers/API/queries/deployments/use-post-deployment-run.ts

@@ -47,6 +47,5 @@ export const usePostDeploymentRun: useMutationFunctionType<
     return res.data;
   };
 
-  // TODO: Add retries for transient server-side errors (5xx, timeouts).
-  return mutate(["usePostDeploymentRun"], fn, { ...options, retry: false });
+  return mutate(["usePostDeploymentRun"], fn, options);
 };

src/frontend/src/controllers/API/queries/deployments/use-post-deployment.ts

@@ -53,10 +53,8 @@ export const usePostDeployment: useMutationFunctionType<
     return res.data;
   };
 
-  // TODO: Add retries for transient server-side errors (5xx, timeouts).
   return mutate(["usePostDeployment"], fn, {
     ...options,
-    retry: false,
     onSuccess: () => {
       return queryClient.refetchQueries({ queryKey: ["useGetDeployments"] });
     },

src/frontend/src/controllers/API/services/__tests__/request-processor.test.ts

@@ -0,0 +1,189 @@
+// biome-ignore-all lint/suspicious/noExplicitAny: test mocks
+const mockQueryClient = {
+  invalidateQueries: jest.fn(),
+};
+
+let mockCapturedQueryOptions: any = null;
+let mockCapturedMutationOptions: any = null;
+
+jest.mock("@tanstack/react-query", () => ({
+  useQueryClient: jest.fn(() => mockQueryClient),
+  useQuery: jest.fn((options: any) => {
+    mockCapturedQueryOptions = options;
+    return { data: undefined, isLoading: false };
+  }),
+  useMutation: jest.fn((options: any) => {
+    mockCapturedMutationOptions = options;
+    return { mutate: jest.fn(), mutateAsync: jest.fn() };
+  }),
+}));
+
+import { UseRequestProcessor } from "../request-processor";
+
+// ---------------------------------------------------------------------------
+// Helpers
+// ---------------------------------------------------------------------------
+
+// `axios.isAxiosError` checks `error.isAxiosError === true`, so fixtures must
+// set that flag to be classified as HTTP errors rather than transient.
+const axiosError = (status: number) => ({
+  isAxiosError: true,
+  response: { status },
+});
+const axiosNetworkError = () => ({ isAxiosError: true, response: undefined });
+const nonAxiosError = () => new Error("Network Error");
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  mockCapturedQueryOptions = null;
+  mockCapturedMutationOptions = null;
+});
+
+// ---------------------------------------------------------------------------
+// Queries: retry policy
+// ---------------------------------------------------------------------------
+
+describe("UseRequestProcessor.query retry policy", () => {
+  const setup = (options: any = {}) => {
+    const { query } = UseRequestProcessor();
+    query(["k"], async () => ({}), options);
+    if (mockCapturedQueryOptions == null) {
+      throw new Error("query was not called by UseRequestProcessor");
+    }
+    return mockCapturedQueryOptions;
+  };
+
+  it("does not retry on any 4xx response", () => {
+    const { retry } = setup();
+    for (const status of [400, 401, 403, 404, 409, 422, 429, 499]) {
+      expect(retry(0, axiosError(status))).toBe(false);
+    }
+  });
+
+  it("retries up to 5 times on 5xx responses", () => {
+    const { retry } = setup();
+    for (const status of [500, 502, 503, 504]) {
+      expect(retry(0, axiosError(status))).toBe(true);
+      expect(retry(4, axiosError(status))).toBe(true);
+      expect(retry(5, axiosError(status))).toBe(false);
+    }
+  });
+
+  it("retries up to 5 times on axios errors with no response", () => {
+    const { retry } = setup();
+    expect(retry(0, axiosNetworkError())).toBe(true);
+    expect(retry(4, axiosNetworkError())).toBe(true);
+    expect(retry(5, axiosNetworkError())).toBe(false);
+  });
+
+  it("treats non-axios / unknown errors as transient (retries)", () => {
+    const { retry } = setup();
+    expect(retry(0, undefined)).toBe(true);
+    expect(retry(0, {})).toBe(true);
+    expect(retry(0, { response: {} })).toBe(true);
+    expect(retry(0, nonAxiosError())).toBe(true);
+  });
+
+  it("allows per-call options.retry to override the default", () => {
+    const { retry } = setup({ retry: false });
+    expect(retry).toBe(false);
+  });
+
+  it("allows per-call options.retry as a number to override the default", () => {
+    const { retry } = setup({ retry: 10 });
+    expect(retry).toBe(10);
+  });
+
+  it("allows per-call options.retryDelay to override the default", () => {
+    const customDelay = jest.fn(() => 42);
+    const { retryDelay } = setup({ retryDelay: customDelay });
+    expect(retryDelay).toBe(customDelay);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Mutations: retry policy
+// ---------------------------------------------------------------------------
+
+describe("UseRequestProcessor.mutate retry policy", () => {
+  const setup = (options: any = {}) => {
+    const { mutate } = UseRequestProcessor();
+    mutate(["k"], async () => ({}), options);
+    if (mockCapturedMutationOptions == null) {
+      throw new Error("mutate was not called by UseRequestProcessor");
+    }
+    return mockCapturedMutationOptions;
+  };
+
+  it("does not retry on any 4xx response", () => {
+    const { retry } = setup();
+    for (const status of [400, 401, 403, 404, 409, 422, 429, 499]) {
+      expect(retry(0, axiosError(status))).toBe(false);
+    }
+  });
+
+  it("retries up to 3 times on 5xx responses", () => {
+    const { retry } = setup();
+    expect(retry(0, axiosError(500))).toBe(true);
+    expect(retry(2, axiosError(500))).toBe(true);
+    expect(retry(3, axiosError(500))).toBe(false);
+  });
+
+  it("retries up to 3 times on axios errors with no response", () => {
+    const { retry } = setup();
+    expect(retry(0, axiosNetworkError())).toBe(true);
+    expect(retry(2, axiosNetworkError())).toBe(true);
+    expect(retry(3, axiosNetworkError())).toBe(false);
+  });
+
+  it("respects options.retry === false", () => {
+    const { retry } = setup({ retry: false });
+    expect(retry).toBe(false);
+  });
+
+  it("respects options.retry === 0 (nullish coalescing, not falsy)", () => {
+    const { retry } = setup({ retry: 0 });
+    expect(retry).toBe(0);
+  });
+
+  it("respects a custom options.retry callback", () => {
+    const customRetry = jest.fn(() => true);
+    const { retry } = setup({ retry: customRetry });
+    expect(retry).toBe(customRetry);
+  });
+
+  it("respects a custom options.retryDelay", () => {
+    const customDelay = jest.fn(() => 42);
+    const { retryDelay } = setup({ retryDelay: customDelay });
+    expect(retryDelay).toBe(customDelay);
+  });
+});
+
+// ---------------------------------------------------------------------------
+// retryDelay: exponential backoff capped at 30s
+// ---------------------------------------------------------------------------
+
+describe("UseRequestProcessor retryDelay", () => {
+  it("uses exponential backoff capped at 30s for queries", () => {
+    const { query } = UseRequestProcessor();
+    query(["k"], async () => ({}));
+    const { retryDelay } = mockCapturedQueryOptions;
+    expect(retryDelay(0)).toBe(1000);
+    expect(retryDelay(1)).toBe(2000);
+    expect(retryDelay(2)).toBe(4000);
+    expect(retryDelay(3)).toBe(8000);
+    expect(retryDelay(4)).toBe(16000);
+    expect(retryDelay(5)).toBe(30000);
+    expect(retryDelay(10)).toBe(30000);
+  });
+
+  it("uses exponential backoff capped at 30s for mutations", () => {
+    const { mutate } = UseRequestProcessor();
+    mutate(["k"], async () => ({}));
+    const { retryDelay } = mockCapturedMutationOptions;
+    expect(retryDelay(0)).toBe(1000);
+    expect(retryDelay(1)).toBe(2000);
+    expect(retryDelay(2)).toBe(4000);
+    expect(retryDelay(10)).toBe(30000);
+  });
+});

src/frontend/src/controllers/API/services/request-processor.ts

@@ -6,11 +6,33 @@ import {
   useQuery,
   useQueryClient,
 } from "@tanstack/react-query";
+import axios from "axios";
 import type {
   MutationFunctionType,
   QueryFunctionType,
 } from "../../../types/api";
 
+// 4xx responses are intentional client-side rejections (auth, validation,
+// deployment guards, etc.) and won't change on retry.
+function isClientError(error: unknown): boolean {
+  if (!axios.isAxiosError(error)) return false;
+  const status = error.response?.status;
+  return typeof status === "number" && status >= 400 && status < 500;
+}
+
+function makeRetry(maxRetries: number) {
+  return (failureCount: number, error: unknown) => {
+    if (isClientError(error)) return false;
+    return failureCount < maxRetries;
+  };
+}
+
+const queryRetry = makeRetry(5);
+const mutationRetry = makeRetry(3);
+
+const retryDelay = (attemptIndex: number) =>
+  Math.min(1000 * 2 ** attemptIndex, 30000);
+
 export function UseRequestProcessor(): {
   query: QueryFunctionType;
   mutate: MutationFunctionType;
@@ -26,8 +48,8 @@ export function UseRequestProcessor(): {
     return useQuery({
       queryKey,
       queryFn,
-      retry: 5,
-      retryDelay: (attemptIndex) => Math.min(1000 * 2 ** attemptIndex, 30000),
+      retry: queryRetry,
+      retryDelay,
       ...options,
     });
   }
@@ -45,8 +67,8 @@ export function UseRequestProcessor(): {
         options.onSettled && options.onSettled(data, error, variables, context);
       },
       ...options,
-      retry: options.retry ?? 3,
-      retryDelay: (attemptIndex) => Math.min(1000 * 2 ** attemptIndex, 30000),
+      retry: options.retry ?? mutationRetry,
+      retryDelay: options.retryDelay ?? retryDelay,
     });
   }