From 7fa0cfbb26b93c0afdd6aacea326614adfb1b72e Mon Sep 17 00:00:00 2001 From: Bilawal Sidhu <106619546+bilawalsidhu@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:22:08 -0500 Subject: [PATCH] feat(search): Nominatim as the last-resort geocoder (#573) Supersedes #350 by @sendmebits, ported onto the current component layout with fixes from review. Verified before merge: format, package boundaries, unit suite, build, and the browser tracking suite; a browser harness for the change where one applies; adversarial review applied. Co-authored-by: sendmebits <16546409+sendmebits@users.noreply.github.com> --- CHANGELOG.md | 22 ++ DATA_SOURCES.md | 3 +- docs/CURRENT-STATE.md | 11 + scripts/format-scope.json | 3 + scripts/package-boundaries.json | 3 +- server/providers/common/http.js | 4 +- server/providers/local.js | 2 + server/providers/regional.js | 6 +- server/providers/regional/http.js | 58 ++-- server/providers/regional/place.js | 278 +++++++++++++++- src/data/dataCredits.js | 2 +- src/nominatimGeocode.js | 171 ++++++++++ src/nominatimGeocode.test.mjs | 175 ++++++++++ src/search/defaults.js | 19 +- src/tooling/nominatimSearchRoute.test.mjs | 368 ++++++++++++++++++++++ 15 files changed, 1077 insertions(+), 48 deletions(-) create mode 100644 src/nominatimGeocode.js create mode 100644 src/nominatimGeocode.test.mjs create mode 100644 src/tooling/nominatimSearchRoute.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ae9bcfbd..e9b60c4c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,28 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md skip, so this affects every Overpass-backed layer: Mapped Installations, traffic roads and annotation geometry. Mirror rotation, cooldown and cache admission are unchanged (#420 — thanks @GladiatorrX9). +- Place search has a last resort. With no Google Maps key, and when Photon does + not answer, a named-place search now falls back to OpenStreetMap's Nominatim + through `/api/geocode`, so search and voice fly-to still work on a keyless + globe. The route keeps to the service's usage policy: an identifying + User-Agent and Referer, at most one request per second, answers cached, one + upstream call shared between identical searches in flight, a bounded queue so + a burst is refused rather than held, and a queued search dropped once its + caller has given up (#350 — thanks @sendmebits). + +- Regional upstream reads now hold their deadline through the response body. The + abort timer was cleared as soon as the headers arrived, so an upstream that + answered and then stalled mid-body had no deadline at all. Redirect policy is + now stated per call rather than inherited, and the fixed Nominatim endpoints + refuse to be redirected. + +- The location search box answers two kinds of query without a network request + or an API key. A decimal-degree coordinate — `43.1731, -79.0384`, or either + order when N/S/E/W say which is which — flies straight there; a bundled city + or landmark name typed exactly (`paris`, `sf`, `Golden Gate Bridge`) flies to + the bundled place. Anything else, including anything malformed, goes to the + existing geocoders unchanged. Degrees/minutes/seconds and grid references are + not parsed and fall through the same way (#388 — thanks @KuraPiee). - Draped annotation geometry — area fills and outlines, routes and arrows — classifies onto terrain as well as 3D tiles. On a keyless boot, where Cesium's diff --git a/DATA_SOURCES.md b/DATA_SOURCES.md index 9d546acfe..9c3262770 100644 --- a/DATA_SOURCES.md +++ b/DATA_SOURCES.md @@ -29,7 +29,7 @@ How to read this: | **TomTom Traffic API** (flow vector tiles) | Live congestion coloring for the traffic layer (optional, BYOK) | [TomTom for Developers terms](https://developer.tomtom.com) (proprietary, your own key; free tier currently 200K tile requests/month — see [current pricing](https://docs.tomtom.com/pricing/)) | "Traffic flow data © TomTom" — registered when live mode activates | | **OpenStreetMap (Overpass API)** | Viewport-bounded mapped installation context for Global Context | ODbL 1.0 | "© OpenStreetMap contributors" (incomplete mapped context) | | **Photon** (komoot) | Keyless place search — the fallback when no Google Maps key is configured, or when Google declines the Geocoding request | Service: free public instance, [fair use](https://photon.komoot.io) (no bulk/heavy use); underlying data: **ODbL 1.0** | "Photon (komoot)" + "© OpenStreetMap contributors" | -| **OpenStreetMap (Nominatim)** | Reverse-geocoded place label in the cockpit Local Info page | ODbL 1.0 + Nominatim usage policy | "© OpenStreetMap contributors" | +| **OpenStreetMap (Nominatim)** | Reverse-geocoded place label in the cockpit Local Info page, and last-resort forward place search via `/api/geocode` when Google and Photon do not answer | ODbL 1.0 + [Nominatim usage policy](https://operations.osmfoundation.org/policies/nominatim/) (maximum 1 request/second, results cached, identifying User-Agent) | "© OpenStreetMap contributors" | | **Open-Meteo** | Current weather in the cockpit Local Info page and cockpit-local dynamic atmospheric effects | [CC BY 4.0 data licence and adjacent-link attribution requirement](https://open-meteo.com/en/licence) | Linked "Weather data by Open-Meteo.com" beside the displayed local data | | **Google News RSS** | Primary locality-matched headlines in the cockpit Regional News page | [Google News Terms of Service](https://www.google.com/intl/en_us/terms_google_news.html) restrict use to personal, noncommercial use; linked articles remain third-party publisher content and retain publisher terms | "Google News RSS" plus each article's linked publisher/domain | | **GDELT Project DOC 2.0** | Fail-soft fallback for location-matched cockpit headlines | [GDELT Terms of Use](https://www.gdeltproject.org/about.html#termsofuse): unrestricted academic/commercial/governmental dataset use, with citation and link required; linked articles retain publisher terms | "GDELT Project" plus each article's linked publisher/domain | @@ -73,6 +73,7 @@ How to read this: - **Re:Earth Terrain.** Keyless (no API key). Used two ways: (1) `src/mapStackController.js` swaps in a `Cesium.CesiumTerrainProvider` pointed at Re:Earth's `cesium-mesh/ellipsoid` quantized-mesh endpoint for globe stacks without a Cesium ion token (e.g. OSM), replacing a flat `EllipsoidTerrainProvider`; falls back to the flat provider if the endpoint can't be reached. (2) The server-side `/api/terrain/heights` proxy (disk-cached, serve-stale) resolves per-point ellipsoidal ground height for entity placement. Both are best-effort with a keyless-safe fallback (bundled EGM96 geoid math) if Re:Earth is unreachable. - **OSRM on the FOSSGIS routing servers.** Keyless (no API key). Every route is requested by a person: clicking A and B in the Directions layer, or a voice "route from A to B". The server-side `/api/route` proxy sends an identifying `User-Agent`, pins the upstream host, accepts 2-12 coordinates, rejects any leg over 600 km or a total over 2,500 km, caps the buffered response at 8 MB, times the upstream out at 12 s, rate-limits per client, caches each route for 10 minutes, and coalesces identical requests that are in flight at the same time. Turn-by-turn maneuvers are asked for only when a caller wants them (`steps=1`), so the callers that do not read them send the smaller request. The "fix the map" link the policy asks for is in the Data attribution popover beside the OpenStreetMap credit. - **Global Context installation context.** `/api/military-installations` queries only an allow-listed subset of OSM `military=*` and `landuse=military` features inside a maximum 10° non-dateline viewport. It caches and may serve stale mapped context, but it is neither a global installation database nor evidence of capability, activity, or absence. User-requested Google Places results remain separately sourced candidates unless their returned types explicitly establish military classification; generic offices, museums, and similarly ambiguous matches are excluded from military proximity counts. +- **Place search.** Named-place fly-to uses Google Geocoding when a Maps key is configured, then Photon's public instance, then `/api/geocode` (Nominatim search) when neither answers. `/api/geocode` shares the cockpit's Nominatim queue at no more than one request per second, sends an identifying `User-Agent` and `Referer`, caches answers for five minutes, shares one upstream call between identical searches already in flight, bounds the queue so a burst is refused rather than held, and drops a queued search whose caller has given up. It asks for one result per search and is not used for systematic or bulk queries. - **Cockpit regional briefing.** `/api/regional-brief` rounds aircraft coordinates into 0.1° cache cells, caches results for five minutes, and serializes Nominatim calls at no more than one request per second. Google News RSS is queried with the resolved locality/region first; GDELT is used only when that RSS query fails or is empty. Google's published Google News terms restrict that source to personal, noncommercial use, so commercial deployments must disable/replace it or obtain separate permission; GDELT permits commercial dataset use with citation. The Data attribution popover identifies the active headline sources; article links retain publisher attribution. Headlines are location-query matches, not verified incidents, risk rankings, or evidence that a location is safe. Empty, partial, stale, and unavailable source states remain distinct. Open-Meteo supplies current conditions independently of the news source. `WX OFF` disables cockpit weather rendering only; the Local Info briefing still fetches its source-backed weather values and displays the required linked Open-Meteo credit. - **Dynamic weather presentation.** While cockpit mode is active, `/api/weather-effects` requests current Open-Meteo observations for the aircraft/camera location, rounds coordinates into 0.1° cache cells, caches results for five minutes, and may retain a stale observation for up to 30 minutes during a transient outage. WMO condition code selects the visual family; observed cloud cover, precipitation, visibility, wind speed, and wind direction bound its strength and motion. Missing or expired weather renders no synthetic atmospheric effect, and normal globe view never renders the weather overlay. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index e0fad54b8..d2192dd44 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -3079,6 +3079,17 @@ geocoders instead of being answered from the bundle. Degrees/minutes/seconds and grid references are not parsed. An answer either provider gives is exact, so nearby-landmark recovery stands aside for it — otherwise a typed coordinate could be replaced by whatever Places finds near the current view. The portable `./search` export +Google first when configured, Photon/OpenStreetMap as fallback (including Google +transport failures or declined requests), and the local `/api/geocode` route +over Nominatim as a last resort when neither answers. That route keeps to the +public instance's usage policy: an identifying `User-Agent` and `Referer`, at +most one request per second shared with the cockpit's reverse lookups, answers +cached for five minutes, one upstream call shared between identical searches +already in flight, a queue bounded at four waiting searches (a burst past it is +refused with 429 and `Retry-After`, never held), and a queued search dropped +once its caller has given up rather than spending a slot on an answer nobody +will read. A hit whose coordinates or bounding box are missing or out of range +is discarded rather than coerced. The portable `./search` export provides the service and adapters; it reads no environment or application state. Existing browser/server key setup is unchanged. diff --git a/scripts/format-scope.json b/scripts/format-scope.json index e2d4521b7..86ef14292 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -123,6 +123,9 @@ "server/providers/regional/http.js", "server/providers/regional/news.js", "server/providers/regional/place.js", + "src/nominatimGeocode.js", + "src/nominatimGeocode.test.mjs", + "src/tooling/nominatimSearchRoute.test.mjs", "server/providers/regional/query.js", "server/providers/regional/weather-effects.js", "server/providers/regional/weather.js", diff --git a/scripts/package-boundaries.json b/scripts/package-boundaries.json index d1a993cdd..e960da933 100644 --- a/scripts/package-boundaries.json +++ b/scripts/package-boundaries.json @@ -231,7 +231,8 @@ "server/providers/regional/query.js", "server/providers/regional/weather-effects.js", "server/providers/regional/weather.js", - "src/data/regionalModel.js" + "src/data/regionalModel.js", + "src/nominatimGeocode.js" ], "external": [] }, diff --git a/server/providers/common/http.js b/server/providers/common/http.js index 561d19cf8..b3a902949 100644 --- a/server/providers/common/http.js +++ b/server/providers/common/http.js @@ -49,8 +49,8 @@ export async function readResponseTextCapped(response, maxBytes, signal) { } /** Parse a fetch() JSON response only after enforcing a hard byte cap. */ -export async function readResponseJsonCapped(response, maxBytes) { - return JSON.parse(await readResponseTextCapped(response, maxBytes)); +export async function readResponseJsonCapped(response, maxBytes, signal) { + return JSON.parse(await readResponseTextCapped(response, maxBytes, signal)); } /** diff --git a/server/providers/local.js b/server/providers/local.js index 52a48f6d8..eb9059bd5 100644 --- a/server/providers/local.js +++ b/server/providers/local.js @@ -7,6 +7,7 @@ import { adsbdbProxy } from './aircraft/enrichment.js'; import { overpassProxy } from './overpass.js'; import { militaryInstallationsProxy } from './military-installations.js'; import { regionalBriefProxy } from './regional/briefing.js'; +import { geocodeProxy } from './regional/place.js'; import { weatherEffectsProxy } from './regional/weather-effects.js'; import { cctvProxy } from './cctv.js'; import { defaultSourceRoot } from './common/source-root.js'; @@ -32,6 +33,7 @@ function localProviderPlugins() { overpassProxy(), militaryInstallationsProxy(), regionalBriefProxy(), + geocodeProxy(), weatherEffectsProxy(), cctvProxy({ sourceRoot: defaultSourceRoot }), radioBrowserProxy(), diff --git a/server/providers/regional.js b/server/providers/regional.js index 3b097c8ef..e2c4fb671 100644 --- a/server/providers/regional.js +++ b/server/providers/regional.js @@ -4,4 +4,8 @@ export { } from './regional/briefing.js'; export { weatherEffectsProxy } from './regional/weather-effects.js'; export { validRegionalPoint } from './regional/query.js'; -export { createRegionalPlaceProvider } from './regional/place.js'; +export { + createRegionalPlaceProvider, + createNominatimSearchProvider, + geocodeProxy, +} from './regional/place.js'; diff --git a/server/providers/regional/http.js b/server/providers/regional/http.js index 9d8a7bc1e..b67808f98 100644 --- a/server/providers/regional/http.js +++ b/server/providers/regional/http.js @@ -5,42 +5,52 @@ import { const REGIONAL_MAX_RESPONSE_BYTES = 2 * 1024 * 1024; -async function fetchRegionalJson( - url, - { +/** + * One bounded upstream read for the regional providers. + * + * The deadline covers the BODY, not just the headers. `return`ing the reader's + * promise from inside a try/finally leaves the try block the moment the promise + * exists, so the abort timer used to be cleared before a single byte of body had + * arrived — an upstream that answered its headers and then stalled had no + * deadline at all. Awaiting inside the try keeps the timer alive until the body + * is in hand, and the same signal is handed to the reader so an abort stops the + * read rather than just the connect. + * + * `redirect` is stated rather than inherited: feeds that legitimately redirect + * (news RSS) follow, and a fixed API endpoint can be given 'error' so a + * redirect cannot steer the proxy at a destination this code never named. + * + * @param {string} url + * @param {{headers?: object, timeoutMs?: number, maxBytes?: number, redirect?: 'follow'|'error'|'manual'}} [options] + */ +async function fetchRegional(url, read, options = {}) { + const { headers = {}, timeoutMs = 9000, maxBytes = REGIONAL_MAX_RESPONSE_BYTES, - } = {}, -) { + redirect = 'follow', + } = options; const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); try { - const response = await fetch(url, { signal: controller.signal, headers }); + const response = await fetch(url, { + signal: controller.signal, + headers, + redirect, + }); if (!response.ok) throw new Error(`Upstream returned ${response.status}`); - return readResponseJsonCapped(response, maxBytes); + return await read(response, maxBytes, controller.signal); } finally { clearTimeout(timeout); } } -async function fetchRegionalText( - url, - { - headers = {}, - timeoutMs = 9000, - maxBytes = REGIONAL_MAX_RESPONSE_BYTES, - } = {}, -) { - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), timeoutMs); - try { - const response = await fetch(url, { signal: controller.signal, headers }); - if (!response.ok) throw new Error(`Upstream returned ${response.status}`); - return readResponseTextCapped(response, maxBytes); - } finally { - clearTimeout(timeout); - } +function fetchRegionalJson(url, options = {}) { + return fetchRegional(url, readResponseJsonCapped, options); +} + +function fetchRegionalText(url, options = {}) { + return fetchRegional(url, readResponseTextCapped, options); } export { fetchRegionalJson, fetchRegionalText }; diff --git a/server/providers/regional/place.js b/server/providers/regional/place.js index 0e9e1ac26..d1815a2fa 100644 --- a/server/providers/regional/place.js +++ b/server/providers/regional/place.js @@ -1,20 +1,123 @@ +import { makeRateLimiter, clientKey } from '../common/rate-limit.js'; +import { coalesceProxyRequest } from '../common/http.js'; import { fetchRegionalJson } from './http.js'; import { normalizeRegionalPlace } from '../../../src/data/regionalModel.js'; +import { + nominatimToGeocodeResult, + nominatimViewboxFromBounds, +} from '../../../src/nominatimGeocode.js'; -/** Construct the serialized Nominatim adapter with a trusted endpoint. */ +/** + * The usage policy for the public Nominatim instance asks for a User-Agent or + * Referer that identifies the application, and states that stock library + * agents will not do. Both are sent. + */ +const NOMINATIM_HEADERS = Object.freeze({ + 'User-Agent': + 'gods-eye-view/0.1 (+https://github.com/bilawalsidhu/gods-eye-view)', + Referer: 'https://github.com/bilawalsidhu/gods-eye-view', +}); + +/** + * Minimum spacing between upstream calls. The policy states an absolute + * maximum of one request per second; 1.1 s keeps clock jitter from crossing it. + */ +const NOMINATIM_MIN_SPACING_MS = 1100; + +/** + * How many searches may be waiting for their turn. + * + * One request per second and an unbounded queue are incompatible: a burst of + * searches would keep the upstream busy long after everyone who asked has given + * up, which is exactly the load the policy asks callers not to create. Past + * this depth a search is refused at once instead of being promised a slot + * minutes away. + */ +const NOMINATIM_MAX_PENDING = 4; + +/** + * How long a queued search may wait before it is not worth sending. The + * browser gives up well before this, so anything reaching the front later than + * this is answering nobody. + */ +const NOMINATIM_MAX_WAIT_MS = 10_000; + +const NOMINATIM_SEARCH_CACHE_MS = 5 * 60_000; + +const NOMINATIM_SEARCH_MAX_CACHE = 80; + +const NOMINATIM_SEARCH_MAX_QUERY = 200; + +// The pacer is deliberately module state while everything else here is +// per-instance. One request per second is a budget for the whole application, +// not for each provider object: two instances each pacing themselves would +// send two requests a second between them. Reverse lookups and searches +// therefore queue together. +let _nominatimQueue = Promise.resolve(); + +let _nominatimLastRequestAt = 0; + +let _nominatimPending = 0; + +/** Raised when the queue is already as deep as it is allowed to get. */ +function queueFullError() { + return Object.assign(new Error('Place search queue is full'), { + code: 'NOMINATIM_QUEUE_FULL', + }); +} + +/** Raised when a queued search waited so long that nobody is left to answer. */ +function abandonedError() { + return Object.assign(new Error('Place search was abandoned'), { + code: 'NOMINATIM_ABANDONED', + }); +} + +/** + * Run one piece of upstream work, never closer than the policy spacing to the + * last one. + * + * `bounded` applies the queue-depth and staleness limits. The cockpit's reverse + * lookups are a slow background trickle and stay unbounded; the search route, + * which a person can fire as fast as they can type, does not. + * + * @param {() => Promise} work + * @param {{bounded?: boolean, signal?: AbortSignal}} [options] + */ +function enqueueNominatim(work, { bounded = false, signal } = {}) { + if (bounded && _nominatimPending >= NOMINATIM_MAX_PENDING) + return Promise.reject(queueFullError()); + if (bounded) _nominatimPending += 1; + const queuedAt = Date.now(); + const task = _nominatimQueue.then(async () => { + try { + const waitMs = Math.max( + 0, + NOMINATIM_MIN_SPACING_MS - (Date.now() - _nominatimLastRequestAt), + ); + if (waitMs) await new Promise((resolve) => setTimeout(resolve, waitMs)); + // Nobody is waiting for this any more: sending it would spend the one + // request per second the policy allows on an answer with no reader. + if (bounded && Date.now() - queuedAt > NOMINATIM_MAX_WAIT_MS) + throw abandonedError(); + if (signal?.aborted) throw abandonedError(); + _nominatimLastRequestAt = Date.now(); + return await work(); + } finally { + if (bounded) _nominatimPending -= 1; + } + }); + _nominatimQueue = task.catch(() => null); + return task; +} + +/** Construct the serialized Nominatim reverse adapter with a trusted endpoint. */ export function createRegionalPlaceProvider({ endpoint = 'https://nominatim.openstreetmap.org/reverse', requestJson = fetchRegionalJson, } = {}) { - let _nominatimQueue = Promise.resolve(); - - let _nominatimLastRequestAt = 0; - function fetchRegionalPlace(point) { - const task = _nominatimQueue.then(async () => { - const waitMs = Math.max(0, 1100 - (Date.now() - _nominatimLastRequestAt)); - if (waitMs) await new Promise((resolve) => setTimeout(resolve, waitMs)); - _nominatimLastRequestAt = Date.now(); + return enqueueNominatim(async () => { const params = new URLSearchParams({ format: 'jsonv2', lat: point.latitude.toFixed(5), @@ -24,19 +127,162 @@ export function createRegionalPlaceProvider({ 'accept-language': 'en', }); const payload = await requestJson(`${endpoint}?${params}`, { - headers: { - 'User-Agent': - 'GodsEyeView/0.1 (+https://github.com/bilawalsidhu/gods-eye-view)', - Referer: 'https://github.com/bilawalsidhu/gods-eye-view', - }, + headers: NOMINATIM_HEADERS, + redirect: 'error', }); return normalizeRegionalPlace(payload); }); - _nominatimQueue = task.catch(() => null); - return task; } return fetchRegionalPlace; } export const fetchRegionalPlace = createRegionalPlaceProvider(); + +/** + * Construct the forward search adapter with a trusted endpoint. + * + * The policy asks that results be cached, and warns that a client repeating the + * same query may be treated as faulty, so an answer is remembered and identical + * searches already in flight share one upstream call rather than queueing + * behind each other. + */ +export function createNominatimSearchProvider({ + endpoint = 'https://nominatim.openstreetmap.org/search', + requestJson = fetchRegionalJson, +} = {}) { + const cache = new Map(); + const inFlight = new Map(); + + const trimCache = () => { + while (cache.size > NOMINATIM_SEARCH_MAX_CACHE) { + const oldest = cache.keys().next().value; + if (oldest === undefined) break; + cache.delete(oldest); + } + }; + + return async function fetchNominatimSearch(query, bounds, { signal } = {}) { + const cacheKey = `${query.toLowerCase()}|${bounds || ''}`; + const cached = cache.get(cacheKey); + if (cached && Date.now() - cached.cachedAt <= NOMINATIM_SEARCH_CACHE_MS) { + return { ...cached.payload, cached: true }; + } + const { promise } = coalesceProxyRequest(inFlight, cacheKey, async () => { + const params = new URLSearchParams({ + format: 'jsonv2', + q: query, + addressdetails: '1', + limit: '1', + 'accept-language': 'en', + }); + const viewbox = nominatimViewboxFromBounds(bounds); + if (viewbox) params.set('viewbox', viewbox); + const rows = await enqueueNominatim( + () => + requestJson(`${endpoint}?${params}`, { + headers: NOMINATIM_HEADERS, + redirect: 'error', + }), + { bounded: true, signal }, + ); + const result = nominatimToGeocodeResult( + Array.isArray(rows) ? rows[0] : null, + ); + const payload = result + ? { status: 'OK', results: [result] } + : { status: 'ZERO_RESULTS', results: [] }; + cache.set(cacheKey, { payload, cachedAt: Date.now() }); + trimCache(); + return payload; + }); + return await promise; + }; +} + +export const fetchNominatimSearch = createNominatimSearchProvider(); + +/** Vite plugin: last-resort place search over the public Nominatim instance. */ +export function geocodeProxy({ search = fetchNominatimSearch } = {}) { + const limiter = makeRateLimiter({ + windowMs: 60_000, + max: 30, + globalMax: 90, + }); + + function install(middlewares) { + middlewares.use('/api/geocode', async (req, res) => { + if (req.method !== 'GET') { + res.writeHead(405, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: 'Method Not Allowed' })); + return; + } + if (!limiter(clientKey(req))) { + res.writeHead(429, { + 'Content-Type': 'application/json', + 'Retry-After': '10', + }); + res.end(JSON.stringify({ error: 'Rate limit exceeded' })); + return; + } + const url = new URL(req.url || '', 'http://localhost'); + const query = String(url.searchParams.get('q') || '').trim(); + if (!query || query.length > NOMINATIM_SEARCH_MAX_QUERY) { + res.writeHead(400, { 'Content-Type': 'application/json' }); + res.end( + JSON.stringify({ + error: 'A place query of 1-200 characters is required', + }), + ); + return; + } + // A browser that gave up is no longer waiting; the queue reads this + // before spending its slot. + const abandoned = new AbortController(); + req.on?.('aborted', () => abandoned.abort()); + res.on?.('close', () => abandoned.abort()); + try { + const payload = await search(query, url.searchParams.get('bounds'), { + signal: abandoned.signal, + }); + if (res.writableEnded) return; + res.writeHead(200, { + 'Content-Type': 'application/json', + 'Cache-Control': payload.cached ? 'public, max-age=60' : 'no-store', + }); + res.end( + JSON.stringify({ status: payload.status, results: payload.results }), + ); + } catch (error) { + if (res.writableEnded) return; + const busy = + error?.code === 'NOMINATIM_QUEUE_FULL' || + error?.code === 'NOMINATIM_ABANDONED'; + res.writeHead(busy ? 429 : 503, { + 'Content-Type': 'application/json', + 'Cache-Control': 'no-store', + ...(busy ? { 'Retry-After': '5' } : {}), + }); + res.end( + JSON.stringify({ + error: busy + ? 'Place search is busy' + : 'Place search is temporarily unavailable', + }), + ); + } + }); + } + + return { + name: 'geocode-proxy', + configureServer(server) { + install(server.middlewares); + }, + configurePreviewServer(server) { + install(server.middlewares); + }, + }; +} + +export { NOMINATIM_MAX_PENDING }; diff --git a/src/data/dataCredits.js b/src/data/dataCredits.js index c946a21fc..29a76a15e 100644 --- a/src/data/dataCredits.js +++ b/src/data/dataCredits.js @@ -99,7 +99,7 @@ export const DATA_CREDITS = [ { key: 'cockpit-place-osm', html: - 'Cockpit place context: ' + + 'Cockpit place context and last-resort place search: ' + '© OpenStreetMap contributors ' + 'via Nominatim (ODbL 1.0)', }, diff --git a/src/nominatimGeocode.js b/src/nominatimGeocode.js new file mode 100644 index 000000000..b7dd6145a --- /dev/null +++ b/src/nominatimGeocode.js @@ -0,0 +1,171 @@ +/** + * Map a Nominatim search hit into the result shape the camera framing already + * reads (types + viewport). Pure — no network. + * + * Every numeric field arrives as a string from the upstream JSON and may be + * absent, blank, or nonsense. A coordinate that is not a real coordinate must + * be refused rather than coerced: `Number(null)` is 0, so a missing pair read + * as `(0, 0)` would fly the operator to the Atlantic and look like a hit. + */ + +const ADDRESS_TYPE_MAP = Object.freeze({ + country: ['country', 'political'], + nation: ['country', 'political'], + state: ['administrative_area_level_1', 'political'], + province: ['administrative_area_level_1', 'political'], + region: ['administrative_area_level_1', 'political'], + county: ['administrative_area_level_2', 'political'], + district: ['administrative_area_level_2', 'political'], + municipality: ['locality', 'political'], + city: ['locality', 'political'], + town: ['locality', 'political'], + village: ['locality', 'political'], + hamlet: ['locality', 'political'], + suburb: ['neighborhood', 'political'], + neighbourhood: ['neighborhood', 'political'], + neighborhood: ['neighborhood', 'political'], + quarter: ['neighborhood', 'political'], + postcode: ['postal_code'], + road: ['route'], + highway: ['route'], + peak: ['natural_feature'], + mountain_range: ['natural_feature'], + wood: ['natural_feature'], + forest: ['natural_feature'], + park: ['park'], + nature_reserve: ['park', 'natural_feature'], + aerodrome: ['airport'], + airport: ['airport'], + university: ['university'], + stadium: ['stadium'], + island: ['natural_feature'], +}); + +/** + * Read a field that must be a finite number inside a range. + * + * Null, undefined, blank, whitespace, booleans, arrays and anything that does + * not parse cleanly all return null. Only a string or number is considered, so + * `Number([])` and `Number(true)` cannot become 0 and 1. + * + * @param {unknown} value + * @param {number} min + * @param {number} max + * @returns {number|null} + */ +function boundedNumber(value, min, max) { + if (typeof value !== 'number' && typeof value !== 'string') return null; + if (typeof value === 'string' && value.trim() === '') return null; + const parsed = Number(value); + if (!Number.isFinite(parsed)) return null; + return parsed >= min && parsed <= max ? parsed : null; +} + +/** A latitude that is really a latitude, or null. */ +export function nominatimLatitude(value) { + return boundedNumber(value, -90, 90); +} + +/** A longitude that is really a longitude, or null. */ +export function nominatimLongitude(value) { + return boundedNumber(value, -180, 180); +} + +/** Framing types for the camera, derived from the hit's class and type. */ +export function typesFromNominatim(hit) { + const addresstype = String(hit?.addresstype || '').toLowerCase(); + const osmType = String(hit?.type || '').toLowerCase(); + const osmClass = String(hit?.class || '').toLowerCase(); + const mapped = + ADDRESS_TYPE_MAP[addresstype] || + ADDRESS_TYPE_MAP[osmType] || + (osmClass === 'highway' ? ['route'] : null) || + (osmClass === 'leisure' ? ['park'] : null) || + (osmClass === 'natural' ? ['natural_feature'] : null) || + (osmClass === 'aeroway' ? ['airport'] : null) || + (osmClass === 'amenity' && osmType === 'university' + ? ['university'] + : null); + return mapped ? [...mapped] : ['point_of_interest', 'establishment']; +} + +/** + * The hit's bounding box as a viewport, or null. + * + * Nominatim gives `[south, north, west, east]` as strings. A box with an edge + * outside the real world, an inverted edge, or a missing value is refused — + * framing a viewport with a longitude of 999 sends the camera to nowhere. + * + * @param {object} hit + * @returns {{southwest: {lat: number, lng: number}, northeast: {lat: number, lng: number}}|null} + */ +export function viewportFromNominatim(hit) { + const box = Array.isArray(hit?.boundingbox) ? hit.boundingbox : []; + if (box.length !== 4) return null; + const south = nominatimLatitude(box[0]); + const north = nominatimLatitude(box[1]); + const west = nominatimLongitude(box[2]); + const east = nominatimLongitude(box[3]); + if (south === null || north === null || west === null || east === null) + return null; + if (south > north) return null; + return { + southwest: { lat: south, lng: west }, + northeast: { lat: north, lng: east }, + }; +} + +/** + * Convert one Nominatim hit into a geocoding result, or null. + * + * @param {object} hit + * @returns {{geometry: object, formatted_address: string, types: string[], address_components: object[]}|null} + */ +export function nominatimToGeocodeResult(hit) { + const lat = nominatimLatitude(hit?.lat); + const lng = nominatimLongitude(hit?.lon); + if (lat === null || lng === null) return null; + const label = String(hit?.display_name || hit?.name || '').trim(); + if (!label) return null; + const types = typesFromNominatim(hit); + const viewport = viewportFromNominatim(hit); + const canonical = String(hit?.name || '').trim(); + return { + geometry: { + location: { lat, lng }, + ...(viewport ? { bounds: viewport, viewport } : {}), + }, + formatted_address: label, + types, + address_components: canonical + ? [{ long_name: canonical, short_name: canonical, types: [...types] }] + : [], + }; +} + +/** + * Translate a `swLat,swLng|neLat,neLng` bias into Nominatim's + * `viewbox=west,north,east,south`, or null when the string is unusable. + * + * Both edges are range-checked. A viewbox naming a longitude of 999 is not a + * hint the upstream can use, and sending it is a malformed request rather than + * a soft one. + * + * @param {string} bounds + * @returns {string|null} + */ +export function nominatimViewboxFromBounds(bounds) { + const match = String(bounds || '') + .trim() + .match( + /^(-?\d+(?:\.\d+)?),(-?\d+(?:\.\d+)?)\|(-?\d+(?:\.\d+)?),(-?\d+(?:\.\d+)?)$/, + ); + if (!match) return null; + const swLat = nominatimLatitude(match[1]); + const swLng = nominatimLongitude(match[2]); + const neLat = nominatimLatitude(match[3]); + const neLng = nominatimLongitude(match[4]); + if ([swLat, swLng, neLat, neLng].some((value) => value === null)) return null; + if (swLat >= neLat) return null; + return `${swLng},${neLat},${neLng},${swLat}`; +} diff --git a/src/nominatimGeocode.test.mjs b/src/nominatimGeocode.test.mjs new file mode 100644 index 000000000..b139ad820 --- /dev/null +++ b/src/nominatimGeocode.test.mjs @@ -0,0 +1,175 @@ +// NOMINATIM RESULT MAPPING — what counts as a usable hit. +// +// Every numeric field arrives as a string and may be absent or nonsense, and +// `Number(null)` is 0: a hit with no coordinates read as (0, 0) would fly the +// camera to the Atlantic and look like a successful search. These cases pin +// every field that must be refused rather than coerced. +// +// Run with: npm test +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { + nominatimLatitude, + nominatimLongitude, + nominatimToGeocodeResult, + nominatimViewboxFromBounds, + typesFromNominatim, + viewportFromNominatim, +} from './nominatimGeocode.js'; + +test('a coordinate field is a number in range, or nothing', () => { + assert.equal(nominatimLatitude('43.1731'), 43.1731); + assert.equal(nominatimLatitude(-33.8568), -33.8568); + assert.equal(nominatimLatitude('90'), 90); + assert.equal(nominatimLongitude('-180'), -180); + + for (const value of [ + null, + undefined, + '', + ' ', + 'abc', + NaN, + Infinity, + true, + false, + [], + ['43'], + {}, + '91', + '-90.5', + ]) + assert.equal( + nominatimLatitude(value), + null, + `${JSON.stringify(value)} is not a latitude`, + ); + + for (const value of [null, '', '181', '-180.5', true, []]) + assert.equal( + nominatimLongitude(value), + null, + `${JSON.stringify(value)} is not a longitude`, + ); +}); + +test('a hit without real coordinates produces no result at all', () => { + for (const hit of [ + null, + undefined, + {}, + { lat: null, lon: null, display_name: 'Nowhere' }, + { lat: '', lon: '', display_name: 'Nowhere' }, + { lat: '43.17', lon: null, display_name: 'Half a place' }, + { lat: null, lon: '-79.03', display_name: 'Half a place' }, + { lat: '91', lon: '0', display_name: 'Off the pole' }, + { lat: '0', lon: '999', display_name: 'Off the world' }, + // Coordinates but no name: nothing to show the operator. + { lat: '43.17', lon: '-79.03' }, + { lat: '43.17', lon: '-79.03', display_name: ' ' }, + ]) + assert.equal( + nominatimToGeocodeResult(hit), + null, + `${JSON.stringify(hit)} must not become a result`, + ); +}); + +test('a real hit carries its point, label, types and box', () => { + const result = nominatimToGeocodeResult({ + lat: '43.1731', + lon: '-79.0384', + display_name: 'Niagara Falls, Ontario, Canada', + name: 'Niagara Falls', + addresstype: 'city', + boundingbox: ['43.0500', '43.1500', '-79.1500', '-79.0000'], + }); + assert.deepEqual(result.geometry.location, { lat: 43.1731, lng: -79.0384 }); + assert.equal(result.formatted_address, 'Niagara Falls, Ontario, Canada'); + assert.deepEqual(result.types, ['locality', 'political']); + assert.deepEqual(result.geometry.bounds, { + southwest: { lat: 43.05, lng: -79.15 }, + northeast: { lat: 43.15, lng: -79 }, + }); + assert.equal(result.address_components[0].long_name, 'Niagara Falls'); +}); + +test('a box with an impossible or inverted edge is dropped, not framed', () => { + for (const boundingbox of [ + undefined, + [], + ['43.05', '43.15', '-79.15'], + ['43.05', '43.15', '-79.15', '999'], + ['43.05', '43.15', '-999', '-79.0'], + ['-91', '43.15', '-79.15', '-79.0'], + ['43.05', '91', '-79.15', '-79.0'], + [null, '43.15', '-79.15', '-79.0'], + ['', '43.15', '-79.15', '-79.0'], + // South above north is not a box. + ['43.15', '43.05', '-79.15', '-79.0'], + ]) { + assert.equal( + viewportFromNominatim({ boundingbox }), + null, + `${JSON.stringify(boundingbox)} must not become a viewport`, + ); + const result = nominatimToGeocodeResult({ + lat: '43.1731', + lon: '-79.0384', + display_name: 'Somewhere', + boundingbox, + }); + assert.ok(result, 'the point itself still stands'); + assert.equal( + result.geometry.bounds, + undefined, + 'a place with no usable box is framed as a point', + ); + } +}); + +test('types fall back to a place rather than guessing a category', () => { + assert.deepEqual(typesFromNominatim({ addresstype: 'country' }), [ + 'country', + 'political', + ]); + assert.deepEqual(typesFromNominatim({ class: 'highway', type: 'primary' }), [ + 'route', + ]); + assert.deepEqual(typesFromNominatim({}), [ + 'point_of_interest', + 'establishment', + ]); + assert.deepEqual(typesFromNominatim(null), [ + 'point_of_interest', + 'establishment', + ]); +}); + +test('a viewport hint is sent only when every edge is a real coordinate', () => { + assert.equal( + nominatimViewboxFromBounds('30.1,-97.95|30.52,-97.55'), + '-97.95,30.52,-97.55,30.1', + ); + + for (const bounds of [ + null, + undefined, + '', + 'not bounds', + '30.1,-97.95', + // Out of range on each edge in turn. + '-91,-97.95|30.52,-97.55', + '30.1,-999|30.52,-97.55', + '30.1,-97.95|91,-97.55', + '30.1,-97.95|30.52,999', + // Inverted latitudes are not a box. + '30.52,-97.95|30.1,-97.55', + '30.1,-97.95|30.1,-97.55', + ]) + assert.equal( + nominatimViewboxFromBounds(bounds), + null, + `${JSON.stringify(bounds)} must not become a viewbox`, + ); +}); diff --git a/src/search/defaults.js b/src/search/defaults.js index eab20333f..f97ddbf17 100644 --- a/src/search/defaults.js +++ b/src/search/defaults.js @@ -8,8 +8,8 @@ import { createPresetGeocoder } from './presetGeocoder.js'; /** * Coordinates and bundled names first — both answer offline and with no key — - * then Google when configured, then keyless Photon. Transport stays local to - * setup. + * then Google when configured, then keyless Photon, then the local Nominatim + * route as a last resort. Transport stays local to setup. * * `presets` is the caller's bundled place data. It is passed in rather than * imported so this package keeps reading no application state; with none @@ -43,6 +43,21 @@ export function createDefaultPlaceSearch({ }, }), createPhotonGeocoder({ fetchImpl, endpoint: endpoints.photon }), + // Last resort: the local Nominatim route, which answers with no key when + // neither of the two above did. It speaks the same result shape, so it + // rides the existing Google adapter rather than needing its own. + createGoogleGeocoder({ + request(query, { bias, signal }) { + const params = new URLSearchParams({ q: query }); + if (bias) params.set('bounds', bias); + return fetchImpl( + `${endpoints.nominatim || '/api/geocode'}?${params}`, + { + signal, + }, + ); + }, + }), ], }); return { diff --git a/src/tooling/nominatimSearchRoute.test.mjs b/src/tooling/nominatimSearchRoute.test.mjs new file mode 100644 index 000000000..f0cfa0572 --- /dev/null +++ b/src/tooling/nominatimSearchRoute.test.mjs @@ -0,0 +1,368 @@ +// THE LAST-RESORT PLACE SEARCH ROUTE — what it is allowed to send upstream. +// +// The public instance permits one request per second. One request per second +// and an unbounded queue are incompatible: a burst of searches would keep the +// upstream busy long after everyone who asked has given up. These cases pin the +// queue bound, the shared answer for identical searches, the cancellation path, +// and the chain order that only reaches here when nothing else answered. +// +// Run with: npm test +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { + fetchNominatimSearch, + geocodeProxy, + NOMINATIM_MAX_PENDING, +} from '../../server/providers/regional/place.js'; +import { fetchRegionalJson } from '../../server/providers/regional/http.js'; +import { localProviderPlugins } from '../../server/providers/local.js'; +import { createPlaceSearch, createGoogleGeocoder } from '../search/index.js'; +import { createPhotonGeocoder } from '../keylessGeocoder.js'; + +const HIT = [ + { + lat: '43.1731', + lon: '-79.0384', + display_name: 'Niagara Falls, Ontario, Canada', + name: 'Niagara Falls', + addresstype: 'city', + boundingbox: ['43.05', '43.15', '-79.15', '-79.0'], + }, +]; + +/** Mount the geocode route the way the dev server does. */ +let clientCounter = 0; + +function mountGeocode( + install = (server) => geocodeProxy().configureServer(server), +) { + // A fresh client address per mount: the per-client rate limiter is module + // state shared by every case in this file, and one case's burst must not + // decide another case's outcome. + clientCounter += 1; + const remoteAddress = `10.0.0.${clientCounter % 250}`; + const routes = new Map(); + install({ + middlewares: { + use(route, handler) { + routes.set(route, handler); + }, + }, + }); + return async function request(path) { + const requested = new URL(path, 'http://localhost'); + const handler = routes.get('/api/geocode'); + if (!handler) throw new Error('no /api/geocode route'); + // connect strips the mount prefix before the handler sees the request. + const rest = + requested.pathname.slice('/api/geocode'.length) + requested.search; + const req = { + method: 'GET', + url: rest[0] === '/' ? rest : `/${rest}`, + headers: {}, + socket: { remoteAddress }, + on() {}, + }; + return await new Promise((resolve, reject) => { + let status = 200; + let headers = {}; + const res = { + writableEnded: false, + on() {}, + writeHead(code, sent) { + status = code; + headers = sent || {}; + return res; + }, + end(body = '') { + res.writableEnded = true; + let parsed = null; + try { + parsed = body ? JSON.parse(String(body)) : null; + } catch { + parsed = String(body); + } + resolve({ status, headers, body: parsed }); + }, + }; + Promise.resolve(handler(req, res)).catch(reject); + }); + }; +} + +/** Answer the upstream from a stub, counting and optionally stalling calls. */ +async function withUpstream(respond, run) { + const previous = globalThis.fetch; + const calls = []; + globalThis.fetch = async (url, options) => { + calls.push({ url: String(url), options }); + return await respond(String(url), options, calls.length); + }; + try { + return await run(calls); + } finally { + globalThis.fetch = previous; + } +} + +const jsonResponse = (value) => + new Response(JSON.stringify(value), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + +test('the route is part of the application on both dev and preview', () => { + const plugin = localProviderPlugins().find( + (entry) => entry.name === 'geocode-proxy', + ); + assert.ok(plugin, 'the application must compose the place-search route'); + assert.equal(typeof plugin.configureServer, 'function'); + assert.equal(typeof plugin.configurePreviewServer, 'function'); +}); + +test('a search reaches the upstream identified, and the answer is cached', async () => { + const request = mountGeocode(); + await withUpstream( + async () => jsonResponse(HIT), + async (calls) => { + const query = `niagara-${Date.now()}`; + const first = await request(`/api/geocode?q=${query}`); + assert.equal(first.status, 200); + assert.equal(first.body.status, 'OK'); + assert.deepEqual(first.body.results[0].geometry.location, { + lat: 43.1731, + lng: -79.0384, + }); + assert.equal(calls.length, 1); + const headers = calls[0].options.headers; + assert.match( + String(headers['User-Agent']), + /gods-eye-view\/\d/, + 'the policy asks for a User-Agent identifying the application', + ); + assert.ok(headers.Referer, 'a Referer is sent alongside it'); + assert.equal( + calls[0].options.redirect, + 'error', + 'a fixed API endpoint may not be redirected elsewhere', + ); + + // The policy asks that results be cached; a repeat costs no request. + const repeat = await request(`/api/geocode?q=${query}`); + assert.equal(repeat.status, 200); + assert.equal( + calls.length, + 1, + 'the second identical search is served from cache', + ); + }, + ); +}); + +test('identical searches in flight share one upstream call', async () => { + const request = mountGeocode(); + const query = `coalesce-${Date.now()}`; + await withUpstream( + async () => { + await new Promise((resolve) => setTimeout(resolve, 40)); + return jsonResponse(HIT); + }, + async (calls) => { + const answers = await Promise.all([ + request(`/api/geocode?q=${query}`), + request(`/api/geocode?q=${query}`), + request(`/api/geocode?q=${query}`), + ]); + for (const answer of answers) assert.equal(answer.status, 200); + assert.equal(calls.length, 1, 'one upstream call serves all three'); + }, + ); +}); + +test('a burst past the queue bound is refused rather than queued for minutes', async () => { + const request = mountGeocode(); + const stamp = Date.now(); + await withUpstream( + async () => jsonResponse([]), + async (calls) => { + const answers = await Promise.all( + Array.from({ length: 20 }, (_unused, index) => + request(`/api/geocode?q=burst-${stamp}-${index}`), + ), + ); + const refused = answers.filter((answer) => answer.status === 429); + assert.ok( + refused.length > 0, + 'a burst this size must be refused somewhere', + ); + for (const answer of refused) + assert.equal(answer.headers['Retry-After'], '5'); + assert.ok( + calls.length <= NOMINATIM_MAX_PENDING + 1, + `the upstream saw ${calls.length} of 20 searches; the queue bound is ${NOMINATIM_MAX_PENDING}`, + ); + assert.equal( + answers.length, + 20, + 'every caller is answered, none is left hanging', + ); + }, + ); +}); + +test('a query that is missing or too long never reaches the upstream', async () => { + const request = mountGeocode(); + await withUpstream( + async () => jsonResponse(HIT), + async (calls) => { + for (const path of [ + '/api/geocode', + '/api/geocode?q=', + '/api/geocode?q=%20%20', + `/api/geocode?q=${'x'.repeat(201)}`, + ]) { + const answer = await request(path); + assert.equal(answer.status, 400, path); + } + assert.deepEqual(calls, []); + }, + ); +}); + +test('an upstream failure is a service answer, not a crash', async () => { + const request = mountGeocode(); + await withUpstream( + async () => new Response('upstream down', { status: 502 }), + async () => { + const answer = await request(`/api/geocode?q=down-${Date.now()}`); + assert.equal(answer.status, 503); + assert.equal(answer.headers['Cache-Control'], 'no-store'); + assert.match(String(answer.body.error), /temporarily unavailable/); + }, + ); +}); + +test('a search with no hit is a clean empty answer', async () => { + const request = mountGeocode(); + await withUpstream( + async () => jsonResponse([]), + async () => { + const answer = await request(`/api/geocode?q=nowhere-${Date.now()}`); + assert.equal(answer.status, 200); + assert.equal(answer.body.status, 'ZERO_RESULTS'); + assert.deepEqual(answer.body.results, []); + }, + ); +}); + +test('the chain reaches this route only when Google and Photon do not answer', async () => { + const asked = []; + const request = mountGeocode(); + await withUpstream( + async (url) => { + if (url.includes('photon.komoot.io')) { + asked.push('photon'); + return new Response('photon is down', { status: 503 }); + } + asked.push('nominatim'); + return jsonResponse(HIT); + }, + async () => { + const search = createPlaceSearch({ + providers: [ + createGoogleGeocoder({ + request() { + asked.push('google'); + // An unconfigured provider contributes no verdict. + return null; + }, + }), + createPhotonGeocoder({ + fetchImpl: (...args) => globalThis.fetch(...args), + }), + createGoogleGeocoder({ + request(query) { + return request( + `/api/geocode?q=${encodeURIComponent(query)}`, + ).then( + (answer) => + new Response(JSON.stringify(answer.body), { + status: answer.status, + }), + ); + }, + }), + ], + }); + + const outcome = await search.geocode(`chain-${Date.now()}`); + assert.deepEqual( + asked, + ['google', 'photon', 'nominatim'], + `Google first, then Photon, then this route (saw ${asked.join(' → ')})`, + ); + assert.equal(outcome.place.lat, 43.1731); + assert.equal(outcome.fallbackUsed, true); + }, + ); +}); + +test('a search whose caller gave up is dropped before it costs a slot', async () => { + const abandoned = new AbortController(); + abandoned.abort(); + await withUpstream( + async () => jsonResponse(HIT), + async (calls) => { + await assert.rejects( + fetchNominatimSearch(`cancelled-${Date.now()}`, null, { + signal: abandoned.signal, + }), + (error) => error.code === 'NOMINATIM_ABANDONED', + ); + assert.deepEqual(calls, [], 'a search nobody is waiting for is not sent'); + }, + ); +}); + +test('the upstream deadline covers the body, not just the headers', async () => { + // A response whose headers arrive and whose body then never does. Before the + // deadline covered the body, the abort timer was already cleared by the time + // the first byte was due, so this read had no deadline at all. + const stalled = () => { + let cancelled = false; + const stream = new ReadableStream({ + pull() { + return new Promise(() => {}); + }, + cancel() { + cancelled = true; + }, + }); + return { + response: new Response(stream, { status: 200 }), + wasCancelled: () => cancelled, + }; + }; + + const previous = globalThis.fetch; + const { response, wasCancelled } = stalled(); + globalThis.fetch = async (_url, options) => { + // The same signal must reach the body read, or aborting stops nothing. + options.signal.addEventListener('abort', () => { + void response.body?.cancel().catch(() => {}); + }); + return response; + }; + try { + const started = Date.now(); + await assert.rejects( + fetchRegionalJson('https://example.test/stalls', { timeoutMs: 300 }), + 'a stalled body must fail rather than hang', + ); + const elapsed = Date.now() - started; + assert.ok(elapsed < 5000, `gave up after ${elapsed} ms`); + assert.ok(wasCancelled(), 'the abort reached the body read'); + } finally { + globalThis.fetch = previous; + } +});