diff --git a/src/frontend/src/controllers/API/queries/deployment-provider-accounts/use-delete-provider-account.ts b/src/frontend/src/controllers/API/queries/deployment-provider-accounts/use-delete-provider-account.ts index e23d33c527..b0741fedcd 100644 --- a/src/frontend/src/controllers/API/queries/deployment-provider-accounts/use-delete-provider-account.ts +++ b/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); diff --git a/src/frontend/src/controllers/API/queries/deployments/use-delete-deployment.ts b/src/frontend/src/controllers/API/queries/deployments/use-delete-deployment.ts index a7a679eeb0..598b137572 100644 --- a/src/frontend/src/controllers/API/queries/deployments/use-delete-deployment.ts +++ b/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); diff --git a/src/frontend/src/controllers/API/queries/deployments/use-patch-deployment.ts b/src/frontend/src/controllers/API/queries/deployments/use-patch-deployment.ts index 968e7c9c1c..586dc86c94 100644 --- a/src/frontend/src/controllers/API/queries/deployments/use-patch-deployment.ts +++ b/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({ diff --git a/src/frontend/src/controllers/API/queries/deployments/use-patch-snapshot.ts b/src/frontend/src/controllers/API/queries/deployments/use-patch-snapshot.ts index 22a024797c..55810f7a04 100644 --- a/src/frontend/src/controllers/API/queries/deployments/use-patch-snapshot.ts +++ b/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); diff --git a/src/frontend/src/controllers/API/queries/deployments/use-post-deployment-run.ts b/src/frontend/src/controllers/API/queries/deployments/use-post-deployment-run.ts index f26422407c..f11852967c 100644 --- a/src/frontend/src/controllers/API/queries/deployments/use-post-deployment-run.ts +++ b/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); }; diff --git a/src/frontend/src/controllers/API/queries/deployments/use-post-deployment.ts b/src/frontend/src/controllers/API/queries/deployments/use-post-deployment.ts index 52e2fed23b..ef32797c08 100644 --- a/src/frontend/src/controllers/API/queries/deployments/use-post-deployment.ts +++ b/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"] }); }, diff --git a/src/frontend/src/controllers/API/services/__tests__/request-processor.test.ts b/src/frontend/src/controllers/API/services/__tests__/request-processor.test.ts new file mode 100644 index 0000000000..00a401723b --- /dev/null +++ b/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); + }); +}); diff --git a/src/frontend/src/controllers/API/services/request-processor.ts b/src/frontend/src/controllers/API/services/request-processor.ts index 41e9c19a88..6b54d9ef1e 100644 --- a/src/frontend/src/controllers/API/services/request-processor.ts +++ b/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, }); }