diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 00352adbc..75162a3dd 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -3092,7 +3092,10 @@ down | auth-failed` (plus the unchanged `missing-key`/`unsupported`) with > outlasts its retries, the proxy pauses upstream calls for the `Retry-After` > (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 +> gets 502, whose `partial` lists the real heights it has (null for the rest) +> when it has any, and the page keeps those and uses its geoid fallback only +> for the points without one. The same holds when the upstream answers a +> point with a null height. 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 diff --git a/src/data/terrainHeights.test.mjs b/src/data/terrainHeights.test.mjs index b39cf44e1..a3eaddf3d 100644 --- a/src/data/terrainHeights.test.mjs +++ b/src/data/terrainHeights.test.mjs @@ -19,6 +19,7 @@ import assert from 'node:assert/strict'; import { resolveEllipsoidalGround, cachedEllipsoidalGround, + cachedRealEllipsoidalGround, } from './terrainHeights.js'; const AUSTIN = { lat: 30.2672, lon: -97.7431 }; @@ -362,3 +363,47 @@ test('resolveEllipsoidalGround: null and string heights cannot become real zeroe ); } }); + +test('resolveEllipsoidalGround: a 502 keeps the real heights it carries and falls back only for the rest', async () => { + const coords = [ + { lat: 62.001, lon: -150.001, sourceOrthometricM: 10 }, + { lat: 62.002, lon: -150.002, sourceOrthometricM: 10 }, + { lat: 62.003, lon: -150.003, sourceOrthometricM: 10 }, + ]; + await withFakeFetch( + async () => ({ + ok: false, + status: 502, + json: async () => ({ + error: 'terrain heights fetch failed and no cache available for every point', + partial: [{ ellipsoid: 412.5 }, null, { ellipsoid: 'x' }], + }), + }), + async () => { + const out = await resolveEllipsoidalGround(coords); + assert.deepEqual(out[0], { ellipsoid: 412.5, source: 'reearth' }); + assert.equal(out[1].source, 'geoid-fallback'); + assert.equal(out[2].source, 'geoid-fallback', 'a non-numeric height is not real'); + } + ); + assert.equal(cachedRealEllipsoidalGround(62.001, -150.001), 412.5); + assert.equal(cachedRealEllipsoidalGround(62.002, -150.002), null); +}); + +test('resolveEllipsoidalGround: a partial list of the wrong length is ignored', async () => { + const coords = [ + { lat: 63.001, lon: -151.001 }, + { lat: 63.002, lon: -151.002 }, + ]; + await withFakeFetch( + async () => ({ + ok: false, + status: 502, + json: async () => ({ error: 'x', partial: [{ ellipsoid: 7 }] }), + }), + async () => { + const out = await resolveEllipsoidalGround(coords); + assert.deepEqual(out.map((r) => r.source), ['geoid-fallback', 'geoid-fallback']); + } + ); +}); diff --git a/src/data/terrainHeightsProxy.js b/src/data/terrainHeightsProxy.js index f06ff3671..c17e595fb 100644 --- a/src/data/terrainHeightsProxy.js +++ b/src/data/terrainHeightsProxy.js @@ -190,7 +190,8 @@ export async function fetchTerrainChunkWithRetry( * Resolve a requested point batch against a per-point cache and a missing-only * fetcher. Results are reconstructed by canonical key in exact request order, * including duplicates. Stale values remain eligible only when refresh fails. - * If any position has no real height, return 502 with no fabricated result. + * If any position has no real height, return 502 with no fabricated result; + * the body's `partial` lists the real heights it has, null for the rest. * * @param {object} options * @param {Array<[number, number]>} options.points @@ -269,11 +270,19 @@ export async function resolveTerrainHeightRequest({ return entry && validTerrainResult(entry.result) ? entry.result : null; }); if (results.some((result) => result == null)) { + // The real heights this request does have, null elsewhere, so the page + // keeps them and uses its geoid only for the rest. Named `partial`, not + // `results`, so no reader mistakes it for a complete answer; left out when + // no point has a height. + const partial = results.some((result) => result != null) + ? { partial: results } + : {}; return { status: 502, body: { error: 'terrain heights fetch failed and no cache available for every point', + ...partial, }, cacheChanged, upstreamError, diff --git a/src/data/terrainHeightsProxy.test.mjs b/src/data/terrainHeightsProxy.test.mjs index c9e143337..6e0b31332 100644 --- a/src/data/terrainHeightsProxy.test.mjs +++ b/src/data/terrainHeightsProxy.test.mjs @@ -268,3 +268,31 @@ test('terrain parser rejects out-of-range WGS84 points and accepts inclusive bou [0, 0], ]); }); + +test('a 502 lists the real heights it has, null for the rest', async () => { + const cache = new Map([ + [terrainPointKey([1, 1]), { at: 0, result: result('stale-a', 11) }], + ]); + const response = await resolveTerrainHeightRequest({ + points: [ + [1, 1], + [2, 2], + [1, 1], + ], + cache, + ttlMs: 100, + now: () => 1000, + fetchMissing: async () => { + const error = new Error('HTTP 429'); + error.status = 429; + error.coolingDown = true; + throw error; + }, + }); + assert.equal(response.status, 502); + assert.equal('results' in response.body, false); + assert.deepEqual( + response.body.partial.map((item) => item?.id ?? null), + ['stale-a', null, 'stale-a'], + ); +}); diff --git a/src/services/requests.js b/src/services/requests.js index 3e3a819eb..28f56265e 100644 --- a/src/services/requests.js +++ b/src/services/requests.js @@ -150,12 +150,25 @@ export function createApplicationRequestServices({ const query = points .map(({ lat, lon }) => `${lon.toFixed(5)},${lat.toFixed(5)}`) .join(';'); - return requireOk( - await request(`${urls.terrain}?points=${encodeURIComponent(query)}`, { - signal, - }), - 'Terrain heights', - )?.results; + const response = await request( + `${urls.terrain}?points=${encodeURIComponent(query)}`, + { signal }, + ); + // A failed request still carries the real heights the server had, so + // the caller can keep them; null marks a point without one. + const partial = response.data?.partial; + if ( + !response.ok && + Array.isArray(partial) && + partial.length === points.length + ) { + const error = new Error( + `Terrain heights unavailable (${response.status})`, + ); + error.partial = partial; + throw error; + } + return requireOk(response, 'Terrain heights')?.results; }, }, regional: { diff --git a/src/services/requests.test.mjs b/src/services/requests.test.mjs index 2cbc464cd..ddfc378b0 100644 --- a/src/services/requests.test.mjs +++ b/src/services/requests.test.mjs @@ -94,3 +94,20 @@ test('a failed probe is retried after its backoff, not on every query', async () 'GET /api/overpass/status', ]); }); + +test('a failed terrain request carries the real heights the server had', async () => { + const partial = [{ ellipsoid: 412.5 }, null]; + const services = createApplicationRequestServices({ + fetchImpl: async () => Response.json({ error: 'x', partial }, { status: 502 }), + }); + const points = [{ lat: 30, lon: -97 }, { lat: 31, lon: -97 }]; + await assert.rejects(services.terrain.getHeights(points), (error) => { + assert.match(error.message, /Terrain heights unavailable \(502\)/); + assert.deepEqual(error.partial, partial); + return true; + }); + await assert.rejects(services.terrain.getHeights(points.slice(0, 1)), (error) => { + assert.equal(error.partial, undefined, 'a list of the wrong length is not attached'); + return true; + }); +}); diff --git a/src/services/terrainHeights.js b/src/services/terrainHeights.js index f84a4413f..00cddafe7 100644 --- a/src/services/terrainHeights.js +++ b/src/services/terrainHeights.js @@ -226,7 +226,8 @@ export function createTerrainHeights({ * - Remaining (deduplicated) coordinates are sent to `/api/terrain/heights` * in sequential chunks of <=64 points. * - If a chunk request fails (network error, non-ok HTTP, malformed body), - * every point in THAT chunk falls back to geoid math + * every point in THAT chunk without a real height in the failure's + * `partial` falls back to geoid math * (`sourceOrthometricM + geoidHeight` or `geoidHeight` alone) rather than * failing the whole batch — a transient proxy outage in the middle of a * large batch shouldn't blank out entities whose chunk happened to land @@ -314,15 +315,34 @@ export function createTerrainHeights({ cacheStore(item.key, entry); } } - } catch { + } catch (error) { requestSignal?.throwIfAborted(); // Proxy down (or cold cache had nothing to serve-stale) — fall back to - // geoid math for every point in this chunk. `ensureGeoidReady()` is - // awaited lazily, only on the fallback path, so the common (proxy - // healthy) case never pays for the geoid grid's dynamic import. + // geoid math for the points in this chunk. A failure that still + // carries real heights (the proxy's `partial`, e.g. cached points + // during an upstream 429 pause) keeps them; only the rest use the + // geoid. `ensureGeoidReady()` is awaited lazily, only on the fallback + // path, so the common (proxy healthy) case never pays for the geoid + // grid's dynamic import. + const partial = + Array.isArray(error?.partial) && error.partial.length === chunk.length + ? error.partial + : []; + const fallback = []; + chunk.forEach((item, index) => { + const ellipsoid = partial[index]?.ellipsoid; + if (!Number.isFinite(ellipsoid)) { + fallback.push(item); + return; + } + const entry = { ellipsoid, source: 'reearth' }; + resolvedThisCall.set(item.key, entry); + cacheStore(item.key, entry); + }); + if (fallback.length === 0) continue; await ensureGeoidReady(); requestSignal?.throwIfAborted(); - for (const item of chunk) { + for (const item of fallback) { const ellipsoid = geoidFallback( item.lat, item.lon,