From 19b4a7a7e04357ba6aa60a5521f443caef50458f Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:10:50 -0700 Subject: [PATCH 1/2] fix(terrain): keep known heights when a terrain request fails A request with any point the proxy cannot answer gets 502, and the page used to put every point of that chunk on its geoid, including the ones the proxy had real (cached or stale) heights for. During an upstream 429 pause that drops cached ground under every chunk that also holds a new point. The 502 body now carries `partial`: the request's real heights in order, null for points without one, left out when there are none. The terrain request service attaches it to the error it throws, and the page caches those heights and uses its geoid only for the rest. `get_terrain_height` still reports the failure as before. Co-Authored-By: Claude Opus 5.5 --- docs/CURRENT-STATE.md | 5 ++- src/data/terrainHeights.test.mjs | 45 +++++++++++++++++++++++++++ src/data/terrainHeightsProxy.js | 11 ++++++- src/data/terrainHeightsProxy.test.mjs | 28 +++++++++++++++++ src/services/requests.js | 25 +++++++++++---- src/services/requests.test.mjs | 17 ++++++++++ src/services/terrainHeights.js | 32 +++++++++++++++---- 7 files changed, 149 insertions(+), 14 deletions(-) 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, From d0f6f5c0d8790f9e9c8c0f05242039a90a56ea09 Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Wed, 28 Oct 2026 11:24:23 -0700 Subject: [PATCH 2/2] docs(changelog): add the terrain partial-heights entry Co-Authored-By: Claude Opus 5.5 --- changelog.d/1456-terrain-partial-heights.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/1456-terrain-partial-heights.md diff --git a/changelog.d/1456-terrain-partial-heights.md b/changelog.d/1456-terrain-partial-heights.md new file mode 100644 index 000000000..914e5a4b0 --- /dev/null +++ b/changelog.d/1456-terrain-partial-heights.md @@ -0,0 +1,3 @@ +- A terrain request the proxy can only partly answer (a 429 pause, or a point + the upstream returns without a height) now keeps the real heights it has + instead of putting the whole chunk on the geoid (quanticsoul4772, #1456).