mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-04 02:14:01 +00:00
fix(terrain): end retries early on a long Retry-After; cover the 429 pause
A Retry-After longer than the 10 s retry budget used to sleep the whole budget out and then give up, so the proxy's 429 pause started 10 s late and every new-point poll in that window still went upstream and logged. The retry loop now stops at once in that case. Tests: the retry loop no longer sleeps for an unreachable Retry-After; fully cached points, including stale ones, are served during the pause with no upstream call; a 503 does not pause; a shorter concurrent Retry-After never cuts the pause short. CURRENT-STATE now says which requests are still served and which log.
This commit is contained in:
1 parent
a75534e4d3
commit
c1820aea49
6 files changed
+135
-7
No files matched your search
@@ -1,3 +1,5 @@
|
||||
- The terrain proxy stops asking the upstream for a while after a 429
|
||||
(its Retry-After, at most 10 minutes, or 60 s) instead of retrying on every
|
||||
page request (quanticsoul4772, #1402).
|
||||
page request, and a Retry-After longer than its 10 s retry budget now ends
|
||||
the retries at once instead of sleeping the budget out (quanticsoul4772,
|
||||
#1402).
|
||||
@@ -3078,11 +3078,14 @@ down | auth-failed` (plus the unchanged `missing-key`/`unsupported`) with
|
||||
> usable per point, while an uncached absent height still returns 502 rather
|
||||
> than becoming a fabricated ground value. A position the upstream answers
|
||||
> with a null ellipsoid is reported as an absent height rather than a refresh
|
||||
> failure, and is left uncached so a later poll re-asks it. After a 429 that
|
||||
> failure, and is left uncached so a later poll re-asks it. A `Retry-After`
|
||||
> longer than the 10 s retry budget ends the retries at once. After a 429 that
|
||||
> outlasts its retries, the proxy pauses upstream calls for the `Retry-After`
|
||||
> (60 seconds when none, at most 10 minutes): cached and stale points are still
|
||||
> served, uncached points get 502 and use the page's geoid fallback, and the
|
||||
> pause is logged once by the request that started it. Client geoid
|
||||
> (60 seconds when none, at most 10 minutes). A request whose points are all
|
||||
> cached, fresh or stale, is still served; a request with any uncached point
|
||||
> gets 502 and the page uses its geoid fallback for that chunk. Each request
|
||||
> that was waiting on the upstream when the 429 came back logs one warning;
|
||||
> requests refused during the pause do not log. Client geoid
|
||||
> fallbacks wait 60 seconds before retrying and self-heal to Re:Earth on the
|
||||
> first later successful fetch. Smaller chunks reduce timeout risk; complete
|
||||
> camera batches still wait for their sequential requests, and unresolved
|
||||
|
||||
@@ -209,7 +209,8 @@ export function terrainHeightsProxy(options = {}) {
|
||||
*
|
||||
* The first try keeps its empirically required 30s timeout; network errors,
|
||||
* 429, and 5xx receive up to three jittered retries sharing a 10s added-time
|
||||
* budget, with Retry-After honored within that bound.
|
||||
* budget, with Retry-After honored within that bound; a longer Retry-After
|
||||
* ends the retries at once so the 429 pause below starts without delay.
|
||||
*
|
||||
* A chunk that still fails yields nulls for its own positions rather than
|
||||
* rejecting: with several chunks per request, throwing would discard every
|
||||
|
||||
@@ -107,7 +107,10 @@ export function terrainRetryAfterMs(value, nowMs = Date.now()) {
|
||||
/**
|
||||
* Fetch one upstream chunk with jittered exponential backoff. Network errors,
|
||||
* 429, and 5xx retry; other HTTP failures and malformed successes fail fast.
|
||||
* Retry sleeps and retry attempts share a 10s budget after attempt one.
|
||||
* Retry sleeps and retry attempts share a 10s budget after attempt one. A
|
||||
* Retry-After longer than the budget left ends the retries at once: sleeping
|
||||
* out the budget cannot reach the time the upstream asked for, and it would
|
||||
* delay the caller's own back-off (the proxy's 429 pause) by up to 10 s.
|
||||
*
|
||||
* Dependencies are injectable for deterministic offline tests.
|
||||
* @param {Array<[number, number]>} points
|
||||
@@ -168,6 +171,7 @@ export async function fetchTerrainChunkWithRetry(
|
||||
if (remaining <= 0) break;
|
||||
|
||||
const retryAfterMs = terrainRetryAfterMs(error?.retryAfter, now());
|
||||
if (retryAfterMs != null && retryAfterMs > remaining) break;
|
||||
const backoffMs = Math.round(
|
||||
350 * 2 ** attempt * (0.75 + random() * 0.5),
|
||||
);
|
||||
|
||||
@@ -91,6 +91,36 @@ test('429 retry honors Retry-After before the next attempt', async () => {
|
||||
assert.equal(results[0].id, 'recovered');
|
||||
});
|
||||
|
||||
test('a Retry-After longer than the retry budget ends the retries without sleeping', async () => {
|
||||
let clock = 3_000_000;
|
||||
let attempts = 0;
|
||||
const sleeps = [];
|
||||
await assert.rejects(
|
||||
fetchTerrainChunkWithRetry([[1, 1]], {
|
||||
now: () => clock,
|
||||
random: () => 0.5,
|
||||
makeSignal: () => undefined,
|
||||
sleep: async (ms) => {
|
||||
sleeps.push(ms);
|
||||
clock += ms;
|
||||
},
|
||||
fetchImpl: async () => {
|
||||
attempts += 1;
|
||||
return {
|
||||
ok: false,
|
||||
status: 429,
|
||||
headers: {
|
||||
get: (name) => (name.toLowerCase() === 'retry-after' ? '30' : null),
|
||||
},
|
||||
};
|
||||
},
|
||||
}),
|
||||
(error) => error.status === 429 && error.retryAfter === '30',
|
||||
);
|
||||
assert.equal(attempts, 1);
|
||||
assert.deepEqual(sleeps, [], 'no sleep that cannot reach the Retry-After');
|
||||
});
|
||||
|
||||
test('permanently failing upstream exhausts retries and returns 502 without fake results', async () => {
|
||||
let clock = 2_000_000;
|
||||
let attempts = 0;
|
||||
|
||||
@@ -476,3 +476,91 @@ test('a far-off Retry-After pauses the terrain upstream for at most 10 minutes',
|
||||
t.mock.timers.setTime(pausedAt + 10 * 60_000 + 1);
|
||||
assert.equal((await request('/?points=3,4')).status, 200, 'pause over');
|
||||
});
|
||||
|
||||
/** Drive mocked timers until `promise` settles; the retry budget is 10 s. */
|
||||
async function settleOnMockedTime(t, promise) {
|
||||
let settled = false;
|
||||
const tracked = promise.finally(() => {
|
||||
settled = true;
|
||||
});
|
||||
for (let step = 0; step < 60 && !settled; step += 1) {
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
t.mock.timers.tick(1_000);
|
||||
}
|
||||
assert.ok(settled, 'the request settles within its retry budget');
|
||||
return tracked;
|
||||
}
|
||||
|
||||
test('during a terrain 429 pause fully cached points, even stale ones, are still served', async (t) => {
|
||||
isolate(t);
|
||||
t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 1_000_000 });
|
||||
let calls = 0;
|
||||
let limited = false;
|
||||
t.mock.method(globalThis, 'fetch', async () => {
|
||||
calls += 1;
|
||||
return limited
|
||||
? new Response('', { status: 429, headers: { 'retry-after': '300' } })
|
||||
: Response.json({ results: [{ ellipsoid: 100 }] });
|
||||
});
|
||||
const request = install(terrainHeightsProxy());
|
||||
assert.equal((await request('/?points=5,6')).status, 200);
|
||||
// Past the 30-day TTL, so 5,6 is stale and would normally be refreshed.
|
||||
t.mock.timers.setTime(Date.now() + 31 * 24 * 3600_000);
|
||||
limited = true;
|
||||
const first = await settleOnMockedTime(t, request('/?points=1,2'));
|
||||
assert.equal(first.status, 502);
|
||||
assert.equal(calls, 2, 'a Retry-After past the budget is not retried');
|
||||
const stale = await request('/?points=5,6');
|
||||
assert.equal(stale.status, 200);
|
||||
assert.deepEqual(json(stale).results, [{ ellipsoid: 100 }]);
|
||||
assert.equal((await request('/?points=5,6;7,8')).status, 502);
|
||||
assert.equal(calls, 2, 'no upstream call during the pause');
|
||||
});
|
||||
|
||||
test('a terrain upstream failure other than 429 does not pause later requests', async (t) => {
|
||||
isolate(t);
|
||||
t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 1_000_000 });
|
||||
let calls = 0;
|
||||
t.mock.method(globalThis, 'fetch', async () => {
|
||||
calls += 1;
|
||||
return new Response('', { status: 503 });
|
||||
});
|
||||
const request = install(terrainHeightsProxy());
|
||||
const first = await settleOnMockedTime(t, request('/?points=1,2'));
|
||||
assert.equal(first.status, 502);
|
||||
const afterFirst = calls;
|
||||
const second = await settleOnMockedTime(t, request('/?points=3,4'));
|
||||
assert.equal(second.status, 502);
|
||||
assert.ok(calls > afterFirst, 'new points still go upstream after a 503');
|
||||
});
|
||||
|
||||
test('a shorter concurrent Retry-After never cuts a terrain 429 pause short', async (t) => {
|
||||
isolate(t);
|
||||
t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 1_000_000 });
|
||||
let calls = 0;
|
||||
t.mock.method(globalThis, 'fetch', async (raw) => {
|
||||
calls += 1;
|
||||
const points = new URL(raw).searchParams.get('points');
|
||||
if (points.startsWith('0.'))
|
||||
return Response.json({ results: [{ ellipsoid: 1 }] });
|
||||
const retryAfter = points.startsWith('1.') ? '300' : '30';
|
||||
return new Response('', {
|
||||
status: 429,
|
||||
headers: { 'retry-after': retryAfter },
|
||||
});
|
||||
});
|
||||
const request = install(terrainHeightsProxy());
|
||||
// Load the disk cache first so both requests below reach the upstream in
|
||||
// order: the 300 s answer lands first, the 30 s answer after it.
|
||||
assert.equal((await request('/?points=0,0')).status, 200);
|
||||
const [long, short] = await settleOnMockedTime(
|
||||
t,
|
||||
Promise.all([request('/?points=1,2'), request('/?points=3,4')]),
|
||||
);
|
||||
assert.equal(long.status, 502);
|
||||
assert.equal(short.status, 502);
|
||||
const afterBoth = calls;
|
||||
t.mock.timers.setTime(Date.now() + 120_000);
|
||||
assert.equal((await request('/?points=5,6')).status, 502);
|
||||
assert.equal(calls, afterBoth, 'still paused by the 300 s Retry-After');
|
||||
});
|
||||
Reference in new issue
Block a user