Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions lib/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,18 @@ export class KeychainUnavailableError extends Error {
}
}

export class ServerError extends Error {
statusCode: number;
constructor(statusCode: number, message: string) {
super(message);
this.name = "ServerError";
this.statusCode = statusCode;
}
}

const REQUEST_TIMEOUT_MS = 30_000;
const RETRY_BASE_DELAY_MS = 1_000;
const MAX_RETRY_DELAY_MS = 30_000;

export const request = async <T>(
url: string,
Expand Down Expand Up @@ -51,6 +62,10 @@ export const request = async <T>(
console.info("HTTP request", details);
throw new UnauthorizedError("Unauthorized");
}
if (response.status >= 500) {
console.info("HTTP request", { ...details, error: "Server error" });
throw new ServerError(response.status, `Request failed: ${response.status}`);
}
if (!response.ok) {
const error =
response.status === 403
Expand Down Expand Up @@ -116,6 +131,15 @@ export const useAPIRequest = <TResponse, TData = TResponse>(
if (typeof callerRetry === "number") return failureCount < callerRetry;
return callerRetry(failureCount, error);
},
retryDelay: (attemptIndex, error) => {
if (error instanceof ServerError) {
return Math.min(RETRY_BASE_DELAY_MS * 2 ** attemptIndex, MAX_RETRY_DELAY_MS);
}
const callerRetryDelay = options.retryDelay;
if (typeof callerRetryDelay === "function") return callerRetryDelay(attemptIndex, error);
if (typeof callerRetryDelay === "number") return callerRetryDelay;
return Math.min(RETRY_BASE_DELAY_MS * 2 ** attemptIndex, MAX_RETRY_DELAY_MS);
},
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Comment thread
greptile-apps[bot] marked this conversation as resolved.
enabled: !!accessToken && (options.enabled ?? true),
});
};
27 changes: 23 additions & 4 deletions tests/lib/request.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { request, UnauthorizedError } from "@/lib/request";
import { request, ServerError, UnauthorizedError } from "@/lib/request";

const mockFetch = jest.fn();
global.fetch = mockFetch;
Expand Down Expand Up @@ -69,10 +69,29 @@ describe("request", () => {
await expect(request("https://api.example.com/test")).rejects.toThrow("Request failed: 404 Not found");
});

it("throws on non-ok responses", async () => {
it("throws ServerError on 5xx responses", async () => {
mockFetch.mockReturnValueOnce(jsonResponse({ error: "bad" }, 500));
await expect(request("https://api.example.com/test")).rejects.toThrow("Request failed: 500");
expect(mockFetch).toHaveBeenCalledTimes(1);
const thrown = (await request("https://api.example.com/test").catch((e) => e)) as ServerError;
expect(thrown).toBeInstanceOf(ServerError);
expect(thrown.statusCode).toBe(500);
expect(thrown.message).toBe("Request failed: 500");
});

it("throws ServerError on 502 with HTML body without including the body in the message", async () => {
const cloudflareHtml = "<html><body>Ran out of time — we weren't able to render the page in time</body></html>";
mockFetch.mockReturnValueOnce(
Promise.resolve({
ok: false,
status: 502,
json: () => Promise.resolve({}),
text: () => Promise.resolve(cloudflareHtml),
}),
);
const thrown = (await request("https://api.example.com/test").catch((e) => e)) as ServerError;
expect(thrown).toBeInstanceOf(ServerError);
expect(thrown.statusCode).toBe(502);
expect(thrown.message).toBe("Request failed: 502");
expect(thrown.message).not.toContain("Ran out of time");
});

it("aborts the request after 30s timeout", async () => {
Expand Down
124 changes: 116 additions & 8 deletions tests/lib/use-api-request.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { renderHook, waitFor } from "@testing-library/react-native";
import React from "react";

import { KeychainUnavailableError, UnauthorizedError, useAPIRequest } from "@/lib/request";
import { KeychainUnavailableError, ServerError, UnauthorizedError, useAPIRequest } from "@/lib/request";

const mockRefreshToken = jest.fn();
const mockLogout = jest.fn();
Expand Down Expand Up @@ -64,6 +64,20 @@ const renderUseAPIRequestWithCallerRetry = (
{ wrapper: createWrapper(false) },
);

const renderUseAPIRequestWithCallerRetryDelay = (
callerRetryDelay: number | ((attemptIndex: number, error: Error) => number),
) =>
renderHook(
() =>
useAPIRequest<{ ok: boolean }>({
url: "/test",
queryKey: ["test"],
retry: 2,
retryDelay: callerRetryDelay,
}),
{ wrapper: createWrapper(false) },
);

const authHeaderOf = (call: unknown[]): string | undefined => {
const init = call[1] as RequestInit | undefined;
const headers = init?.headers as Record<string, string> | undefined;
Expand All @@ -90,9 +104,7 @@ describe("useAPIRequest", () => {
});

it("attempts refresh on 401 and retries with the new token", async () => {
mockFetch
.mockResolvedValueOnce(jsonResponse({}, 401))
.mockResolvedValueOnce(jsonResponse({ ok: true }));
mockFetch.mockResolvedValueOnce(jsonResponse({}, 401)).mockResolvedValueOnce(jsonResponse({ ok: true }));
mockRefreshToken.mockResolvedValueOnce("fresh-token");

const { result } = renderUseAPIRequest();
Expand Down Expand Up @@ -159,16 +171,17 @@ describe("useAPIRequest", () => {
});

it("propagates a transient 5xx on retry without logging out", async () => {
mockFetch
.mockResolvedValueOnce(jsonResponse({}, 401))
.mockResolvedValue(jsonResponse({ error: "boom" }, 503));
jest.useFakeTimers();
mockFetch.mockResolvedValueOnce(jsonResponse({}, 401)).mockResolvedValue(jsonResponse({ error: "boom" }, 503));
mockRefreshToken.mockResolvedValueOnce("fresh-token");

const { result } = renderUseAPIRequest();

await jest.advanceTimersByTimeAsync(10_000);
await waitFor(() => expect(result.current.isError).toBe(true));
expect(result.current.error?.message).toMatch(/503/);
expect(mockLogout).not.toHaveBeenCalled();
jest.useRealTimers();
});

it("under production retry:2, a scope-stuck 401 triggers refresh exactly once (no extra rotations)", async () => {
Expand All @@ -187,17 +200,20 @@ describe("useAPIRequest", () => {
});

it("under production retry:2, a transient 5xx still retries (auth-only opt-out)", async () => {
jest.useFakeTimers();
mockFetch
.mockResolvedValueOnce(jsonResponse({ error: "boom" }, 500))
.mockResolvedValueOnce(jsonResponse({ error: "boom" }, 500))
.mockResolvedValueOnce(jsonResponse({ ok: true }));

const { result } = renderUseAPIRequest(2);

await jest.advanceTimersByTimeAsync(10_000);
await waitFor(() => expect(result.current.isSuccess).toBe(true));
expect(mockFetch.mock.calls.length).toBeGreaterThanOrEqual(3);
expect(mockRefreshToken).not.toHaveBeenCalled();
expect(mockLogout).not.toHaveBeenCalled();
jest.useRealTimers();
});

it("under production retry:2, a KeychainUnavailableError path does not retry-loop on a locked keychain", async () => {
Expand All @@ -213,13 +229,16 @@ describe("useAPIRequest", () => {
});

it("does not attempt refresh for non-UnauthorizedError failures", async () => {
mockFetch.mockResolvedValueOnce(jsonResponse({ error: "boom" }, 500));
jest.useFakeTimers();
mockFetch.mockResolvedValue(jsonResponse({ error: "boom" }, 500));

const { result } = renderUseAPIRequest();

await jest.advanceTimersByTimeAsync(10_000);
await waitFor(() => expect(result.current.isError).toBe(true));
expect(mockRefreshToken).not.toHaveBeenCalled();
expect(mockLogout).not.toHaveBeenCalled();
jest.useRealTimers();
});

it("ignores a caller-supplied retry:true for UnauthorizedError (auth opt-out wins)", async () => {
Expand All @@ -244,16 +263,105 @@ describe("useAPIRequest", () => {
});

it("ignores a caller-supplied retry function for UnauthorizedError but consults it for other errors", async () => {
jest.useFakeTimers();
mockFetch
.mockResolvedValueOnce(jsonResponse({ error: "boom" }, 500))
.mockResolvedValueOnce(jsonResponse({ ok: true }));
const callerRetry = jest.fn<boolean, [number, Error]>(() => true);

const { result } = renderUseAPIRequestWithCallerRetry(callerRetry);

await jest.advanceTimersByTimeAsync(5_000);
await waitFor(() => expect(result.current.isSuccess).toBe(true));
expect(callerRetry).toHaveBeenCalled();
const errorArg = callerRetry.mock.calls[0]?.[1];
expect(errorArg?.message).toMatch(/500/);
jest.useRealTimers();
});

it("throws ServerError for 5xx responses", async () => {
jest.useFakeTimers();
mockFetch.mockResolvedValue(jsonResponse({ error: "boom" }, 502));

const { result } = renderUseAPIRequest();

await jest.advanceTimersByTimeAsync(10_000);
await waitFor(() => expect(result.current.isError).toBe(true));
expect(result.current.error).toBeInstanceOf(ServerError);
expect((result.current.error as ServerError).statusCode).toBe(502);
jest.useRealTimers();
});

it("applies exponential backoff delay for ServerError retries", async () => {
jest.useFakeTimers();
mockFetch
.mockResolvedValueOnce(jsonResponse({}, 502))
.mockResolvedValueOnce(jsonResponse({}, 502))
.mockResolvedValueOnce(jsonResponse({ ok: true }));

const { result } = renderUseAPIRequest(3);

await jest.advanceTimersByTimeAsync(500);
expect(result.current.isSuccess).toBe(false);

await jest.advanceTimersByTimeAsync(1_000);
expect(mockFetch).toHaveBeenCalledTimes(2);

await jest.advanceTimersByTimeAsync(2_000);
await waitFor(() => expect(result.current.isSuccess).toBe(true));
expect(mockFetch).toHaveBeenCalledTimes(3);
jest.useRealTimers();
});

it("applies default exponential backoff delay for non-ServerError retries", async () => {
jest.useFakeTimers();
mockFetch
.mockRejectedValueOnce(new Error("Network request failed"))
.mockResolvedValueOnce(jsonResponse({ ok: true }));

const { result } = renderUseAPIRequest(2);

await jest.advanceTimersByTimeAsync(500);
expect(mockFetch).toHaveBeenCalledTimes(1);

await jest.advanceTimersByTimeAsync(500);
await waitFor(() => expect(result.current.isSuccess).toBe(true));
expect(mockFetch).toHaveBeenCalledTimes(2);
jest.useRealTimers();
});

it("uses caller-supplied numeric retryDelay for non-ServerError retries", async () => {
jest.useFakeTimers();
mockFetch
.mockRejectedValueOnce(new Error("Network request failed"))
.mockResolvedValueOnce(jsonResponse({ ok: true }));

const { result } = renderUseAPIRequestWithCallerRetryDelay(3_000);

await jest.advanceTimersByTimeAsync(2_999);
expect(mockFetch).toHaveBeenCalledTimes(1);

await jest.advanceTimersByTimeAsync(1);
await waitFor(() => expect(result.current.isSuccess).toBe(true));
expect(mockFetch).toHaveBeenCalledTimes(2);
jest.useRealTimers();
});

it("uses caller-supplied retryDelay function for non-ServerError retries", async () => {
jest.useFakeTimers();
const networkError = new Error("Network request failed");
const callerRetryDelay = jest.fn<number, [number, Error]>(() => 3_000);
mockFetch.mockRejectedValueOnce(networkError).mockResolvedValueOnce(jsonResponse({ ok: true }));

const { result } = renderUseAPIRequestWithCallerRetryDelay(callerRetryDelay);

await jest.advanceTimersByTimeAsync(2_999);
expect(mockFetch).toHaveBeenCalledTimes(1);
expect(callerRetryDelay).toHaveBeenCalledWith(0, networkError);

await jest.advanceTimersByTimeAsync(1);
await waitFor(() => expect(result.current.isSuccess).toBe(true));
expect(mockFetch).toHaveBeenCalledTimes(2);
jest.useRealTimers();
});
});
Loading