From d7986309b6d0b89c8d493378e96f209780f08768 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:24:41 -0700 Subject: [PATCH] fix(terrain): pause upstream calls after a 429 A rate-limited upstream got asked again by every page request for new points (about once a second while the camera moves), each with its own retries, which kept the limit in force and logged "refresh incomplete" each time. After a 429 the proxy now leaves the upstream alone for its Retry-After (at most 10 minutes), or 60 s when it sends none, serving cached points meanwhile. The warning that starts the pause says "(HTTP 429; pausing upstream for N s)"; requests refused during the pause don't log. Co-Authored-By: Claude Opus 5.5 --- server/providers/terrain.js | 44 ++++++++++++++++--- src/tooling/environmentProviders.test.mjs | 52 +++++++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/server/providers/terrain.js b/server/providers/terrain.js index f4817f602..4d122d7ac 100644 --- a/server/providers/terrain.js +++ b/server/providers/terrain.js @@ -6,6 +6,7 @@ import { parseTerrainPoints, resolveTerrainHeightRequest, terrainPointKey, + terrainRetryAfterMs, validTerrainResult, } from '../../src/data/terrainHeightsProxy.js'; import { makeRateLimiter, clientKey } from './common/rate-limit.js'; @@ -55,6 +56,15 @@ export function terrainHeightsProxy(options = {}) { // 2026-08-21). 64 points costs 5.6-12 s, inside the timeout at both ends of // that range. const UPSTREAM_CHUNK = 64; + // After a 429 the upstream is left alone for its Retry-After (at most + // RATE_LIMIT_PAUSE_MAX_MS, so a far-off date can't stop terrain until a + // restart), or RATE_LIMIT_PAUSE_MS when it gives none. Every page poll for + // new points (about once a second while the camera moves) otherwise asks + // again with its own retries, which keeps the limit in force and logs each + // time. + const RATE_LIMIT_PAUSE_MS = 60_000; + const RATE_LIMIT_PAUSE_MAX_MS = 10 * 60_000; + let rateLimitedUntil = 0; const MAX_POINTS = UPSTREAM_CHUNK; /** * Entry ceiling for the point cache, and with it the file written from it. @@ -211,15 +221,31 @@ export function terrainHeightsProxy(options = {}) { * @returns {Promise>} */ async function fetchUpstreamAll(points) { + if (Date.now() < rateLimitedUntil) { + // Rate-limited: asking again only extends the limit. Cached points are + // still served; the rest resolve on the page through its geoid. + const error = new Error('HTTP 429'); + error.coolingDown = true; + throw error; + } const results = []; let firstError = null; for (let i = 0; i < points.length; i += UPSTREAM_CHUNK) { const chunk = points.slice(i, i + UPSTREAM_CHUNK); let chunkResults = []; - try { - chunkResults = await fetchTerrainChunkWithRetry(chunk); - } catch (error) { - firstError = firstError || error; + if (Date.now() >= rateLimitedUntil) { + try { + chunkResults = await fetchTerrainChunkWithRetry(chunk); + } catch (error) { + firstError = firstError || error; + if (error?.message === 'HTTP 429') + rateLimitedUntil = + Date.now() + + Math.min( + terrainRetryAfterMs(error.retryAfter) ?? RATE_LIMIT_PAUSE_MS, + RATE_LIMIT_PAUSE_MAX_MS, + ); + } } // Keep later chunks aligned even if a malformed upstream response omits // trailing positions. The resolver will reject each null individually. @@ -292,9 +318,17 @@ export function terrainHeightsProxy(options = {}) { evictOverflow(); diskDirty = true; } - if (outcome.upstreamError) { + if (outcome.upstreamError?.coolingDown) { + // Logged once, when the pause began. + } else if (outcome.upstreamError) { + const pause = + outcome.upstreamError.message === 'HTTP 429' && + Date.now() < rateLimitedUntil + ? ` (HTTP 429; pausing upstream for ${Math.ceil((rateLimitedUntil - Date.now()) / 1000)} s)` + : ''; console.warn( '[terrain-heights-proxy] refresh incomplete' + + pause + ' — serving stale points when available', ); } else if (outcome.absentPoints > 0) { diff --git a/src/tooling/environmentProviders.test.mjs b/src/tooling/environmentProviders.test.mjs index 5e786e408..dc0bc6ab7 100644 --- a/src/tooling/environmentProviders.test.mjs +++ b/src/tooling/environmentProviders.test.mjs @@ -411,3 +411,55 @@ test('terrain middleware retains successful chunks around a failure and retries ); assert.equal(requests.length, 4); }); + +test('after a 429 the terrain proxy leaves the upstream alone and logs once', async (t) => { + isolate(t); + const warnings = []; + t.mock.method(console, 'warn', (...args) => warnings.push(args.join(' '))); + let calls = 0; + t.mock.method(globalThis, 'fetch', async () => { + calls += 1; + return new Response('', { status: 429 }); + }); + const request = install(terrainHeightsProxy()); + await request('/?points=1,2'); + const afterFirst = calls; + assert.ok(afterFirst >= 1); + assert.equal(warnings.length, 1); + assert.match(warnings[0], /HTTP 429; pausing upstream for 60 s/); + // New points during the pause: no upstream call, no new log line. + const paused = await request('/?points=3,4'); + assert.equal(calls, afterFirst); + assert.equal(paused.status, 502); + assert.equal(warnings.length, 1); +}); + +test('a far-off Retry-After pauses the terrain upstream for at most 10 minutes', async (t) => { + isolate(t); + t.mock.method(console, 'warn', () => {}); + t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 1_000_000 }); + let limited = true; + t.mock.method(globalThis, 'fetch', async () => + limited + ? new Response('', { status: 429, headers: { 'retry-after': '86400' } }) + : Response.json({ results: [{ ellipsoid: 100 }] }), + ); + const request = install(terrainHeightsProxy()); + // The first request spends its retry budget on 429s, then the pause begins. + let settled = false; + const first = request('/?points=1,2').then((res) => { + settled = true; + return res; + }); + while (!settled) { + await new Promise((resolve) => setImmediate(resolve)); + t.mock.timers.tick(1_000); + } + assert.equal((await first).status, 502); + limited = false; + const pausedAt = Date.now(); + t.mock.timers.setTime(pausedAt + 9 * 60_000); + assert.equal((await request('/?points=3,4')).status, 502, 'still paused'); + t.mock.timers.setTime(pausedAt + 10 * 60_000 + 1); + assert.equal((await request('/?points=3,4')).status, 200, 'pause over'); +});