mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-22 06:56:35 +00:00
fix(maps): stop asking an index that is not answering, and untie the place cache
Three things an install feels rather than reads. A service that cannot be reached cost the full timeout on every call. Each call site falls back to the old path, but pays 3.5 s first — so an instance behind a firewall that drops rather than refuses, or one that simply has no route to the service, pays it on every keystroke of autocomplete and on all three queries a booking import runs per venue. Ten venues is over a hundred seconds of waiting for an answer that was never coming. A process-wide breaker now fails instantly after four consecutive failures and tries again a minute later, so an unreachable index costs one timeout instead of one per request. A 404 does not count against it, because "no such place" is an answer, and neither does an oversized body, because the service replied. The offline place cache hung off the map-tile switch. That setting is about tiles — its own text says "off keeps the cache to trip data + documents only" — and it gets turned off for space, tens of megabytes of it. A trip's places are about one. Somebody who turned tiles off therefore got an empty offline search with no explanation, and "prepare for offline" reported success having stored no place at all. The two run independently now; the tile progress phase stays gated on the tile setting, so a run without tiles still reports no tile phase. Tests for the merged search path, which had none: the wiring around mergeSearchResults was uncovered, so which list goes in first and what `source` says were both free to change. Neither is cosmetic — index-first is the whole point of the feature, and the search log writes `source` into the corpus a candidate index gets scored against later. Two of the eight cases pin the fix from the previous commit: an index failure keeps the OpenStreetMap answer, and neither source is asked twice. place-shadow lands on its own coverage entry rather than the 80 catch-all, where a new domain could shed twenty points unnoticed. Pinned under its measured figure on purpose; the exact ratchet gets regenerated with the repo's own script after a full run.
This commit is contained in:
@@ -97,6 +97,51 @@ function instanceToken(): string {
|
||||
return `trek-${(hash >>> 0).toString(36)}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop asking a service that is not answering.
|
||||
*
|
||||
* Every call site here is in front of something a person is waiting for, and
|
||||
* each one falls back to the old path when the index fails — but it pays the
|
||||
* timeout first, every single time. An instance behind a firewall that drops
|
||||
* rather than refuses, or one whose network simply cannot reach the service,
|
||||
* therefore pays it on every keystroke of autocomplete and on every one of the
|
||||
* three queries a booking import runs per venue. Ten venues is over a hundred
|
||||
* seconds of waiting for an answer that was never going to come.
|
||||
*
|
||||
* So: after a few consecutive failures, fail instantly for a while instead of
|
||||
* dialling out. One request pays the timeout, the rest are free, and after the
|
||||
* cooldown the next call tries again for real — if it works, the count resets
|
||||
* and nothing was permanently switched off.
|
||||
*
|
||||
* Counted per process, deliberately. This guards the instance's own latency,
|
||||
* not the service, so it neither needs nor deserves storage.
|
||||
*/
|
||||
const BREAKER_FAILURES_BEFORE_OPEN = 4;
|
||||
const BREAKER_COOLDOWN_MS = 60_000;
|
||||
let breakerFailures = 0;
|
||||
let breakerOpenUntil = 0;
|
||||
|
||||
/**
|
||||
* Whether a failure says anything about reachability.
|
||||
*
|
||||
* A 404 does not: "no such place" is a perfectly good answer, and
|
||||
* trekPlacesById turns it into null. Nor does an oversized body, which means
|
||||
* the service answered and answered too much. What counts is a request that
|
||||
* never came back, and a server error, because both mean asking again costs the
|
||||
* same and returns the same.
|
||||
*/
|
||||
function countsAgainstReachability(err: unknown): boolean {
|
||||
const status = (err as { status?: number } | null)?.status;
|
||||
if (typeof status === 'number') return status >= 500 || status === 429;
|
||||
return !(err instanceof Error && err.message === 'TREK Places API response too large');
|
||||
}
|
||||
|
||||
/** Test seam: the breaker is process state, so a suite has to be able to clear it. */
|
||||
export function resetTrekPlacesBreaker(): void {
|
||||
breakerFailures = 0;
|
||||
breakerOpenUntil = 0;
|
||||
}
|
||||
|
||||
export function trekPlacesBaseUrl(): string {
|
||||
const configured = (readEnv().maps.trekPlacesUrl || '').trim();
|
||||
return (configured || DEFAULT_TREK_PLACES_URL).replace(/\/+$/, '');
|
||||
@@ -112,6 +157,10 @@ async function getJson<T>(
|
||||
if (v !== undefined && v !== null && v !== '') url.searchParams.set(k, String(v));
|
||||
}
|
||||
|
||||
if (Date.now() < breakerOpenUntil) {
|
||||
throw new Error('TREK Places API unreachable, not retrying yet');
|
||||
}
|
||||
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(() => controller.abort(), opts.timeoutMs ?? TIMEOUT_MS);
|
||||
try {
|
||||
@@ -130,7 +179,25 @@ async function getJson<T>(
|
||||
// Read with a cap rather than res.json(): a hostile or broken upstream
|
||||
// should not be able to hand us an unbounded body to buffer.
|
||||
const text = await readCapped(res, opts.maxBytes ?? MAX_BYTES);
|
||||
return JSON.parse(text) as T;
|
||||
const parsed = JSON.parse(text) as T;
|
||||
// An answer of any kind means the service is there.
|
||||
breakerFailures = 0;
|
||||
return parsed;
|
||||
} catch (err: unknown) {
|
||||
if (countsAgainstReachability(err)) {
|
||||
breakerFailures += 1;
|
||||
if (breakerFailures >= BREAKER_FAILURES_BEFORE_OPEN) {
|
||||
breakerOpenUntil = Date.now() + BREAKER_COOLDOWN_MS;
|
||||
// Back to the threshold rather than to zero: when the cooldown ends,
|
||||
// one more failure re-opens the breaker instead of buying another four
|
||||
// full timeouts.
|
||||
breakerFailures = BREAKER_FAILURES_BEFORE_OPEN - 1;
|
||||
}
|
||||
} else {
|
||||
// The service answered. A 404 is an answer.
|
||||
breakerFailures = 0;
|
||||
}
|
||||
throw err;
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user