Skip to content

Commit 125f373

Browse files
authored
fix(web): back off billing retries instead of hammering a failing transport (#6446)
The packaged client's first open of an unmaterialized team project fires a burst of od:// requests; when that burst hits transient transport failures the proxy answers with synthetic 502s, and the billing read's retry loop made it worse: every mounted consumer armed its own fixed 5s timer, each timer's retry event fanned out to every consumer, and each listener force-refetched past the coalescing cache — one failure became a standing 5s-cadence request storm that fed the very burst it was waiting out (observed by QA as hundreds of repeated billing requests while the left panel span). - Replace the per-hook fixed 5s retry timer with one module-level schedule per requestKey whose delay grows 5s → 10s → 20s → 40s → 60s cap while failures are consecutive; a success resets the count but deliberately leaves a pending timer armed so stale consumers still re-sync. - Stop forcing the retry read. Failures are never cached, so the plain coalesced path is a genuine refetch, and a retry that lands just after a concurrent consumer's success now joins that fresh result instead of evicting it and issuing another request. - Hard-expiry revalidation keeps its forced read via an explicit force flag on the retry event — its cached answer is void by definition, which the plain path cannot know. Red tests first: both new specs fail on the previous implementation (fixed-cadence retry fires at t=10s; retry stampedes a third request past a 0.5s-old success) and pass with the fix. 403 stays fail-closed with no retry; the 83 web test files touching workspace context/billing all pass.
1 parent ad74903 commit 125f373

2 files changed

Lines changed: 248 additions & 24 deletions

File tree

apps/web/src/collab/useWorkspaceContext.ts

Lines changed: 91 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,6 +1009,7 @@ class WorkspaceBillingHttpError extends Error {
10091009
export function resetWorkspaceBillingCache(): void {
10101010
cachedWorkspaceBillingResponses.clear();
10111011
resetWorkspaceBillingInterestRegistry();
1012+
resetWorkspaceBillingRetrySchedules();
10121013
}
10131014

10141015
type BillingInvalidation = Extract<
@@ -1101,7 +1102,6 @@ export function useWorkspaceBillingResponse(
11011102
const activeScopeKeyRef = useRef<string | null>(billingScopeKey);
11021103
const activeRequestKeyRef = useRef<string | null>(billingRequestKey);
11031104
const requestEpochRef = useRef(0);
1104-
const retryTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
11051105
const runtimeManagedRef = useRef(false);
11061106
const interestOwnerIdRef = useRef('');
11071107
if (!interestOwnerIdRef.current) {
@@ -1115,7 +1115,9 @@ export function useWorkspaceBillingResponse(
11151115
return () => {
11161116
mountedRef.current = false;
11171117
requestEpochRef.current += 1;
1118-
if (retryTimerRef.current) clearTimeout(retryTimerRef.current);
1118+
// The retry schedule is module-level and deliberately survives this
1119+
// unmount: another consumer of the same requestKey may still be
1120+
// mounted, and a timer that fires with no listeners is a no-op.
11191121
};
11201122
}, []);
11211123

@@ -1195,10 +1197,7 @@ export function useWorkspaceBillingResponse(
11951197
activeRequestKeyRef.current === requestKey
11961198
) {
11971199
runtimeManagedRef.current = Boolean(response.workspaceRuntime);
1198-
if (retryTimerRef.current) {
1199-
clearTimeout(retryTimerRef.current);
1200-
retryTimerRef.current = null;
1201-
}
1200+
clearWorkspaceBillingRetryFailures(requestKey);
12021201
cachedWorkspaceBillingResponses.set(scopeKey, response);
12031202
setState({ scopeKey, response });
12041203
}
@@ -1231,20 +1230,10 @@ export function useWorkspaceBillingResponse(
12311230
},
12321231
});
12331232
}
1234-
if (!revoked && !retryTimerRef.current) {
1235-
retryTimerRef.current = setTimeout(() => {
1236-
retryTimerRef.current = null;
1237-
if (
1238-
mountedRef.current &&
1239-
activeScopeKeyRef.current === scopeKey &&
1240-
activeRequestKeyRef.current === requestKey
1241-
) {
1242-
window.dispatchEvent(new CustomEvent(WORKSPACE_BILLING_RETRY_EVENT, {
1243-
detail: { requestKey },
1244-
}));
1245-
}
1246-
}, WORKSPACE_BILLING_RETRY_MS);
1247-
}
1233+
// A revoked read (403) fails closed and must not retry. Everything
1234+
// else — including the packaged client's synthetic proxy 502s —
1235+
// retries on the shared, exponentially backed-off schedule.
1236+
if (!revoked) scheduleWorkspaceBillingRetry(requestKey);
12481237
}
12491238
}
12501239
}, [
@@ -1306,8 +1295,13 @@ export function useWorkspaceBillingResponse(
13061295
const expired = enforceWorkspaceBillingHardExpiry(current);
13071296
cachedWorkspaceBillingResponses.set(scopeKey, expired);
13081297
setState({ scopeKey, response: expired });
1298+
// `force: true` — hard expiry KNOWS the cached answer is void (that is
1299+
// the whole point of the timer), so the revalidation must bypass any
1300+
// settled coalescing entry, exactly like an identity change. Failure
1301+
// retries deliberately dispatch WITHOUT force so they can share a
1302+
// concurrent consumer's fresh success instead.
13091303
window.dispatchEvent(new CustomEvent(WORKSPACE_BILLING_RETRY_EVENT, {
1310-
detail: { requestKey },
1304+
detail: { requestKey, force: true },
13111305
}));
13121306
};
13131307
timer = setTimeout(
@@ -1381,8 +1375,15 @@ export function useWorkspaceBillingResponse(
13811375
if (event.key === WORKSPACE_BILLING_REFRESH_STORAGE_KEY) refreshAfterIdentityChange();
13821376
};
13831377
const onRetry = (event: Event) => {
1384-
const requestKey = (event as CustomEvent<{ requestKey?: string }>).detail?.requestKey;
1385-
if (requestKey === activeRequestKeyRef.current) void loadBilling(false, true);
1378+
const detail = (event as CustomEvent<{ requestKey?: string; force?: boolean }>).detail;
1379+
if (detail?.requestKey !== activeRequestKeyRef.current) return;
1380+
// Failure retries are deliberately NOT forced: a failed read is never
1381+
// cached, so the plain coalesced path is a genuine refetch — and when a
1382+
// concurrent consumer just succeeded, joining that fresh result re-syncs
1383+
// this one without adding another request to an already-struggling
1384+
// transport. Hard-expiry revalidation dispatches with `force: true`
1385+
// because its cached answer is void by definition.
1386+
void loadBilling(false, detail?.force === true);
13861387
};
13871388
window.addEventListener('focus', refresh);
13881389
window.addEventListener('pageshow', refresh);
@@ -1577,8 +1578,74 @@ export function workspaceBillingSnapshotForContext(
15771578
}
15781579

15791580
const WORKSPACE_BILLING_POLL_MS = 30_000;
1580-
const WORKSPACE_BILLING_RETRY_MS = 5_000;
1581+
const WORKSPACE_BILLING_RETRY_BASE_MS = 5_000;
1582+
const WORKSPACE_BILLING_RETRY_MAX_MS = 60_000;
15811583
const WORKSPACE_BILLING_RETRY_EVENT = 'od:workspace-billing-retry';
1584+
1585+
/**
1586+
* One retry schedule per billing `requestKey`, shared by every mounted
1587+
* consumer — the module-level counterpart of the per-hook timer it replaced.
1588+
*
1589+
* Two properties are load-bearing for the packaged (od://) client, whose
1590+
* proxy answers with synthetic 502s (`OD_PROTOCOL_PROXY_FAILED`) when the
1591+
* bursty first-open request load hits a transient transport failure:
1592+
*
1593+
* 1. The delay grows exponentially (5s → 10s → 20s → 40s → 60s cap) while
1594+
* failures are consecutive. A fixed 5s cadence against a struggling
1595+
* transport is self-defeating — each retry adds to the very burst that
1596+
* is producing the 502s it is retrying.
1597+
* 2. The schedule is keyed once per requestKey, not once per mounted hook.
1598+
* N consumers failing on the same shared read used to arm N timers whose
1599+
* N events each fanned out to N listeners; one schedule dispatches one
1600+
* retry event per cycle and the listeners' coalesced reads share one
1601+
* network request.
1602+
*
1603+
* A success only RESETS the consecutive-failure count — it deliberately does
1604+
* not cancel a pending timer. Consumers hold their own state, so a success
1605+
* observed by one consumer has not reached the others; letting the pending
1606+
* retry fire re-syncs them through the coalescing cache (a fresh success
1607+
* within the share window costs zero network requests).
1608+
*/
1609+
type WorkspaceBillingRetrySchedule = {
1610+
consecutiveFailures: number;
1611+
timer: ReturnType<typeof setTimeout> | null;
1612+
};
1613+
const workspaceBillingRetrySchedules = new Map<string, WorkspaceBillingRetrySchedule>();
1614+
1615+
function scheduleWorkspaceBillingRetry(requestKey: string): void {
1616+
if (typeof window === 'undefined') return;
1617+
let schedule = workspaceBillingRetrySchedules.get(requestKey);
1618+
if (!schedule) {
1619+
schedule = { consecutiveFailures: 0, timer: null };
1620+
workspaceBillingRetrySchedules.set(requestKey, schedule);
1621+
}
1622+
if (schedule.timer != null) return;
1623+
const delay = Math.min(
1624+
WORKSPACE_BILLING_RETRY_BASE_MS * 2 ** schedule.consecutiveFailures,
1625+
WORKSPACE_BILLING_RETRY_MAX_MS,
1626+
);
1627+
schedule.consecutiveFailures += 1;
1628+
schedule.timer = setTimeout(() => {
1629+
schedule.timer = null;
1630+
// Dispatch unconditionally: listeners filter on their own active
1631+
// requestKey, and an event nobody is mounted for is a no-op.
1632+
window.dispatchEvent(
1633+
new CustomEvent(WORKSPACE_BILLING_RETRY_EVENT, { detail: { requestKey } }),
1634+
);
1635+
}, delay);
1636+
}
1637+
1638+
function clearWorkspaceBillingRetryFailures(requestKey: string): void {
1639+
const schedule = workspaceBillingRetrySchedules.get(requestKey);
1640+
if (schedule) schedule.consecutiveFailures = 0;
1641+
}
1642+
1643+
function resetWorkspaceBillingRetrySchedules(): void {
1644+
for (const schedule of workspaceBillingRetrySchedules.values()) {
1645+
if (schedule.timer != null) clearTimeout(schedule.timer);
1646+
}
1647+
workspaceBillingRetrySchedules.clear();
1648+
}
15821649
export const WORKSPACE_BILLING_REFRESH_EVENT = 'od:workspace-billing-refresh';
15831650
const WORKSPACE_BILLING_REFRESH_STORAGE_KEY = 'od.workspaceBilling.refreshAt';
15841651

apps/web/tests/useWorkspaceBilling.scope.test.tsx

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type {
99
import { resetCoalescedGet } from '../src/lib/coalesced-get';
1010
import {
1111
lastResolvedWorkspaceContext,
12+
notifyWorkspaceBillingRefresh,
1213
notifyWorkspaceContextRefresh,
1314
resetWorkspaceBillingCache,
1415
resetWorkspaceContextCache,
@@ -938,6 +939,162 @@ describe('useWorkspaceBilling explicit scope', () => {
938939
);
939940
}, 7_000);
940941

942+
it('backs off exponentially while billing keeps failing and re-arms at the base delay after a success', async () => {
943+
// Packaged-client regression (first-open loading spin): the od:// proxy
944+
// answers billing with synthetic 502s under bursty first-open load, and a
945+
// FIXED 5s retry cadence kept feeding the burst it was waiting out. The
946+
// schedule must grow 5s → 10s → 20s → 40s … and reset once a read succeeds.
947+
vi.useFakeTimers();
948+
Object.defineProperty(document, 'visibilityState', {
949+
configurable: true,
950+
get: () => 'hidden', // silence the 30s compatibility poll — this test meters retries only
951+
});
952+
let billingCalls = 0;
953+
let failBilling = true;
954+
vi.stubGlobal(
955+
'fetch',
956+
vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => {
957+
const url = String(input);
958+
if (url === '/api/workspace/directory') {
959+
return workspaceDirectoryResponse(teamContext('workspace-a'));
960+
}
961+
if (url === '/api/workspace/context') {
962+
return new Response(JSON.stringify({ context: teamContext('workspace-a') }), {
963+
status: 200,
964+
headers: { 'content-type': 'application/json' },
965+
});
966+
}
967+
const interest = billingInterestResponse(input, init);
968+
if (interest) return interest;
969+
if (url.startsWith('/api/workspace/billing?')) {
970+
billingCalls += 1;
971+
if (failBilling) {
972+
return new Response(JSON.stringify({ error: 'od_protocol_proxy_failed' }), {
973+
status: 502,
974+
headers: { 'content-type': 'application/json' },
975+
});
976+
}
977+
return new Response(JSON.stringify(billingResponse('workspace-a', '2.50')), {
978+
status: 200,
979+
headers: { 'content-type': 'application/json' },
980+
});
981+
}
982+
throw new Error(`unexpected fetch ${url}`);
983+
}),
984+
);
985+
const flush = async (ms: number) => {
986+
await act(async () => {
987+
await vi.advanceTimersByTimeAsync(ms);
988+
});
989+
};
990+
991+
try {
992+
const hook = renderHook(() => useWorkspaceBillingResponse());
993+
await flush(0);
994+
await flush(0);
995+
expect(billingCalls).toBe(1); // mount read failed (502)
996+
997+
await flush(5_000); // 1st retry at base 5s
998+
expect(billingCalls).toBe(2);
999+
1000+
await flush(5_000); // t=10s — a fixed 5s cadence would fire here
1001+
expect(billingCalls).toBe(2); // backed off: next retry is 10s out
1002+
1003+
await flush(5_000); // t=15s — the 10s retry lands
1004+
expect(billingCalls).toBe(3);
1005+
1006+
await flush(20_000); // t=35s — the 20s retry lands
1007+
expect(billingCalls).toBe(4);
1008+
1009+
failBilling = false;
1010+
await flush(40_000); // t=75s — the 40s retry lands and SUCCEEDS
1011+
expect(billingCalls).toBe(5);
1012+
expect(hook.result.current?.workspaceBalance?.balanceUsd).toBe('2.50');
1013+
1014+
// A later failure starts over at the base delay — the success reset
1015+
// the consecutive-failure count.
1016+
failBilling = true;
1017+
act(() => {
1018+
notifyWorkspaceBillingRefresh();
1019+
});
1020+
await flush(0);
1021+
expect(billingCalls).toBe(6); // pushed refresh failed (502)
1022+
await flush(5_000); // retry at base 5s again, not 60s
1023+
expect(billingCalls).toBe(7);
1024+
} finally {
1025+
delete (document as unknown as Record<string, unknown>).visibilityState;
1026+
}
1027+
}, 15_000);
1028+
1029+
it('a retry joins a concurrent consumer\'s fresh success instead of stampeding another request', async () => {
1030+
// De-forcing the retry path: failures are never cached, so the plain
1031+
// coalesced read is a genuine refetch — and when another mounted consumer
1032+
// just fetched a fresh success, the retry must share it (zero new network
1033+
// requests against a struggling transport) rather than evict it.
1034+
vi.useFakeTimers();
1035+
Object.defineProperty(document, 'visibilityState', {
1036+
configurable: true,
1037+
get: () => 'hidden',
1038+
});
1039+
let billingCalls = 0;
1040+
vi.stubGlobal(
1041+
'fetch',
1042+
vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => {
1043+
const url = String(input);
1044+
if (url === '/api/workspace/directory') {
1045+
return workspaceDirectoryResponse(teamContext('workspace-a'));
1046+
}
1047+
if (url === '/api/workspace/context') {
1048+
return new Response(JSON.stringify({ context: teamContext('workspace-a') }), {
1049+
status: 200,
1050+
headers: { 'content-type': 'application/json' },
1051+
});
1052+
}
1053+
const interest = billingInterestResponse(input, init);
1054+
if (interest) return interest;
1055+
if (url.startsWith('/api/workspace/billing?')) {
1056+
billingCalls += 1;
1057+
if (billingCalls === 1) {
1058+
return new Response(JSON.stringify({ error: 'od_protocol_proxy_failed' }), {
1059+
status: 502,
1060+
headers: { 'content-type': 'application/json' },
1061+
});
1062+
}
1063+
return new Response(JSON.stringify(billingResponse('workspace-a', '2.50')), {
1064+
status: 200,
1065+
headers: { 'content-type': 'application/json' },
1066+
});
1067+
}
1068+
throw new Error(`unexpected fetch ${url}`);
1069+
}),
1070+
);
1071+
const flush = async (ms: number) => {
1072+
await act(async () => {
1073+
await vi.advanceTimersByTimeAsync(ms);
1074+
});
1075+
};
1076+
1077+
try {
1078+
const first = renderHook(() => useWorkspaceBillingResponse());
1079+
await flush(0);
1080+
await flush(0);
1081+
expect(billingCalls).toBe(1); // first consumer's mount read failed → retry armed at +5s
1082+
1083+
await flush(4_500);
1084+
const second = renderHook(() => useWorkspaceBillingResponse());
1085+
await flush(0);
1086+
await flush(0);
1087+
expect(billingCalls).toBe(2); // second consumer's mount read succeeded
1088+
expect(second.result.current?.workspaceBalance?.balanceUsd).toBe('2.50');
1089+
1090+
await flush(500); // t=5s — the first consumer's retry fires
1091+
expect(billingCalls).toBe(2); // joined the 0.5s-old success; no third request
1092+
expect(first.result.current?.workspaceBalance?.balanceUsd).toBe('2.50');
1093+
} finally {
1094+
delete (document as unknown as Record<string, unknown>).visibilityState;
1095+
}
1096+
}, 15_000);
1097+
9411098
it('keeps daemon-authored last-good state on a retryable directory outage', async () => {
9421099
let billingCalls = 0;
9431100
vi.stubGlobal(

0 commit comments

Comments
 (0)