diff --git a/scripts/format-scope.json b/scripts/format-scope.json index 60e436097..f0ede6ebf 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -1211,6 +1211,7 @@ "src/tooling/nominatimOutlineRoute.test.mjs", "src/data/areaGeometry.test.mjs", "src/data/areaResolver.test.mjs", + "src/sources/osmFeatures.test.mjs", "src/layers/osmPlaces/index.test.mjs", "src/tooling/boundedCache.test.mjs", "src/tooling/osmFeaturesRoute.test.mjs", diff --git a/src/sources/osmFeatures.js b/src/sources/osmFeatures.js index 52be8c4d0..e40806381 100644 --- a/src/sources/osmFeatures.js +++ b/src/sources/osmFeatures.js @@ -49,6 +49,14 @@ export function createOsmFeatureSource({ error: body?.error || `OpenStreetMap search failed (${response.status}).`, }; + // A garbled success (a proxy's error page) is a failure, not zero places. + if (!body || typeof body !== 'object') + return { + ok: false, + status: response.status, + code: null, + error: 'OpenStreetMap search returned an unreadable answer.', + }; return { ok: true, ...body }; }; } diff --git a/src/sources/osmFeatures.test.mjs b/src/sources/osmFeatures.test.mjs new file mode 100644 index 000000000..49d2d10fb --- /dev/null +++ b/src/sources/osmFeatures.test.mjs @@ -0,0 +1,94 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createOsmFeatureSource } from './osmFeatures.js'; + +const respond = (status, body) => ({ + ok: status >= 200 && status < 300, + status, + json: async () => { + if (typeof body === 'string') throw new SyntaxError('not JSON'); + return body; + }, +}); + +test('the request names only the preset, box, mode and limit', async () => { + let asked = null; + const search = createOsmFeatureSource({ + fetchImpl: async (url) => { + asked = new URL(url, 'http://localhost'); + return respond(200, { features: [], count: 0 }); + }, + }); + const result = await search({ + preset: 'hospital', + bbox: [85.2, 27.6, 85.4, 27.8], + limit: 50, + }); + assert.equal(result.ok, true); + assert.equal(asked.pathname, '/api/osm/features'); + assert.equal(asked.searchParams.get('preset'), 'hospital'); + assert.equal( + asked.searchParams.get('bbox'), + '85.20000,27.60000,85.40000,27.80000', + ); + assert.equal(asked.searchParams.get('mode'), 'features'); + assert.equal(asked.searchParams.get('limit'), '50'); +}); + +test('an aborted search rethrows; a network failure is a refusal', async () => { + const controller = new AbortController(); + const aborting = createOsmFeatureSource({ + fetchImpl: async () => { + controller.abort(); + throw new DOMException('Aborted', 'AbortError'); + }, + }); + await assert.rejects( + aborting( + { preset: 'hospital', bbox: [0, 0, 1, 1] }, + { signal: controller.signal }, + ), + { name: 'AbortError' }, + ); + const offline = createOsmFeatureSource({ + fetchImpl: async () => { + throw new TypeError('fetch failed'); + }, + }); + const refused = await offline({ preset: 'hospital', bbox: [0, 0, 1, 1] }); + assert.deepEqual([refused.ok, refused.status], [false, 0]); +}); + +test('server refusals keep their status and code', async () => { + const notConfigured = createOsmFeatureSource({ + fetchImpl: async () => + respond(503, { + error: 'Place search needs a configured Overpass server.', + code: 'OVERPASS_NOT_CONFIGURED', + }), + }); + const refused = await notConfigured({ + preset: 'hospital', + bbox: [0, 0, 1, 1], + }); + assert.equal(refused.ok, false); + assert.equal(refused.status, 503); + assert.equal(refused.code, 'OVERPASS_NOT_CONFIGURED'); + const busy = createOsmFeatureSource({ + fetchImpl: async () => respond(429, 'Too Many Requests'), + }); + const limited = await busy({ preset: 'hospital', bbox: [0, 0, 1, 1] }); + assert.equal(limited.ok, false); + assert.equal(limited.status, 429); + assert.equal(limited.code, null); + assert.match(limited.error, /429/); +}); + +test('a success whose body is not JSON is a failure, not zero places', async () => { + const garbled = createOsmFeatureSource({ + fetchImpl: async () => respond(200, 'proxy error'), + }); + const result = await garbled({ preset: 'hospital', bbox: [0, 0, 1, 1] }); + assert.equal(result.ok, false); + assert.equal(result.status, 200); +}); diff --git a/src/tooling/localServices.test.mjs b/src/tooling/localServices.test.mjs index e54f93e9e..84a2f6926 100644 --- a/src/tooling/localServices.test.mjs +++ b/src/tooling/localServices.test.mjs @@ -747,6 +747,8 @@ test('Realtime service configuration selects compatible endpoint/model without f endpoint: 'https://voice.example/client-secrets', models: { standard: 'configured-model' }, resolveApiKey: () => 'server-fixture', + // Pinned, so the expected tool list never depends on this machine's env. + overpassConfigured: () => false, fetchImpl: async (url, options) => { assert.equal(url, 'https://voice.example/client-secrets'); assert.equal(options.redirect, 'error'); @@ -754,10 +756,8 @@ test('Realtime service configuration selects compatible endpoint/model without f const payload = JSON.parse(options.body); assert.equal(payload.session.model, 'configured-model'); assert.deepEqual( - payload.session.tools.map((tool) => tool.name), - realtimeTools({ - overpass: Boolean(process.env.OVERPASS_UPSTREAMS), - }).map((tool) => tool.name), + payload.session.tools, + realtimeTools({ overpass: false }), ); return Response.json({ value: 'short-lived-fixture' }); }, diff --git a/src/voice/gevActions.test.mjs b/src/voice/gevActions.test.mjs index 462ca32ff..44073d60b 100644 --- a/src/voice/gevActions.test.mjs +++ b/src/voice/gevActions.test.mjs @@ -5257,6 +5257,76 @@ test('different OSM siblings from one response do not race the server slot or me assert.deepEqual(resultKinds, ['hospital', 'school']); }); +test('an analyst follow-up on OSM places keeps the area they were found in', async () => { + const viewer = { + clock: { onTick: { addEventListener: () => () => {} } }, + scene: { canvas: { addEventListener() {}, removeEventListener() {} } }, + camera: { + moveEnd: { addEventListener() {} }, + positionCartographic: { height: 300_000, latitude: 0, longitude: 0 }, + }, + }; + let rows = []; + const osmLayer = { + setResults(next) { + rows = next; + }, + getAnalystRecords: () => rows, + }; + const runner = createGevActionRunner({ + viewer, + styleManager: {}, + dataManager: { + layers: new Map([['osm-places', { module: osmLayer }]]), + isEnabled: (id) => id === 'osm-places', + getAll: () => [], + }, + areaOptions: { + findRegion: async () => null, + findAdminCandidates: async () => [ + { + kind: 'state', + name: 'Test Province', + country: 'Testland', + source: 'natural-earth', + polygons: [ + [ + [ + [0, 0], + [1, 0], + [1, 1], + [0, 1], + [0, 0], + ], + ], + ], + }, + ], + }, + osmSearch: async () => ({ + ok: true, + truncated: false, + features: [ + { id: 'node/1', lat: 0.5, lon: 0.5, tags: { name: 'North Hospital', emergency: 'yes' } }, + { id: 'node/2', lat: 0.4, lon: 0.6, tags: { name: 'South Hospital', emergency: 'no' } }, + ], + }), + }); + const area = await runner('resolve_area', { query: 'Test Province', level: 'admin1' }); + assert.equal(area.ok, true); + const found = await runner('osm_query', { what: 'hospitals', areaId: area.areaId }); + assert.equal(found.ok, true, found.error); + assert.equal(found.count, 2); + const followUp = await runner('analyst_query', { + layers: ['osm-places'], + followUp: true, + filters: [{ field: 'emergency', op: 'eq', value: 'yes' }], + }); + assert.equal(followUp.ok, true, followUp.error); + assert.equal(followUp.count, 1); + assert.equal(followUp.scopeLabel, 'inside Test Province'); +}); + test('a capped Contacts snapshot is a lower bound from one immutable cohort', async () => { globalThis.window = globalThis.window || { clearTimeout,