From 1db55f734c29d48f76703c4018cb6382b7ba62c1 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 29 Aug 2026 21:05:02 +0200 Subject: [PATCH] fix(maps): wait longer than the query we ask Overpass to run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The query carries `[timeout:20]`, so the mirror is allowed twenty seconds of work on it — and the fetch around it was aborted after twelve. A mirror under load that would have answered at fifteen never got the chance, and the caller saw a failed area. That is what makes a corridor search along a long drive report half its stretches as unsearchable: the stretches were being searched, we just hung up. Both numbers now come from one constant, the client budget is the query's own plus room to hand the answer back, and a test holds the relationship rather than the two literals. --- server/src/app-config/derive.ts | 4 +++- server/src/nest/maps/maps.helpers.ts | 16 +++++++++++++- server/src/nest/maps/maps.service.ts | 3 ++- server/tests/unit/nest/maps.service.test.ts | 24 +++++++++++++++------ 4 files changed, 37 insertions(+), 10 deletions(-) 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); }); });