diff --git a/CHANGELOG.md b/CHANGELOG.md index 96eb6d91c..c89335375 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,14 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md ### Fixed +- Reduce terrain-height timeouts when Re:Earth slows down. Batches are + sized against measured response latency on both browser and server to reduce + request timeouts, and a partial upstream failure now + keeps the heights that did resolve rather than discarding them. A position + the upstream answers with no height is reported as an absent reading instead + of a failed refresh, so the log distinguishes a slow or broken upstream from + one that simply has no value for a coordinate. + - Separate optional Google server credentials for Places and Street View from the browser key, contributed by Tom-Neverwinter (#110). Provider Settings, Pinokio's app-specific credential handling and setup diagnostics recognize diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 65caab7a7..dedc5b3de 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1211,12 +1211,20 @@ This is the current runtime/source-of-truth snapshot for the project. > settled dim contact always completes its release after tracking ends. > - **Terrain-height resilience:** `/api/terrain/heights` caches canonical > 5-decimal points individually, reconstructs reordered/overlapping batches -> in exact request order, and refreshes only missing or stale points. Network, -> 429, and 5xx failures receive bounded jittered retries with `Retry-After`; -> stale real heights remain usable per point, while an uncached absent height -> still returns 502 rather than becoming a fabricated ground value. Client -> geoid fallbacks wait 60 seconds before retrying and self-heal to Re:Earth on -> the first later successful fetch. +> in exact request order, and refreshes only missing or stale points. Upstream +> calls and browser requests are both chunked at 64 points, sized to fit +> their 30s deadlines across Re:Earth's observed 87-186 ms/point range; a chunk +> that still fails contributes nulls for its own positions instead of +> discarding the chunks that resolved. Network, 429, and 5xx failures receive +> bounded jittered retries with `Retry-After`; stale real heights remain +> 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. 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 +> placement continues to use the existing prior until real heights arrive. > - **Overpass cache admission:** `/api/overpass` parses and sanitizes requests, > then checks fresh memory, identical in-flight work, and fresh disk entries > before invoking its local 90/min limiter. Cache and single-flight responses diff --git a/server/providers/terrain.js b/server/providers/terrain.js index 23de62f42..e58a63ea5 100644 --- a/server/providers/terrain.js +++ b/server/providers/terrain.js @@ -12,18 +12,27 @@ import { /** * Re:Earth terrain point-height proxy: batched lon/lat → ellipsoidal height * lookups, keyless. Upstream: https://terrain.reearth.land/heights.json - * (≤256 points per call). Terrain doesn't move, so results are cached to + * (UPSTREAM_CHUNK points per call — sized against measured latency, see + * below). Terrain doesn't move, so results are cached to * disk with a long TTL (30 days) — mirrors celestrakProxy's memory+disk * cache and serve-stale shape. Cache entries and stale fallback are keyed per * 5dp point, so reordered and partially overlapping batches reuse prior work. * Only missing/stale points go upstream; the response is rebuilt in exact - * request order. Oversized requests (>256 points) are chunked sequentially. + * request order. Larger requests are chunked sequentially, and one failing + * chunk does not discard the chunks that resolved. */ export function terrainHeightsProxy() { const TTL_MS = 30 * 24 * 3600_000; const CACHE_DIR = path.join(process.cwd(), '.gev-cache'); const CACHE_PATH = path.join(CACHE_DIR, 'terrain-heights.json'); - const UPSTREAM_CHUNK = 256; + // Sized against measured upstream latency, not the documented page cap. + // Re:Earth serves 87-186 ms/point depending on load (observed 2026-09-12, + // a 2x swing within one hour). 256 points therefore costs 22-48 s and blows + // the 30 s attempt timeout whenever the service is slow — the long-standing + // cause of "[terrain-heights-proxy] refresh incomplete" (owner incident + // 2026-08-21). 64 points costs 5.6-12 s, inside the timeout at both ends of + // that range. + const UPSTREAM_CHUNK = 64; const MAX_POINTS = 2000; /** @type {Map} keyed by canonical 5dp lon/lat. */ @@ -98,24 +107,39 @@ export function terrainHeightsProxy() { } /** - * Fetch all missing chunks sequentially (upstream caps each call at 256). + * Fetch all missing chunks sequentially, UPSTREAM_CHUNK points per call. * * 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. + * + * A chunk that still fails yields nulls for its own positions rather than + * rejecting: with several chunks per request, throwing would discard every + * chunk that did resolve and re-fetch it on the next poll. The resolver + * caches the resolved positions and reports the null ones as omitted. Total + * failure (nothing resolved) still rejects, so the real upstream error + * reaches the caller instead of a generic omission count. * @param {Array<[number, number]>} points * @returns {Promise>} */ async function fetchUpstreamAll(points) { const results = []; + let firstError = null; for (let i = 0; i < points.length; i += UPSTREAM_CHUNK) { const chunk = points.slice(i, i + UPSTREAM_CHUNK); - const chunkResults = await fetchTerrainChunkWithRetry(chunk); + let chunkResults = []; + try { + chunkResults = await fetchTerrainChunkWithRetry(chunk); + } catch (error) { + firstError = firstError || error; + } // Keep later chunks aligned even if a malformed upstream response omits // trailing positions. The resolver will reject each null individually. for (let j = 0; j < chunk.length; j += 1) results.push(chunkResults[j] ?? null); } + if (firstError && results.every((result) => result == null)) + throw firstError; return results; } @@ -169,6 +193,16 @@ export function terrainHeightsProxy() { '[terrain-heights-proxy] refresh incomplete' + ' — serving stale points when available', ); + } else if (outcome.absentPoints > 0) { + // Not a refresh failure. The upstream answered every position and + // returned a null height for a few of them (~0.16%, transient); + // the next poll re-asks and the client meanwhile resolves those + // through its bundled geoid. Informational, not actionable. + console.info( + `[terrain-heights-proxy] ${outcome.absentPoints}` + + ` of ${outcome.requestedPoints} position(s) had no upstream` + + ' height this poll — retrying next cycle', + ); } send(outcome.status, outcome.body); } catch (err) { diff --git a/src/data/terrainHeights.js b/src/data/terrainHeights.js index 89c4a3eff..de1dd9860 100644 --- a/src/data/terrainHeights.js +++ b/src/data/terrainHeights.js @@ -17,7 +17,7 @@ // single request much past ~700-1500 points (depending on coordinate // precision) risks a raw socket-level 431 *before* the proxy's own // request handler runs. So every network request this module issues is -// chunked at CHUNK_SIZE (200 points), well under that ceiling, and +// chunked at CHUNK_SIZE (64 points), well under that ceiling, and // chunks are sent SEQUENTIALLY (not in parallel) to keep this client // well-behaved against a single dev-server proxy. // 2. The proxy's disk cache is keyed by the raw `points` query string, with @@ -32,7 +32,9 @@ import { ensureGeoidReady, geoidHeight } from './geoid.js'; /** Max points per outgoing request to `/api/terrain/heights` (see file header, point 1). */ -const CHUNK_SIZE = 200; +// Match the server's upstream batch so sequential upstream work also fits +// within this client's 30-second request deadline when Re:Earth slows. +const CHUNK_SIZE = 64; /** Avoid repeatedly hitting a known-failing proxy from warm fallback reads. */ const GEOID_FALLBACK_COOLDOWN_MS = 60_000; @@ -116,7 +118,7 @@ async function fetchChunk(chunk) { // the response's own lon/lat fields to re-match — those are also present // for callers who want them, but this module doesn't need them. for (let i = 0; i < chunk.length; i += 1) { - const ellipsoid = Number(body.results[i]?.ellipsoid); + const ellipsoid = body.results[i]?.ellipsoid; if (!Number.isFinite(ellipsoid)) throw new Error(`non-finite ellipsoid height at index ${i}`); out.set(chunk[i].key, ellipsoid); } @@ -145,7 +147,7 @@ function geoidFallback(lat, lon, sourceOrthometricM) { * - Results already warm in the in-memory cache are returned without any * network call. * - Remaining (deduplicated) coordinates are sent to `/api/terrain/heights` - * in sequential chunks of <=200 points. + * 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 * (`sourceOrthometricM + geoidHeight` or `geoidHeight` alone) rather than diff --git a/src/data/terrainHeights.test.mjs b/src/data/terrainHeights.test.mjs index c7dd5a5f4..b39cf44e1 100644 --- a/src/data/terrainHeights.test.mjs +++ b/src/data/terrainHeights.test.mjs @@ -79,7 +79,7 @@ test('resolveEllipsoidalGround: happy path maps results IN ORDER', async () => { assert.ok(capturedUrl.includes('points=')); }); -test('resolveEllipsoidalGround: a 300-point input issues exactly 2 fetches (chunked at <=200/request) AND preserves order across the chunk boundary', async () => { +test('resolveEllipsoidalGround: a 300-point input issues exactly 5 fetches (chunked at <=64/request) AND preserves order across the chunk boundary', async () => { // Each coord carries a DISTINGUISHABLE lat/lon derived from its index so // the oracle below can be an independent function of the point's INPUT // identity — not an echo of whatever the request happened to send. This is @@ -119,11 +119,10 @@ test('resolveEllipsoidalGround: a 300-point input issues exactly 2 fetches (chun async () => { const out = await resolveEllipsoidalGround(coords); assert.equal(out.length, 300); - // Positional correctness at and across the chunk boundary (chunk 1 = - // [0,199], chunk 2 = [200,299]): each output must equal the oracle of + // Positional correctness across every chunk boundary: outputs equal the oracle of // its OWN input coord. The module rounds to 5dp before sending, so // compare against the oracle of the rounded coord to stay exact. - for (const k of [0, 1, 199, 200, 299]) { + for (let k = 0; k < coords.length; k += 1) { const lat = Number(coords[k].lat.toFixed(5)); const lon = Number(coords[k].lon.toFixed(5)); assert.equal( @@ -135,9 +134,10 @@ test('resolveEllipsoidalGround: a 300-point input issues exactly 2 fetches (chun } } ); - assert.equal(fetchCount, 2, `expected 2 fetches for 300 points at <=200/chunk, got ${fetchCount}`); + assert.equal(fetchCount, 5); + assert.deepEqual(chunkSizes, [64, 64, 64, 64, 44]); for (const size of chunkSizes) { - assert.ok(size <= 200, `chunk size ${size} exceeds the 200-point ceiling`); + assert.ok(size <= 64, `chunk size ${size} exceeds the 64-point ceiling`); } assert.equal( chunkSizes.reduce((a, b) => a + b, 0), @@ -350,3 +350,15 @@ test('resolveEllipsoidalGround: an empty input array resolves to an empty array ); assert.equal(fetchCount, 0); }); + +test('resolveEllipsoidalGround: null and string heights cannot become real zeroes', async () => { + for (const [index, value] of [null, '', '0'].entries()) { + await withFakeFetch( + async () => ({ ok: true, json: async () => ({ results: [{ ellipsoid: value }] }) }), + async () => { + const out = await resolveEllipsoidalGround([{ lat: 11 + index, lon: 41 }]); + assert.equal(out[0].source, 'geoid-fallback'); + } + ); + } +}); diff --git a/src/data/terrainHeightsProxy.js b/src/data/terrainHeightsProxy.js index 7e6eb406f..bfb058693 100644 --- a/src/data/terrainHeightsProxy.js +++ b/src/data/terrainHeightsProxy.js @@ -60,6 +60,28 @@ export function validTerrainResult(result) { return Boolean(result) && Number.isFinite(result.ellipsoid); } +/** + * A row the upstream DID return for its position, but with no usable height: + * Re:Earth sporadically answers with a real `elevation` and a null + * `geoid`/`ellipsoid`. Measured at ~0.16% of points, and transient — the same + * coordinate resolves on a later poll. That is an absent datum, not a failed + * refresh, so the point is left uncached to be re-asked while the client + * covers it through its bundled geoid. A null/undefined entry is NOT this + * case: it means the position was dropped from the response, which is a + * genuine upstream fault. + * @param {unknown} result + */ +export function absentTerrainDatum(result) { + return ( + typeof result === 'object' && + result !== null && + !Array.isArray(result) && + result.ellipsoid === null && + result.geoid === null && + Number.isFinite(result.elevation) + ); +} + /** * Convert Retry-After (delta-seconds or HTTP-date) to milliseconds. * @param {string|null|undefined} value @@ -194,24 +216,33 @@ export async function resolveTerrainHeightRequest({ let cacheChanged = false; let upstreamError = null; + let absentPoints = 0; if (missing.length > 0) { try { const fetched = await fetchMissing(missing.map((item) => item.point)); if (!Array.isArray(fetched)) throw new Error('malformed upstream response (no results array)'); const fetchedAt = now(); + let omittedPositions = 0; for (let i = 0; i < missing.length; i += 1) { const result = fetched[i]; - if (!validTerrainResult(result)) continue; - cache.set(missing[i].key, { at: fetchedAt, result }); - cacheChanged = true; + if (validTerrainResult(result)) { + cache.set(missing[i].key, { at: fetchedAt, result }); + cacheChanged = true; + } else if (absentTerrainDatum(result)) { + // Upstream answered but had no height. Transient, so deliberately + // left uncached: the next poll re-asks and usually succeeds. + absentPoints += 1; + } else { + // Missing or malformed positions remain upstream faults. + omittedPositions += 1; + } } - if ( - fetched.length !== missing.length || - missing.some((_, i) => !validTerrainResult(fetched[i])) - ) { + if (omittedPositions > 0 || fetched.length !== missing.length) { + const dropped = + omittedPositions || Math.abs(fetched.length - missing.length); upstreamError = new Error( - 'upstream omitted one or more terrain heights', + `upstream omitted ${dropped} terrain position(s)`, ); } } catch (error) { @@ -232,6 +263,8 @@ export async function resolveTerrainHeightRequest({ }, cacheChanged, upstreamError, + absentPoints, + requestedPoints: unique.size, }; } return { @@ -239,5 +272,7 @@ export async function resolveTerrainHeightRequest({ body: { results }, cacheChanged, upstreamError, + absentPoints, + requestedPoints: unique.size, }; } diff --git a/src/data/terrainHeightsProxy.test.mjs b/src/data/terrainHeightsProxy.test.mjs index cd05dca8b..f6b53de9a 100644 --- a/src/data/terrainHeightsProxy.test.mjs +++ b/src/data/terrainHeightsProxy.test.mjs @@ -145,3 +145,85 @@ test('stale per-point entries serve through a failed refresh only when every poi ['stale-b', 'stale-a'], ); }); + +test('a null-height row counts as absent, not a refresh failure, and is re-asked next poll', async () => { + const cache = new Map(); + let calls = 0; + const options = { + points: [ + [-97.428, 40.194], + [139.729, 35.693], + ], + cache, + ttlMs: 30 * 24 * 3600_000, + now: () => 1_000_000, + // Re:Earth's observed transient shape: real elevation, null geoid/ellipsoid. + fetchMissing: async (missing) => { + calls += 1; + return missing.map(([lon, lat]) => + lon === -97.428 + ? { lon, lat, elevation: 443.855, geoid: null, ellipsoid: null } + : result('real', 69.93), + ); + }, + }; + + const first = await resolveTerrainHeightRequest(options); + assert.equal( + first.upstreamError, + null, + 'upstream answered every position; absence is not its fault', + ); + assert.equal(first.absentPoints, 1); + assert.equal(first.requestedPoints, 2); + assert.equal( + first.status, + 502, + 'still no usable height, and none may be fabricated', + ); + assert.equal( + cache.has(terrainPointKey([-97.428, 40.194])), + false, + 'absent points stay uncached', + ); + assert.equal( + cache.get(terrainPointKey([139.729, 35.693])).result.id, + 'real', + 'a batch-mate with a real height still caches', + ); + + await resolveTerrainHeightRequest(options); + assert.equal(calls, 2, 'a transient null must be re-asked, never suppressed'); +}); + +test('a dropped position remains a genuine upstream fault', async () => { + const response = await resolveTerrainHeightRequest({ + points: [ + [1, 1], + [2, 2], + ], + cache: new Map(), + ttlMs: 10_000, + now: () => 5_000, + fetchMissing: async () => [result('ok-a', 11)], + }); + + assert.notEqual(response.upstreamError, null); + assert.match(response.upstreamError.message, /omitted/); + assert.equal(response.absentPoints, 0, 'an omission is not an absence'); +}); + +test('malformed height objects remain refresh failures, not absent readings', async () => { + for (const row of [{}, [], { ellipsoid: '0' }, { ellipsoid: null }]) { + const response = await resolveTerrainHeightRequest({ + points: [[1, 2]], + cache: new Map(), + ttlMs: 10_000, + fetchMissing: async () => [row], + }); + assert.equal(response.status, 502); + assert.ok(response.upstreamError); + assert.equal(response.absentPoints, 0); + assert.equal(response.cacheChanged, false); + } +}); diff --git a/src/tooling/environmentProviders.test.mjs b/src/tooling/environmentProviders.test.mjs index 38e87c6f6..1541d25e2 100644 --- a/src/tooling/environmentProviders.test.mjs +++ b/src/tooling/environmentProviders.test.mjs @@ -88,18 +88,22 @@ test('terrain middleware chunks missing points and reconstructs repeated/reorder const points = Array.from({ length: 257 }, (_, i) => `${i / 100},1`); const res = await request('/?points=' + points.join(';')); assert.equal(res.status, 200); - assert.deepEqual(sizes, [256, 1]); + // Tracks UPSTREAM_CHUNK in server/providers/terrain.js (64): 257 points + // split into four full chunks and a remainder. + assert.deepEqual(sizes, [64, 64, 64, 64, 1]); const reordered = await request('/?points=2.56,1;0,1;2.56,1'); assert.deepEqual(json(reordered), { results: [{ ellipsoid: 102.56 }, { ellipsoid: 100 }, { ellipsoid: 102.56 }], }); - assert.equal(calls, 2); + // Five chunks for the first batch; the reordered request is fully cached. + assert.equal(calls, 5); assert.equal((await request('/?points=invalid')).status, 400); assert.equal( (await request('/?points=' + Array(2001).fill('0,1').join(';'))).status, 500, ); - assert.equal(calls, 2); + // Rejected requests add no upstream calls: still the five from the first batch. + assert.equal(calls, 5); }); test('terrain middleware migrates valid legacy disk points without fabricating omitted heights', async (t) => { @@ -254,3 +258,40 @@ test('GBFS keeps host/path/method guards, response caps and distinct information ); assert.equal((await request(target('station_status.json'))).status, 502); }); + +test('terrain middleware retains successful chunks around a failure and retries only missing points', async (t) => { + isolate(t); + const requests = []; + let fail = true; + t.mock.method(globalThis, 'fetch', async (raw) => { + const points = new URL(raw).searchParams + .get('points') + .split(';') + .map((p) => p.split(',').map(Number)); + requests.push(points); + if (fail && points[0][0] === 0.64) return new Response('', { status: 400 }); + return Response.json({ + results: points.map(([lon]) => ({ ellipsoid: lon + 100 })), + }); + }); + const request = install(terrainHeightsProxy()); + const points = Array.from({ length: 130 }, (_, i) => `${i / 100},1`); + const url = '/?points=' + points.join(';'); + assert.equal((await request(url)).status, 502); + assert.deepEqual( + requests.map((p) => p.length), + [64, 64, 2], + ); + fail = false; + const recovered = await request(url); + assert.equal(recovered.status, 200); + assert.deepEqual( + requests[3], + Array.from({ length: 64 }, (_, i) => [(i + 64) / 100, 1]), + ); + assert.deepEqual( + json(recovered).results, + Array.from({ length: 130 }, (_, i) => ({ ellipsoid: i / 100 + 100 })), + ); + assert.equal(requests.length, 4); +});