From 08eec8fd4086b21ea7e06e8556cf53386ddf63f3 Mon Sep 17 00:00:00 2001 From: xrpbanks <126300068+xrpbanks@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:42:48 +0200 Subject: [PATCH 1/2] fix(login): respect shared startup rate-limit cooldowns --- .github/workflows/ci.yml | 1 + frontend/lib/backendRequest.ts | 44 +++-- .../tests/backend_warmup_rate_limit.test.mjs | 161 ++++++++++++++++++ 3 files changed, 196 insertions(+), 10 deletions(-) create mode 100644 tools/tests/backend_warmup_rate_limit.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d17be8a..935ce16 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -318,6 +318,7 @@ jobs: tools/tests/account_erasure_ui.test.mjs tools/tests/account_privacy_locales.test.mjs tools/tests/backend_proxy_logout_fallback.test.mjs + tools/tests/backend_warmup_rate_limit.test.mjs tools/tests/calorieapp_embed_readiness.test.mjs tools/tests/identity_locales.test.mjs tools/tests/xaman_logout_request.test.mjs diff --git a/frontend/lib/backendRequest.ts b/frontend/lib/backendRequest.ts index 1fa4efc..a4dd371 100644 --- a/frontend/lib/backendRequest.ts +++ b/frontend/lib/backendRequest.ts @@ -12,7 +12,8 @@ const BACKEND_WARMUP_ATTEMPT_TIMEOUT_MS = 70_000; const BACKEND_WARMUP_INITIAL_RETRY_DELAY_MS = 5_000; const BACKEND_WARMUP_MAX_RETRY_DELAY_MS = 30_000; const BACKEND_WARMUP_RATE_LIMIT_DELAY_MS = 30_000; -const BACKEND_WARMUP_MAX_RETRY_AFTER_MS = 60_000; + +type WarmupRateLimit = { retryAt: number }; export class BackendRequestTimeoutError extends Error { constructor() { @@ -82,10 +83,7 @@ function retryAfterDelayMs(response: Response): number | null { return null; } - return Math.min( - seconds * 1_000, - BACKEND_WARMUP_MAX_RETRY_AFTER_MS - ); + return seconds * 1_000; } const retryAt = Date.parse(value); @@ -98,12 +96,17 @@ function retryAfterDelayMs(response: Response): number | null { return null; } - return Math.min(delayMs, BACKEND_WARMUP_MAX_RETRY_AFTER_MS); + return delayMs; } function retryDelayMs(response: Response | null, retryCount: number): number { if (response?.status === 429) { - return retryAfterDelayMs(response) ?? BACKEND_WARMUP_RATE_LIMIT_DELAY_MS; + // Retry-After is a minimum, never a reason to retry earlier. Keep a + // local pause even when the edge reports zero to prevent request bursts. + return Math.max( + retryAfterDelayMs(response) ?? 0, + BACKEND_WARMUP_RATE_LIMIT_DELAY_MS + ); } return Math.min( @@ -128,7 +131,8 @@ async function discardResponseBody(response: Response) { async function waitForBackendReadyAt( backendBaseUrl: string, signal?: AbortSignal, - timeoutMs = DEFAULT_BACKEND_WARMUP_TIMEOUT_MS + timeoutMs = DEFAULT_BACKEND_WARMUP_TIMEOUT_MS, + rateLimit: WarmupRateLimit = { retryAt: 0 } ) { const deadline = Date.now() + timeoutMs; let retryCount = 0; @@ -136,6 +140,17 @@ async function waitForBackendReadyAt( while (Date.now() < deadline) { throwIfAborted(signal); + // Both routes reach the same backend. A visible rate limit on either + // route must also pause retries on the other (including CORS failures). + const cooldownMs = Math.min( + rateLimit.retryAt - Date.now(), + deadline - Date.now() + ); + if (cooldownMs > 0) { + await new Promise((resolve) => setTimeout(resolve, cooldownMs)); + continue; + } + const remainingMs = deadline - Date.now(); const attemptTimeoutMs = Math.max( 1, @@ -170,6 +185,12 @@ async function waitForBackendReadyAt( } const requestedDelayMs = retryDelayMs(retryResponse, retryCount); + if (retryResponse?.status === 429) { + rateLimit.retryAt = Math.max( + rateLimit.retryAt, + Date.now() + requestedDelayMs + ); + } retryCount += 1; const boundedRetryDelayMs = Math.min( requestedDelayMs, @@ -198,6 +219,7 @@ export async function waitForBackendReady( ) { const sameOriginBaseUrl = "/api/backend"; const normalizedBaseUrl = backendBaseUrl.replace(/\/$/, ""); + const rateLimit: WarmupRateLimit = { retryAt: 0 }; if (normalizedBaseUrl === sameOriginBaseUrl) { return waitForBackendReadyAt(sameOriginBaseUrl, signal, timeoutMs); @@ -218,12 +240,14 @@ export async function waitForBackendReady( waitForBackendReadyAt( normalizedBaseUrl, directController.signal, - timeoutMs + timeoutMs, + rateLimit ), waitForBackendReadyAt( sameOriginBaseUrl, sameOriginController.signal, - timeoutMs + timeoutMs, + rateLimit ), ]); } catch { diff --git a/tools/tests/backend_warmup_rate_limit.test.mjs b/tools/tests/backend_warmup_rate_limit.test.mjs new file mode 100644 index 0000000..29ea826 --- /dev/null +++ b/tools/tests/backend_warmup_rate_limit.test.mjs @@ -0,0 +1,161 @@ +import assert from "node:assert/strict"; +import { readFile } from "node:fs/promises"; +import { createRequire } from "node:module"; +import test from "node:test"; +import vm from "node:vm"; + +const requireFromFrontend = createRequire( + new URL("../../frontend/package.json", import.meta.url) +); +const typescript = requireFromFrontend("typescript"); +const source = await readFile( + new URL("../../frontend/lib/backendRequest.ts", import.meta.url), + "utf8" +); +const compiled = typescript.transpileModule(source, { + compilerOptions: { + module: typescript.ModuleKind.CommonJS, + target: typescript.ScriptTarget.ES2022, + }, +}).outputText; + +function response(status, retryAfter = null, contentType = "text/plain") { + return { + ok: status >= 200 && status < 300, + status, + headers: { + get: (name) => name === "retry-after" ? retryAfter : contentType, + }, + json: async () => ({ status: "ok" }), + body: { cancel: async () => {} }, + }; +} + +const healthy = () => response(200, null, "application/json"); + +function warmupHarness(responses) { + let now = Date.UTC(2026, 0, 1); + const startedAt = now; + let nextTimer = 0; + const timers = new Map(); + const requests = []; + const module = { exports: {} }; + class Clock extends Date { + static now() { return now; } + } + const context = vm.createContext({ + AbortController, + Date: Clock, + module, + exports: module.exports, + process: { env: {} }, + setTimeout(callback, delay) { + const id = ++nextTimer; + timers.set(id, { at: now + delay, callback }); + return id; + }, + clearTimeout: (id) => timers.delete(id), + fetch: async (url, options) => { + const index = requests.length; + requests.push({ url, options, at: now - startedAt }); + if (typeof responses === "function") return responses(url, index); + assert.ok(index < responses.length, "Unexpected extra health request"); + return responses[index]; + }, + }); + vm.runInContext(compiled, context); + + return { + requests, + start: (signal, timeout = 180_000, baseUrl = "/api/backend") => + module.exports.waitForBackendReady(baseUrl, signal, timeout), + async settle(operation) { + let outcome; + operation.then( + (value) => { outcome = { value }; }, + (error) => { outcome = { error }; } + ); + for (let tick = 0; tick < 100; tick += 1) { + // Drain actual promises before advancing the synthetic clock. No + // external requests or real retry delays are used by these tests. + await new Promise(setImmediate); + if (outcome) { + if (outcome.error) throw outcome.error; + return outcome.value; + } + const next = [...timers.entries()].sort((a, b) => a[1].at - b[1].at)[0]; + assert.ok(next, "Warmup stalled without a timer"); + timers.delete(next[0]); + now = next[1].at; + next[1].callback(); + } + assert.fail("Warmup exceeded the bounded test clock"); + }, + }; +} + +test("ready backend proceeds immediately without credentials", async () => { + const harness = warmupHarness([healthy()]); + await harness.settle(harness.start()); + assert.deepEqual(harness.requests.map(({ at }) => at), [0]); + assert.equal(harness.requests[0].options.credentials, "omit"); +}); + +test("repeated zero Retry-After values cannot cause a request burst", async () => { + const harness = warmupHarness([ + response(429, "0"), response(429, "0"), response(429, "0"), healthy(), + ]); + await harness.settle(harness.start()); + assert.deepEqual(harness.requests.map(({ at }) => at), [0, 30_000, 60_000, 90_000]); +}); + +for (const retryAfter of ["120", "Thu, 01 Jan 2026 00:02:00 GMT"]) { + test(`server cooldown is not shortened: ${retryAfter}`, async () => { + const harness = warmupHarness([response(429, retryAfter), healthy()]); + await harness.settle(harness.start()); + assert.deepEqual(harness.requests.map(({ at }) => at), [0, 120_000]); + }); +} + +test("cooldown beyond the login window ends without another request", async () => { + const harness = warmupHarness([response(429, "240"), healthy()]); + await assert.rejects(harness.settle(harness.start()), { + name: "BackendRequestTimeoutError", + }); + assert.equal(harness.requests.length, 1); +}); + +for (const retryAfter of [null, "invalid", "-1", "5"]) { + test(`rate limit retains a minimum pause: ${retryAfter}`, async () => { + const harness = warmupHarness([response(429, retryAfter), healthy()]); + await harness.settle(harness.start()); + assert.deepEqual(harness.requests.map(({ at }) => at), [0, 30_000]); + }); +} + +test("ordinary startup responses keep their existing backoff", async () => { + const harness = warmupHarness([response(503), response(200), healthy()]); + await harness.settle(harness.start()); + assert.deepEqual(harness.requests.map(({ at }) => at), [0, 5_000, 15_000]); +}); + +test("cancelled login sends no health request", async () => { + const harness = warmupHarness([]); + const controller = new AbortController(); + controller.abort(new Error("Login cancelled")); + await assert.rejects(harness.settle(harness.start(controller.signal)), /Login cancelled/); + assert.equal(harness.requests.length, 0); +}); + +for (const limitedUrl of ["/api/backend/health", "https://backend.example/health"]) { + test(`both wake-up paths respect a cooldown from ${limitedUrl}`, async () => { + let limitedRequests = 0; + const harness = warmupHarness((url) => { + if (url !== limitedUrl) throw new Error("Network response unavailable"); + return limitedRequests++ === 0 ? response(429, "120") : healthy(); + }); + await harness.settle(harness.start(undefined, 180_000, "https://backend.example")); + assert.equal(harness.requests.filter(({ at }) => at < 120_000).length, 2); + assert.equal(harness.requests.filter(({ url }) => url === limitedUrl).length, 2); + }); +} From 55229a446b4035c30e3b31610eddf946329f1ff2 Mon Sep 17 00:00:00 2001 From: xrpbanks <126300068+xrpbanks@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:51:07 +0200 Subject: [PATCH 2/2] fix(login): cancel startup cooldown timers immediately --- frontend/lib/backendRequest.ts | 21 ++++++++- .../tests/backend_warmup_rate_limit.test.mjs | 45 +++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/frontend/lib/backendRequest.ts b/frontend/lib/backendRequest.ts index a4dd371..2bc9310 100644 --- a/frontend/lib/backendRequest.ts +++ b/frontend/lib/backendRequest.ts @@ -71,6 +71,23 @@ function throwIfAborted(signal?: AbortSignal) { } } +function waitForRetry(delayMs: number, signal?: AbortSignal) { + return new Promise((resolve, reject) => { + throwIfAborted(signal); + + const onAbort = () => { + clearTimeout(timeoutId); + signal?.removeEventListener("abort", onAbort); + reject(signal?.reason ?? new Error("Request aborted")); + }; + const timeoutId = setTimeout(() => { + signal?.removeEventListener("abort", onAbort); + resolve(); + }, delayMs); + signal?.addEventListener("abort", onAbort, { once: true }); + }); +} + function retryAfterDelayMs(response: Response): number | null { const value = response.headers.get("retry-after")?.trim(); if (!value) { @@ -147,7 +164,7 @@ async function waitForBackendReadyAt( deadline - Date.now() ); if (cooldownMs > 0) { - await new Promise((resolve) => setTimeout(resolve, cooldownMs)); + await waitForRetry(cooldownMs, signal); continue; } @@ -197,7 +214,7 @@ async function waitForBackendReadyAt( Math.max(0, deadline - Date.now()) ); if (boundedRetryDelayMs > 0) { - await new Promise((resolve) => setTimeout(resolve, boundedRetryDelayMs)); + await waitForRetry(boundedRetryDelayMs, signal); throwIfAborted(signal); } } diff --git a/tools/tests/backend_warmup_rate_limit.test.mjs b/tools/tests/backend_warmup_rate_limit.test.mjs index 29ea826..48d0409 100644 --- a/tools/tests/backend_warmup_rate_limit.test.mjs +++ b/tools/tests/backend_warmup_rate_limit.test.mjs @@ -67,6 +67,14 @@ function warmupHarness(responses) { return { requests, + elapsed: () => now - startedAt, + pendingTimers: () => timers.size, + abortAfter(controller, delay) { + timers.set(++nextTimer, { + at: now + delay, + callback: () => controller.abort(new Error("Login cancelled")), + }); + }, start: (signal, timeout = 180_000, baseUrl = "/api/backend") => module.exports.waitForBackendReady(baseUrl, signal, timeout), async settle(operation) { @@ -159,3 +167,40 @@ for (const limitedUrl of ["/api/backend/health", "https://backend.example/health assert.equal(harness.requests.filter(({ url }) => url === limitedUrl).length, 2); }); } + +test("cancelling a long retry pause immediately clears its timer", async () => { + const harness = warmupHarness([response(429, "120")]); + const controller = new AbortController(); + harness.abortAfter(controller, 5_000); + await assert.rejects(harness.settle(harness.start(controller.signal)), /Login cancelled/); + assert.equal(harness.elapsed(), 5_000); + assert.equal(harness.pendingTimers(), 0); + assert.equal(harness.requests.length, 1); +}); + +test("cancelling a shared cooldown stops both wake-up paths immediately", async () => { + const harness = warmupHarness((url) => { + if (url === "/api/backend/health") return response(429, "120"); + throw new Error("Network response unavailable"); + }); + const controller = new AbortController(); + harness.abortAfter(controller, 10_000); + await assert.rejects( + harness.settle(harness.start(controller.signal, 180_000, "https://backend.example")), + /Login cancelled/ + ); + assert.equal(harness.elapsed(), 10_000); + assert.equal(harness.pendingTimers(), 0); + assert.equal(harness.requests.length, 2); +}); + +test("a successful route immediately cancels the other route's retry timer", async () => { + const harness = warmupHarness((url) => { + if (url === "/api/backend/health") return healthy(); + throw new Error("Network response unavailable"); + }); + await harness.settle(harness.start(undefined, 180_000, "https://backend.example")); + assert.equal(harness.elapsed(), 0); + assert.equal(harness.pendingTimers(), 0); + assert.equal(harness.requests.length, 2); +});