From 7860cbf59e05549604c08414a086dacf3f5adf2b Mon Sep 17 00:00:00 2001 From: Said Date: Tue, 20 Oct 2026 00:57:24 +0100 Subject: [PATCH 1/2] fix(terrain): bound each height request to one upstream chunk --- docs/CURRENT-STATE.md | 5 +++ server/providers/terrain.js | 10 +++--- src/terrainHeightsBounds.test.mjs | 21 ++++++++++++ src/tooling/environmentProviders.test.mjs | 41 ++++++++++++----------- 4 files changed, 53 insertions(+), 24 deletions(-) diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 55b7a63ab..e5c4bb9f9 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1,5 +1,10 @@ # God's Eye View Current State +Terrain height requests accept at most 64 points, matching the browser service's +existing batch size and one bounded upstream chunk. Larger requests return HTTP +413 before disk-cache loading or upstream work; malformed coordinates remain 400. +The browser continues to split larger workloads into sequential 64-point requests. + Shared distance checks use `src/geoDistance.js` with `{ lat, lon }` points and the IUGG mean spherical Earth radius, 6371.0088 km. `greatCircleKm` is canonical; `greatCircleMeters` is its exact metre conversion. Missing, diff --git a/server/providers/terrain.js b/server/providers/terrain.js index 32083d23a..c678ab7c0 100644 --- a/server/providers/terrain.js +++ b/server/providers/terrain.js @@ -20,8 +20,8 @@ import { admitSameSiteKeyless } from './common/same-site.js'; * 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. Larger requests are chunked sequentially, and one failing - * chunk does not discard the chunks that resolved. + * request order. Each request is bounded to one 64-point upstream chunk; + * larger client workloads are split by the browser terrain service. * * Bounds, because every distinct 5dp point is its own key: coordinates must be * in WGS-84 range (rejected at parse), the point cache has an entry ceiling @@ -55,7 +55,7 @@ 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; - const MAX_POINTS = 2000; + const MAX_POINTS = UPSTREAM_CHUNK; /** * Entry ceiling for the point cache, and with it the file written from it. * @@ -255,7 +255,6 @@ export function terrainHeightsProxy(options = {}) { res.end(JSON.stringify({ error: 'Rate limit exceeded' })); return; } - await loadDiskOnce(); const parsedUrl = new URL(req.url || '', 'http://internal'); const rawPoints = parsedUrl.searchParams.get('points'); const points = parseTerrainPoints(rawPoints); @@ -267,12 +266,13 @@ export function terrainHeightsProxy(options = {}) { return; } if (points.length > MAX_POINTS) { - send(500, { + send(413, { error: `too many points (${points.length}); max ${MAX_POINTS} per request`, }); return; } + await loadDiskOnce(); const outcome = await resolveTerrainHeightRequest({ points, cache: mem, diff --git a/src/terrainHeightsBounds.test.mjs b/src/terrainHeightsBounds.test.mjs index cd1a9ad36..3746f4fe4 100644 --- a/src/terrainHeightsBounds.test.mjs +++ b/src/terrainHeightsBounds.test.mjs @@ -366,3 +366,24 @@ test('a second client keeps its own quota', async (t) => { }); assert.notEqual(neighbour.statusCode, 429); }); + +test('one 64-point chunk is accepted; larger batches fail before cache or upstream work', async (t) => { + const root = await mkdtemp(path.join(tmpdir(), 'terrain-batch-limit-')); + t.after(() => rm(root, { recursive: true, force: true })); + const asked = stubTerrainUpstream(t); + const route = install( + terrainHeightsProxy({ cachePath: path.join(root, 'cache.json') }), + ).get('/api/terrain/heights'); + const points = (count) => + Array.from({ length: count }, (_, i) => `${i / 1000},30`).join(';'); + for (const count of [65, 2001]) { + const response = await request(route, { url: `/?points=${points(count)}` }); + assert.equal(response.statusCode, 413); + assert.match(response.body.error, /max 64/); + assert.equal(asked.length, 0); + } + const response = await request(route, { url: `/?points=${points(64)}` }); + assert.equal(response.statusCode, 200); + assert.equal(response.body.results.length, 64); + assert.equal(asked.length, 64); +}); diff --git a/src/tooling/environmentProviders.test.mjs b/src/tooling/environmentProviders.test.mjs index b6faeb34c..bbb0e9d6e 100644 --- a/src/tooling/environmentProviders.test.mjs +++ b/src/tooling/environmentProviders.test.mjs @@ -98,7 +98,7 @@ test('standalone composition mounts every extracted provider exactly once withou assert.equal(plugins.filter((p) => p.name === factory().name).length, 1); }); -test('terrain middleware chunks missing points and reconstructs repeated/reordered requests from cache', async (t) => { +test('terrain middleware bounds missing points and reconstructs repeated/reordered requests from cache', async (t) => { isolate(t); let calls = 0; const sizes = []; @@ -116,25 +116,23 @@ test('terrain middleware chunks missing points and reconstructs repeated/reorder }); }); const request = install(terrainHeightsProxy()); - const points = Array.from({ length: 257 }, (_, i) => `${i / 100},1`); + const points = Array.from({ length: 64 }, (_, i) => `${i / 100},1`); const res = await request('/?points=' + points.join(';')); assert.equal(res.status, 200); - // 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(sizes, [64]); + const reordered = await request('/?points=0.63,1;0,1;0.63,1'); assert.deepEqual(json(reordered), { - results: [{ ellipsoid: 102.56 }, { ellipsoid: 100 }, { ellipsoid: 102.56 }], + results: [{ ellipsoid: 100.63 }, { ellipsoid: 100 }, { ellipsoid: 100.63 }], }); - // Five chunks for the first batch; the reordered request is fully cached. - assert.equal(calls, 5); + // One bounded batch; the reordered request is fully cached. + assert.equal(calls, 1); assert.equal((await request('/?points=invalid')).status, 400); assert.equal( (await request('/?points=' + Array(2001).fill('0,1').join(';'))).status, - 500, + 413, ); - // Rejected requests add no upstream calls: still the five from the first batch. - assert.equal(calls, 5); + // Rejected requests add no upstream calls. + assert.equal(calls, 1); }); test('terrain middleware migrates valid legacy disk points without fabricating omitted heights', async (t) => { @@ -388,21 +386,26 @@ test('terrain middleware retains successful chunks around a failure and retries }); 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); + const urls = [0, 64, 128].map( + (start) => '/?points=' + points.slice(start, start + 64).join(';'), + ); assert.deepEqual( - requests.map((p) => p.length), - [64, 64, 2], + await Promise.all(urls.map(async (url) => (await request(url)).status)), + [200, 502, 200], + ); + assert.deepEqual( + requests.map((p) => p.length).sort((a, b) => a - b), + [2, 64, 64], ); fail = false; - const recovered = await request(url); - assert.equal(recovered.status, 200); + const recovered = await Promise.all(urls.map((url) => request(url))); + assert.ok(recovered.every((response) => response.status === 200)); assert.deepEqual( requests[3], Array.from({ length: 64 }, (_, i) => [(i + 64) / 100, 1]), ); assert.deepEqual( - json(recovered).results, + recovered.flatMap((response) => json(response).results), Array.from({ length: 130 }, (_, i) => ({ ellipsoid: i / 100 + 100 })), ); assert.equal(requests.length, 4); From f436d72d1bccf20f95141fb470d497a35831b157 Mon Sep 17 00:00:00 2001 From: Said Date: Tue, 20 Oct 2026 00:57:54 +0100 Subject: [PATCH 2/2] docs: record terrain fix --- changelog.d/1276-terrain-issue-fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/1276-terrain-issue-fix.md diff --git a/changelog.d/1276-terrain-issue-fix.md b/changelog.d/1276-terrain-issue-fix.md new file mode 100644 index 000000000..17e8f4b50 --- /dev/null +++ b/changelog.d/1276-terrain-issue-fix.md @@ -0,0 +1 @@ +- Bound each height request to one upstream chunk (asturksever, #1276).