mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-04 02:14:01 +00:00
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 <[email protected]>
This commit is contained in:
1 parent
5ae22ac333
commit
d7986309b6
2 files changed
+91
-5
No files matched your search
@@ -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<Array<object>>}
|
||||
*/
|
||||
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) {
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
Reference in new issue
Block a user