Skip to content
Closed
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
73 changes: 69 additions & 4 deletions apps/app/src/hooks/queries/query-helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, it } from "vitest";
import { describe, expect, it, vi } from "vitest";
import type {
ThreadListEntry,
ThreadWithRuntime,
Expand Down Expand Up @@ -38,7 +38,15 @@ import {
import { createQueryClientTestHarness } from "@/test/queryClientTestHarness";
import { HttpError } from "@/lib/api";
import { BbHttpError } from "@/lib/sdk";
import { isTransientReadError, requireEnabledQueryArg } from "./query-helpers";
import {
getTransientReadRetryDelay,
isTransientReadError,
requireEnabledQueryArg,
shouldRetryTransientReadQuery,
TRANSIENT_READ_MAX_ATTEMPTS,
TRANSIENT_READ_RETRY_BASE_DELAY_MS,
TRANSIENT_READ_RETRY_MAX_DELAY_MS,
} from "./query-helpers";

describe("requireEnabledQueryArg", () => {
it("returns the value when present", () => {
Expand Down Expand Up @@ -73,10 +81,10 @@ describe("requireEnabledQueryArg", () => {
});

describe("isTransientReadError", () => {
it("matches browser transport failures but not HTTP responses", () => {
it("matches transport failures and retryable HTTP responses", () => {
expect(isTransientReadError(new TypeError("Failed to fetch"))).toBe(true);
expect(isTransientReadError(new TypeError("Load failed"))).toBe(true);
expect(isTransientReadError({ name: "AbortError" })).toBe(true);
expect(isTransientReadError({ name: "AbortError" })).toBe(false);
expect(
isTransientReadError(
new HttpError({ status: 404, message: "Not found" }),
Expand All @@ -92,12 +100,69 @@ describe("isTransientReadError", () => {
}),
),
).toBe(false);
expect(
isTransientReadError(
new HttpError({
status: 503,
message: "Queue full",
retryable: true,
}),
),
).toBe(true);
expect(
isTransientReadError(
new BbHttpError({
status: 503,
message: "Queue full",
body: { retryable: true },
code: "database_read_unavailable",
retryable: true,
}),
),
).toBe(true);
expect(isTransientReadError(new Error("Unexpected parse error"))).toBe(
false,
);
});
});

describe("read query retry policy", () => {
it("stops after the maximum attempt count", () => {
const error = new HttpError({
status: 503,
message: "Queue full",
retryable: true,
});

expect(
shouldRetryTransientReadQuery(TRANSIENT_READ_MAX_ATTEMPTS - 2, error),
).toBe(true);
expect(
shouldRetryTransientReadQuery(TRANSIENT_READ_MAX_ATTEMPTS - 1, error),
).toBe(false);
});

it("adds jitter to capped exponential delays", () => {
const random = vi.spyOn(Math, "random");
random.mockReturnValue(0);
expect(getTransientReadRetryDelay(0)).toBe(
TRANSIENT_READ_RETRY_BASE_DELAY_MS / 2,
);
expect(getTransientReadRetryDelay(20)).toBe(
TRANSIENT_READ_RETRY_MAX_DELAY_MS / 2,
);

random.mockReturnValue(0.999);
expect(getTransientReadRetryDelay(0)).toBeGreaterThan(
TRANSIENT_READ_RETRY_BASE_DELAY_MS / 2,
);
expect(getTransientReadRetryDelay(20)).toBeLessThan(
TRANSIENT_READ_RETRY_MAX_DELAY_MS,
);
random.mockRestore();
});
});

function makeStatusResponse(
state: WorkspaceStatus["workingTree"]["state"],
): EnvironmentStatusResponse {
Expand Down
23 changes: 17 additions & 6 deletions apps/app/src/hooks/queries/query-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import { BbHttpError } from "@/lib/sdk";
* project-scoped variants so they age out their cached suggestions together.
*/
export const PROMPT_HISTORY_STALE_TIME_MS = 10_000;
export const TRANSIENT_READ_RETRY_COUNT = 2;
export const TRANSIENT_READ_RETRY_DELAY_MS = 250;
export const TRANSIENT_READ_MAX_ATTEMPTS = 5;
export const TRANSIENT_READ_RETRY_BASE_DELAY_MS = 250;
export const TRANSIENT_READ_RETRY_MAX_DELAY_MS = 4_000;

interface RequireEnabledQueryArgArgs<T> {
value: T | null | undefined;
Expand Down Expand Up @@ -43,14 +44,14 @@ function normalizeErrorMessage(message: string): string {

export function isTransientReadError(error: unknown): boolean {
if (error instanceof DOMException && error.name === "AbortError") {
return true;
return false;
}

if (toRecord(error)?.name === "AbortError") {
return true;
return false;
}
if (error instanceof HttpError || error instanceof BbHttpError) {
return false;
return error.retryable;
}

const record = toRecord(error);
Expand All @@ -70,9 +71,19 @@ export function shouldRetryTransientReadQuery(
failureCount: number,
error: unknown,
): boolean {
if (failureCount >= TRANSIENT_READ_RETRY_COUNT) {
if (failureCount >= TRANSIENT_READ_MAX_ATTEMPTS - 1) {
return false;
}

return isTransientReadError(error);
}

export function getTransientReadRetryDelay(attemptIndex: number): number {
const boundedAttemptIndex = Math.max(0, attemptIndex);
const exponentialDelay = Math.min(
TRANSIENT_READ_RETRY_BASE_DELAY_MS * 2 ** boundedAttemptIndex,
TRANSIENT_READ_RETRY_MAX_DELAY_MS,
);
const jitterRange = exponentialDelay / 2;
return Math.floor(jitterRange + Math.random() * jitterRange);
}
138 changes: 138 additions & 0 deletions apps/app/src/hooks/queries/sidebar-navigation-retry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
// @vitest-environment jsdom

import { QueryObserver } from "@tanstack/react-query";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { SidebarBootstrapResponse } from "@bb/server-contract";
import { createAppQueryClient } from "@/lib/query-client";
import { HttpError } from "@/lib/api";
import {
fetchSidebarNavigation,
sidebarNavigationQueryKey,
} from "./sidebar-navigation-query";

const DATABASE_READ_QUEUE_CAPACITY = 32;

function makeSidebarBootstrapResponse(): SidebarBootstrapResponse {
return {
sections: [],
projects: [],
personalProject: {
id: "proj_personal",
kind: "personal",
name: "Personal",
gitRemoteUrl: null,
createdAt: 1,
updatedAt: 1,
sources: [],
threads: [],
defaultExecutionOptions: null,
},
};
}

function jsonResponse(body: unknown, init: ResponseInit): Response {
return new Response(JSON.stringify(body), {
...init,
headers: { "Content-Type": "application/json" },
});
}

afterEach(() => {
vi.restoreAllMocks();
vi.unstubAllGlobals();
});

describe("sidebar navigation retry", () => {
it("recovers after a full database read queue without a reload", async () => {
const sidebar = makeSidebarBootstrapResponse();
let pendingDatabaseReads = DATABASE_READ_QUEUE_CAPACITY;
const fetchMock = vi.fn<typeof fetch>(async () => {
if (pendingDatabaseReads === DATABASE_READ_QUEUE_CAPACITY) {
pendingDatabaseReads--;
return jsonResponse(
{
code: "database_read_unavailable",
message: "The database read queue is full. Try again later.",
retryable: true,
},
{ status: 503 },
);
}
return jsonResponse(sidebar, { status: 200 });
});
vi.stubGlobal("fetch", fetchMock);

const queryClient = createAppQueryClient({
defaultOptions: { queries: { retryDelay: 0 } },
showMutationErrorToasts: false,
});
const queryKey = sidebarNavigationQueryKey();
const observer = new QueryObserver(queryClient, {
queryKey,
queryFn: ({ signal }) => fetchSidebarNavigation(signal),
staleTime: Infinity,
});
const unsubscribe = observer.subscribe(() => {});

await vi.waitFor(() => {
expect(observer.getCurrentResult().isSuccess).toBe(true);
});

expect(fetchMock).toHaveBeenCalledTimes(2);
expect(observer.getCurrentResult().data).toEqual(sidebar);
expect(observer.getCurrentResult().isStale).toBe(false);
expect(queryClient.getQueryData(queryKey)).toEqual(sidebar);

unsubscribe();
queryClient.clear();
});

it.each([
{
name: "a non-retryable server error",
response: () =>
jsonResponse(
{
code: "database_read_unavailable",
message: "Database read failed.",
retryable: false,
},
{ status: 503 },
),
status: 503,
},
{
name: "a client-closed response",
response: () => new Response(null, { status: 499 }),
status: 499,
},
])("does not retry $name", async ({ response, status }) => {
const fetchMock = vi.fn<typeof fetch>(async () => response());
vi.stubGlobal("fetch", fetchMock);

const queryClient = createAppQueryClient({
defaultOptions: { queries: { retryDelay: 0 } },
showMutationErrorToasts: false,
});
const observer = new QueryObserver(queryClient, {
queryKey: ["sidebarNavigation", status],
queryFn: ({ signal }) => fetchSidebarNavigation(signal),
staleTime: Infinity,
});
const unsubscribe = observer.subscribe(() => {});

await vi.waitFor(() => {
expect(observer.getCurrentResult().isError).toBe(true);
});

expect(fetchMock).toHaveBeenCalledTimes(1);
expect(observer.getCurrentResult().error).toBeInstanceOf(HttpError);
expect(observer.getCurrentResult().error).toMatchObject({
retryable: false,
status,
});

unsubscribe();
queryClient.clear();
});
});
8 changes: 4 additions & 4 deletions apps/app/src/hooks/queries/thread-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ import {
resolveThreadTimelinePlaceholder,
} from "./query-placeholders";
import {
getTransientReadRetryDelay,
PROMPT_HISTORY_STALE_TIME_MS,
requireEnabledQueryArg,
shouldRetryTransientReadQuery,
TRANSIENT_READ_RETRY_DELAY_MS,
} from "./query-helpers";
import {
REALTIME_OWNED_MOUNT_BASELINE_QUERY_POLICY,
Expand Down Expand Up @@ -528,7 +528,7 @@ export function useThread(id: string, options?: QueryOptions) {
staleTime: THREAD_DETAIL_STALE_TIME_MS,
refetchOnMount: options?.refetchOnMount ?? true,
retry: shouldRetryTransientReadQuery,
retryDelay: TRANSIENT_READ_RETRY_DELAY_MS,
retryDelay: getTransientReadRetryDelay,
placeholderData: (previousData, previousQuery) =>
resolveThreadPlaceholder(previousData, previousQuery?.queryKey, id) ??
liftThreadListPlaceholder(
Expand Down Expand Up @@ -594,7 +594,7 @@ export function useThreadDetailBootstrap(
enabled,
staleTime: Infinity,
retry: shouldRetryTransientReadQuery,
retryDelay: TRANSIENT_READ_RETRY_DELAY_MS,
retryDelay: getTransientReadRetryDelay,
});
}

Expand Down Expand Up @@ -836,7 +836,7 @@ export function useThreadTimeline(
? {}
: { staleTime: options.staleTime }),
retry: shouldRetryTransientReadQuery,
retryDelay: TRANSIENT_READ_RETRY_DELAY_MS,
retryDelay: getTransientReadRetryDelay,
placeholderData: (previousData, previousQuery) =>
resolveThreadTimelinePlaceholder(
previousData,
Expand Down
8 changes: 8 additions & 0 deletions apps/app/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,21 @@ export class HttpError extends Error {
readonly status: number;
readonly code?: string;
readonly body?: unknown;
readonly retryable: boolean;

constructor(args: {
status: number;
message: string;
code?: string;
body?: unknown;
retryable?: boolean;
}) {
super(`HTTP ${args.status}: ${args.message}`);
this.name = "HttpError";
this.status = args.status;
this.code = args.code;
this.body = args.body;
this.retryable = args.retryable ?? false;
}
}

Expand Down Expand Up @@ -115,6 +118,10 @@ function extractErrorCode(value: unknown): string | undefined {
: undefined;
}

function extractRetryable(value: unknown): boolean {
return toRecord(value)?.retryable === true;
}

async function throwHttpError(res: Response): Promise<never> {
const rawBody = await res.text().catch(() => "");
const contentType = res.headers.get("content-type");
Expand All @@ -130,6 +137,7 @@ async function throwHttpError(res: Response): Promise<never> {
message,
code: extractErrorCode(body),
body,
retryable: extractRetryable(body),
});
}

Expand Down
4 changes: 2 additions & 2 deletions apps/app/src/lib/query-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
import { invalidateActiveThreadBundleQueriesAfterBrowserResume } from "@/hooks/cache-owners/active-thread-lifecycle-cache-owner";
import { cancelActiveQueryFetchesForBrowserSuspend } from "@/hooks/cache-owners/browser-lifecycle-cache-owner";
import {
getTransientReadRetryDelay,
shouldRetryTransientReadQuery,
TRANSIENT_READ_RETRY_DELAY_MS,
} from "@/hooks/queries/query-helpers";

export interface CreateAppQueryClientOptions {
Expand Down Expand Up @@ -146,7 +146,7 @@ export function createAppQueryClient(
staleTime: 2000,
refetchOnWindowFocus: true,
retry: shouldRetryTransientReadQuery,
retryDelay: TRANSIENT_READ_RETRY_DELAY_MS,
retryDelay: getTransientReadRetryDelay,
...defaultOptions?.queries,
},
},
Expand Down
Loading
Loading