diff --git a/server/src/app-config/derive.ts b/server/src/app-config/derive.ts index ec444b9cc..793d0e86f 100644 --- a/server/src/app-config/derive.ts +++ b/server/src/app-config/derive.ts @@ -245,7 +245,9 @@ export function deriveIntegrations(raw: RawEnv) { unsplashAccessKey: raw.UNSPLASH_ACCESS_KEY?.trim(), transitApiBase: stripTrailingSlashes(raw.TRANSIT_API_URL || 'https://api.transitous.org'), overpassUrl: raw.OVERPASS_URL, - overpassTimeoutMs: positiveNumberOr(raw.OVERPASS_TIMEOUT_MS, 12000), + // Longer than the `[timeout:20]` the query itself carries, or we abort an answer the + // mirror was still allowed to be working on. See OVERPASS_QUERY_TIMEOUT_S. + overpassTimeoutMs: positiveNumberOr(raw.OVERPASS_TIMEOUT_MS, 25000), kitineraryExtractorPath: raw.KITINERARY_EXTRACTOR_PATH, /** * One ceiling for a model call, replacing the three per-client constants diff --git a/server/src/nest/maps/maps.helpers.ts b/server/src/nest/maps/maps.helpers.ts index 588f419e5..07b9d1f01 100644 --- a/server/src/nest/maps/maps.helpers.ts +++ b/server/src/nest/maps/maps.helpers.ts @@ -362,10 +362,24 @@ export function resolveOverpassEndpoints(raw: string | undefined = readEnv().int // slow self-hosted endpoint can raise it via OVERPASS_TIMEOUT_MS. A non-positive or // non-numeric value falls back to the default — a 0/negative cap would abort every // request immediately and 502 the search. +/** + * How long Overpass is allowed to spend on one query, in seconds. + * + * Sent inside the query as `[timeout:N]`, so the mirror itself enforces it. Anything we + * wait client-side has to be longer than this or we hang up on an answer that was still + * coming — which is exactly what used to happen: the query asked for twenty seconds of + * work and the fetch was aborted after twelve, so a mirror under load never got to + * finish and a corridor search reported half its stretches as unsearchable. + */ +export const OVERPASS_QUERY_TIMEOUT_S = 20; + +/** The client-side budget: the mirror's own, plus room to hand the answer back. */ +export const OVERPASS_TIMEOUT_DEFAULT_MS = (OVERPASS_QUERY_TIMEOUT_S + 5) * 1000; + export function resolveOverpassTimeoutMs(raw?: string): number { if (raw === undefined) return readEnv().integrations.overpassTimeoutMs; const n = Number(raw); - return Number.isFinite(n) && n > 0 ? n : 12000; + return Number.isFinite(n) && n > 0 ? n : OVERPASS_TIMEOUT_DEFAULT_MS; } // ── Opening hours parsing ──────────────────────────────────────────────────── diff --git a/server/src/nest/maps/maps.service.ts b/server/src/nest/maps/maps.service.ts index 52ce87aee..bf022ef51 100644 --- a/server/src/nest/maps/maps.service.ts +++ b/server/src/nest/maps/maps.service.ts @@ -39,6 +39,7 @@ import { parsePoiCategories, resolveOverpassEndpoints, resolveOverpassTimeoutMs, + OVERPASS_QUERY_TIMEOUT_S, stripWikiMarkup, parseWikipediaTag, toWikiLang, @@ -1224,7 +1225,7 @@ export class MapsService { .join('\n'); // `out center tags ` returns ways/relations with a computed center and caps // the result count in one round-trip. - const query = `[out:json][timeout:20];\n(\n${selectors}\n);\nout center tags ${cap + 25};`; + const query = `[out:json][timeout:${OVERPASS_QUERY_TIMEOUT_S}];\n(\n${selectors}\n);\nout center tags ${cap + 25};`; const elements = await overpassFetch(query); diff --git a/server/tests/unit/nest/maps.service.test.ts b/server/tests/unit/nest/maps.service.test.ts index a78b437e1..d148beef1 100644 --- a/server/tests/unit/nest/maps.service.test.ts +++ b/server/tests/unit/nest/maps.service.test.ts @@ -20,6 +20,8 @@ import { buildUserAgent, resolveOverpassEndpoints, resolveOverpassTimeoutMs, + OVERPASS_QUERY_TIMEOUT_S, + OVERPASS_TIMEOUT_DEFAULT_MS, stripWikiMarkup, parseWikipediaTag, rankCommonsCandidates, @@ -2487,10 +2489,10 @@ describe('resolveOverpassEndpoints', () => { // ── resolveOverpassTimeoutMs (OVERPASS_TIMEOUT_MS override, #1309) ───────────── describe('resolveOverpassTimeoutMs', () => { - it('MAPS-104: falls back to the 12s default for unset / empty / non-numeric values', () => { - expect(resolveOverpassTimeoutMs(undefined)).toBe(12000); - expect(resolveOverpassTimeoutMs('')).toBe(12000); - expect(resolveOverpassTimeoutMs('abc')).toBe(12000); + it('MAPS-104: falls back to the default for unset / empty / non-numeric values', () => { + expect(resolveOverpassTimeoutMs(undefined)).toBe(OVERPASS_TIMEOUT_DEFAULT_MS); + expect(resolveOverpassTimeoutMs('')).toBe(OVERPASS_TIMEOUT_DEFAULT_MS); + expect(resolveOverpassTimeoutMs('abc')).toBe(OVERPASS_TIMEOUT_DEFAULT_MS); }); it('MAPS-105: honours a positive numeric override', () => { @@ -2498,9 +2500,17 @@ describe('resolveOverpassTimeoutMs', () => { }); it('MAPS-106: rejects 0, negative and Infinity — a non-positive cap would 502 every search', () => { - expect(resolveOverpassTimeoutMs('0')).toBe(12000); - expect(resolveOverpassTimeoutMs('-5')).toBe(12000); - expect(resolveOverpassTimeoutMs('Infinity')).toBe(12000); + expect(resolveOverpassTimeoutMs('0')).toBe(OVERPASS_TIMEOUT_DEFAULT_MS); + expect(resolveOverpassTimeoutMs('-5')).toBe(OVERPASS_TIMEOUT_DEFAULT_MS); + expect(resolveOverpassTimeoutMs('Infinity')).toBe(OVERPASS_TIMEOUT_DEFAULT_MS); + }); + + it('MAPS-107: we wait longer than the query we sent is allowed to take', () => { + // The query carries `[timeout:N]`, which the mirror enforces itself. Hanging up + // before that expires aborts an answer that was still legitimately being computed — + // which is how a corridor search came to report half its stretches as unsearchable + // while every mirror was in fact working on them. + expect(OVERPASS_TIMEOUT_DEFAULT_MS).toBeGreaterThan(OVERPASS_QUERY_TIMEOUT_S * 1000); }); });