diff --git a/client/src/components/Map/valhallaDayRoute.test.ts b/client/src/components/Map/valhallaDayRoute.test.ts index c83bdbf5e..3a77c441f 100644 --- a/client/src/components/Map/valhallaDayRoute.test.ts +++ b/client/src/components/Map/valhallaDayRoute.test.ts @@ -192,6 +192,23 @@ describe('a day where Valhalla routes the days', () => { expect([traffic.fossgis.length, traffic.osrm.length]).toEqual([0, 1]); }); + it('VDAY-004c: a day the relay ran out of time on goes to OSRM alone, and the next day asks the relay again', async () => { + let slow = true; + const traffic = listen((body) => + slow + ? HttpResponse.json({ error: 'The TREK routing service took too long for this route' }, { status: 504 }) + : answered(body) + ); + const fallen = await calculateRouteWithLegs(freshDay(3), { profile: 'cycling' }); + expect([fallen.distance, fallen.engine]).toEqual([2000, 'osrm']); + expect([traffic.relay.length, traffic.fossgis.length, traffic.osrm.length]).toEqual([1, 0, 1]); + + slow = false; + const next = await calculateRouteWithLegs(freshDay(3), { profile: 'cycling' }); + expect([next.distance, next.engine]).toEqual([4000, 'valhalla']); + expect([traffic.relay.length, traffic.osrm.length]).toEqual([2, 1]); + }); + it('VDAY-004b: an own Valhalla that does not answer in time hands the day to OSRM without a second wait', async () => { choose({ valhalla_base_url: OWN_VALHALLA }); const timeout = AbortSignal.timeout.bind(AbortSignal); diff --git a/client/src/components/Map/valhallaRoute.test.ts b/client/src/components/Map/valhallaRoute.test.ts index a2b7c886d..d3df67d46 100644 --- a/client/src/components/Map/valhallaRoute.test.ts +++ b/client/src/components/Map/valhallaRoute.test.ts @@ -364,6 +364,7 @@ describe('valhallaRun over a whole day', () => { costing: 'pedestrian', costing_options: { pedestrian: { max_hiking_difficulty: difficulty, use_ferry: 0 } }, directions_options: { units: 'kilometers' }, + directions_type: 'maneuvers', }) }) @@ -469,6 +470,53 @@ describe('valhallaRun over a whole day', () => { expect(calls).toBe(3) }) + it('VALHALLA-RUN-009: asks the relay twenty locations at a time with no pause between the pieces', async () => { + const sizes: number[] = [] + let fossgis = 0 + server.use( + http.post('/api/routing/valhalla/:action', async ({ request }) => { + const body = await request.json() as { locations: unknown[] } + sizes.push(body.locations.length) + return HttpResponse.json(chain(body.locations.length - 1)) + }), + http.post(FOSSGIS_VALHALLA, () => { fossgis++; return HttpResponse.json(chain(1)) }), + ) + const started = Date.now() + + const avoiding = await valhallaRun(stops(14), 'driving', ['toll']) + const long = await valhallaRun(stops(25), 'driving', ['toll']) + + expect(avoiding?.legs).toHaveLength(13) + expect(long?.legs).toHaveLength(24) + expect(sizes).toEqual([14, 20, 6]) + // The pause between pieces was 1.1 s; the relay and an own Valhalla are not paced. + expect(Date.now() - started).toBeLessThan(1000) + expect(fossgis).toBe(0) + }) + + it('VALHALLA-RUN-010: a day cut for the relay that the relay cannot answer is cut again in tens for FOSSGIS', async () => { + const relayed: number[] = [] + const sizes: number[] = [] + server.use( + http.post('/api/routing/valhalla/:action', async ({ request }) => { + relayed.push(((await request.json()) as { locations: unknown[] }).locations.length) + return HttpResponse.json({ error: 'down' }, { status: 502 }) + }), + http.post(FOSSGIS_VALHALLA, async ({ request }) => { + const body = await request.json() as { locations: unknown[] } + sizes.push(body.locations.length) + return HttpResponse.json(chain(body.locations.length - 1)) + }), + ) + + const run = await valhallaRun(stops(14), 'driving', ['toll']) + + expect(run?.legs).toHaveLength(13) + expect(relayed).toEqual([14]) + // FOSSGIS never saw the piece of fourteen it would have refused with error 150. + expect(sizes).toEqual([10, 5]) + }) + it('VALHALLA-RUN-005: fewer than two points is nothing to route', async () => { let calls = 0 server.use(http.post(FOSSGIS_VALHALLA, () => { calls++; return HttpResponse.json(chain(1)) })) @@ -735,10 +783,34 @@ describe('through the TREK services (routing relay)', () => { expect(leg?.distance).toBeCloseTo(295700, 0) expect(relayed).toHaveLength(1) expect(relayed[0].action).toBe('route') - expect(relayed[0].body).toMatchObject({ costing: 'auto', costing_options: { auto: { use_tolls: 0 } } }) + expect(relayed[0].body).toMatchObject({ + costing: 'auto', + costing_options: { auto: { use_tolls: 0 } }, + directions_type: 'maneuvers', + }) expect(fossgis).toBe(0) }) + it('VALHALLA-RELAY-009: a question the relay ran out of time on has no answer, without resting the relay or asking FOSSGIS', async () => { + let relayCalls = 0 + let fossgis = 0 + server.use( + http.post(RELAY, () => { + relayCalls++ + return HttpResponse.json({ error: 'The TREK routing service took too long for this route' }, { status: 504 }) + }), + http.post(FOSSGIS_VALHALLA, () => { fossgis++; return HttpResponse.json(answer()) }), + ) + await expect(valhallaRouteAvoiding(HAMBURG, BERLIN, 'cycling', 'ferry')).resolves.toBeNull() + // Asked once: the relay would only run out of time on it again. + expect([relayCalls, fossgis]).toEqual([1, 0]) + expect(valhallaBase()).toBe(TREK_VALHALLA) + + server.use(http.post(RELAY, () => { relayCalls++; return HttpResponse.json(answer()) })) + await expect(valhallaRouteAvoiding(HAMBURG, BERLIN, 'driving', 'toll')).resolves.not.toBeNull() + expect([relayCalls, fossgis]).toEqual([2, 0]) + }) + it('VALHALLA-RELAY-002: turns to FOSSGIS without a word where the relay cannot answer, and leaves it alone a while', async () => { for (const status of [401, 429, 503]) { resetValhallaTurns() diff --git a/client/src/components/Map/valhallaRoute.ts b/client/src/components/Map/valhallaRoute.ts index d610c0ddd..ef3b9c096 100644 --- a/client/src/components/Map/valhallaRoute.ts +++ b/client/src/components/Map/valhallaRoute.ts @@ -70,7 +70,7 @@ const RELAY_MAX_WAIT_MS = 60 * 60_000 /** * How long a route or a height question may take through the relay or at an own Valhalla. - * The server gives the TREK Valhalla ten seconds; this leaves room for its 502 to arrive + * The server gives the TREK Valhalla ten seconds; this leaves room for its 504 to arrive * first. Where the relay itself hangs, FOSSGIS answers soon after; where an own Valhalla * does, OSRM routes the day, rather than the day waiting as long as the browser would. */ @@ -115,11 +115,11 @@ export type AvoidClass = RouteAvoidClass const MAX_LOCATIONS = 10 /** - * Pause between the pieces of a split day, in milliseconds. + * The pace of the public instance, in milliseconds between two questions. * - * The public instance allows about one request a second per client, and a day long - * enough to be split is exactly the case that would fire two back to back. Matches the - * 1100 ms the road trip rail already leaves between its own days. + * It allows about one request a second per client, and a day long enough to be split is + * exactly the case that would fire two back to back; `valhallaTurn` holds every question + * to it there. Matches the 1100 ms the road trip rail already leaves between its own days. */ const CHUNK_PAUSE_MS = 1100 @@ -307,9 +307,9 @@ export async function valhallaRouteAvoiding( * came back within two metres and under a second of the same day asked whole, because * the shared point is routed to and from identically. * - * The pieces go out one after another with a pause, since a day long enough to need - * splitting is exactly the one that would otherwise fire two requests inside a second - * at a host that allows one. + * The pieces go out one after another. Those that reach the public instance wait their + * turn there (`valhallaTurn`), since a day long enough to need splitting is exactly the + * one that would otherwise fire two requests inside a second at a host that allows one. */ export async function valhallaRun( waypoints: Waypoint[], @@ -372,12 +372,12 @@ interface RunAsk { } /** - * Locations one request carries: ten wherever it may end up at the public instance, - * twenty at the TREK Valhalla asked alone and at an own one. + * Locations one request carries: ten at the public instance, twenty at the TREK + * Valhalla and at an own one. A day cut for the relay that the relay then cannot + * answer is cut again for FOSSGIS (`chainRun`). */ -function locationsPerRequest(base: string, publicFallback: boolean): number { - const mayReachPublic = base === FOSSGIS_VALHALLA || (base === TREK_VALHALLA && publicFallback) - return mayReachPublic ? MAX_LOCATIONS : VALHALLA_RELAY_MAX_LOCATIONS +function locationsPerRequest(base: string): number { + return base === FOSSGIS_VALHALLA ? MAX_LOCATIONS : VALHALLA_RELAY_MAX_LOCATIONS } async function chainRun( @@ -389,18 +389,38 @@ async function chainRun( ): Promise { const base = valhallaBase() if (!mayAsk(base, ask.publicFallback) || waypoints.length < 2) return null + const size = locationsPerRequest(base) + const run = await chainRunSized(base, waypoints, size, profile, avoid, signal, ask) + // The relay fell over during a day cut for it. It rests now, so the base reads as FOSSGIS + // and the day is asked again there in pieces of ten. + const recut = run === null && base === TREK_VALHALLA && ask.publicFallback && waypoints.length > MAX_LOCATIONS + if (recut && Date.now() < relayRestingUntil && !signal?.aborted) { + return chainRun(waypoints, profile, avoid, signal, ask) + } + return run +} +async function chainRunSized( + base: string, + waypoints: Waypoint[], + size: number, + profile: 'driving' | 'walking' | 'cycling', + avoid: readonly AvoidClass[], + signal: AbortSignal | undefined, + ask: RunAsk, +): Promise { const legs: ValhallaLeg[] = [] - const chunks = chunkWaypoints(waypoints, locationsPerRequest(base, ask.publicFallback)) - for (const [index, chunk] of chunks.entries()) { + // No pause between the pieces: a question that reaches FOSSGIS waits its turn in + // valhallaTurn, and the relay and an own Valhalla are not paced. + for (const chunk of chunkWaypoints(waypoints, size)) { if (signal?.aborted) return null - if (index > 0) await pause(CHUNK_PAUSE_MS, signal) // Retried per piece rather than per day. The caller retries the whole task, so a // rate limit on the last piece of a split day would otherwise throw away the pieces // that already answered and ask for all of them again — three times over, against a // host that allows one request a second. const request = () => requestRun(base, chunk, profile, avoid, signal, ask) - const run = await askWithRetry(base, request, signal, ask.publicFallback) + // FOSSGIS stands in only for a piece of at most ten; a longer one is cut again instead. + const run = await askWithRetry(base, request, signal, ask.publicFallback && chunk.length <= MAX_LOCATIONS) // A day is one answer or none. Half a day of legs would be worse than no answer: // the schedule chains leg times blindly and cannot tell that it is missing some, so // it would print a complete, plausible, wrong timetable rather than a gap. @@ -488,6 +508,9 @@ export function resetValhallaTurns(): void { * relay refusing the body. */ async function relayCannotAnswer(response: Response): Promise { + // 504: this one question took too long. The relay still answers the next one, and + // FOSSGIS would take as long over this one. + if (response.status === 504) return false if (response.status === 400) { // A body that is not JSON cannot be Valhalla's own refusal, so it reads as the relay's. const data: unknown = await response.clone().json().catch(() => ({})) @@ -543,7 +566,7 @@ export async function valhallaPost( const relayed = await askRelay(action, body, signal) if (relayed) return relayed } - if (base === TREK_VALHALLA && !publicFallback) return new Response(null, { status: 503 }) + if (base === TREK_VALHALLA && (!publicFallback || overPublicLimit(action, body))) return new Response(null, { status: 503 }) const host = base === TREK_VALHALLA ? FOSSGIS_VALHALLA : base await valhallaTurn(host, signal) return fetch(`${host}/${action}`, { @@ -557,6 +580,12 @@ export async function valhallaPost( }) } +/** Whether a route carries more locations than the public instance takes, so it cannot stand in for the relay. */ +function overPublicLimit(action: ValhallaRelayAction, body: object): boolean { + const locations = (body as { locations?: unknown }).locations + return action === 'route' && Array.isArray(locations) && locations.length > MAX_LOCATIONS +} + /** * Waits out the host's one request a second before the next question is put. * @@ -575,7 +604,8 @@ export function valhallaSpacing(signal?: AbortSignal): Promise { * on a second ask, and asking anyway is the request the memo exists to save. Nor, where * nothing may stand in for this engine, when the relay now rests or the host used up its * whole deadline: the second ask would not leave this tab or would most likely wait as - * long again, and OSRM is waiting for the day. + * long again, and OSRM is waiting for the day. Nor when the relay ran out of time on + * it: the relay does not rest for that, so the second ask would wait at the same host. */ async function askWithRetry( base: string, @@ -587,6 +617,8 @@ async function askWithRetry( for (let attempt = 0; (!answer || tooSlow(answer)) && attempt < CHUNK_RETRIES && !signal?.aborted; attempt++) { if (notValhalla.has(base)) break if (!publicFallback && (tooSlow(answer) || (base === TREK_VALHALLA && Date.now() < relayRestingUntil))) break + // A question the relay ran out of time on would go to the same host again. + if (tooSlow(answer) && base === TREK_VALHALLA) break await pause(CHUNK_PAUSE_MS, signal) answer = await ask() } @@ -618,6 +650,8 @@ function routeBody( costing, costing_options: { [costing]: options }, directions_options: { units: 'kilometers' }, + // The maneuvers, where the ferry spans come from, without the narrative nothing reads. + directions_type: 'maneuvers', } } @@ -640,6 +674,8 @@ async function postRoute( // is the public instance's one-per-second. if (response.status === 404 || response.status === 405) notValhalla.add(base) if (response.status === 400 && (await refusesTheWay(response))) return NO_PATH + // The relay's word that this question ran out its deadline, read as one that timed out here. + if (response.status === 504) return TOO_SLOW return null } return await response.json() @@ -727,7 +763,7 @@ export async function valhallaTraceAttributes( } } -/** One request, at most ten locations. */ +/** One request, at most ten locations at the public instance and twenty elsewhere. */ async function requestRun( base: string, waypoints: Waypoint[], diff --git a/server/src/nest/roadtrip/roadtrip-router.service.ts b/server/src/nest/roadtrip/roadtrip-router.service.ts index dc747923b..d5d395e4b 100644 --- a/server/src/nest/roadtrip/roadtrip-router.service.ts +++ b/server/src/nest/roadtrip/roadtrip-router.service.ts @@ -173,17 +173,43 @@ async function refusalOf(response: Response): Promise<'no_way' | 'too_long' | nu /** The public Valhalla FOSSGIS runs, which TREK ships with. */ const FOSSGIS_VALHALLA = 'https://valhalla1.openstreetmap.de'; +/** The public OSRM hosts FOSSGIS runs, one per profile, which TREK ships with. */ +const FOSSGIS_OSRM = 'https://routing.openstreetmap.de'; /** Locations one request to FOSSGIS may carry; the eleventh is refused with error 150. */ const FOSSGIS_MAX_LOCATIONS = 10; +/** Whether a question goes to one of the public hosts, which allow about one request a second. */ +function isPublicHost(url: string): boolean { + return url.startsWith(`${FOSSGIS_VALHALLA}/`) || url.startsWith(`${FOSSGIS_OSRM}/`); +} + +/** + * A piece of a day cut for the relay, twenty locations, that has to go to + * FOSSGIS instead, which takes ten: the day is cut again for it (`valhallaLegs`). + */ +class TooManyForFossgis extends Error { + constructor() { + super('The route carries more locations than FOSSGIS takes'); + this.name = 'TooManyForFossgis'; + } +} + +/** The locations a route body carries, none for a question of another kind. */ +function locationCount(body: object): number { + const locations = (body as { locations?: unknown }).locations; + return Array.isArray(locations) ? locations.length : 0; +} + /** * What the relay says when it cannot answer for the TREK services right now: * no key, an allowance spent, a body it does not take, a path the host does * not know, a host that is down or resting. The question then goes to - * FOSSGIS, as the planner in the browser sends it. + * FOSSGIS, as the planner in the browser sends it. A 504 is not among them: + * that one question took too long, FOSSGIS would take as long over it, and + * the relay answers the next one as before. */ function relayCannotAnswer(status: number): boolean { - return [401, 403, 404, 405, 413, 429].includes(status) || status >= 500; + return [401, 403, 404, 405, 413, 429].includes(status) || (status >= 500 && status !== 504); } /** The relay's answer read as the queue reads a router's: the payload, a refusal, or a failure. */ @@ -342,7 +368,13 @@ export class RoadtripRouterService { return ['driving', 'walking', 'cycling', ...plugin]; } + /** + * One question to a router. The public hosts are asked one after another + * through a queue that leaves 1.1 s between them, as they ask; an admin's + * own OSRM or Valhalla sets its own limits and is asked at once. + */ private async request(url: string, body?: unknown, timeoutMs = 20000): Promise { + if (!isPublicHost(url)) return this.fetchPayload(url, body, timeoutMs); const before = this.queue; let release!: () => void; this.queue = new Promise((resolve) => { @@ -353,34 +385,39 @@ export class RoadtripRouterService { const delay = this.nextRequest - Date.now(); if (delay > 0) await new Promise((resolve) => setTimeout(resolve, delay)); this.nextRequest = Date.now() + 1100; - const response = await safeFetchAdminConfigured(url, { - signal: AbortSignal.timeout(timeoutMs), - headers: { 'User-Agent': 'TREK Roadtrip', 'Content-Type': 'application/json', 'X-Client-Id': 'trek' }, - ...(body ? { method: 'POST', body: JSON.stringify(body) } : {}), - }); - if (!response.ok) { - // A 400 from Valhalla says why: a way that does not exist is no failure of the host. - if (response.status !== 400) discardBody(response); - else { - const why = await refusalOf(response); - if (why) throw new RouteRefusal(why, 'Routing provider returned 400'); - } - throw new Error(`Routing provider returned ${response.status}`); - } - const payload = await readCappedJson(response, 8 * 1024 * 1024); - if (payload === undefined) throw new Error('Routing response is invalid or too large'); - return payload; + return await this.fetchPayload(url, body, timeoutMs); } finally { release(); } } + private async fetchPayload(url: string, body: unknown, timeoutMs: number): Promise { + const response = await safeFetchAdminConfigured(url, { + signal: AbortSignal.timeout(timeoutMs), + headers: { 'User-Agent': 'TREK Roadtrip', 'Content-Type': 'application/json', 'X-Client-Id': 'trek' }, + ...(body ? { method: 'POST', body: JSON.stringify(body) } : {}), + }); + if (!response.ok) { + // A 400 from Valhalla says why: a way that does not exist is no failure of the host. + if (response.status !== 400) discardBody(response); + else { + const why = await refusalOf(response); + if (why) throw new RouteRefusal(why, 'Routing provider returned 400'); + } + throw new Error(`Routing provider returned ${response.status}`); + } + const payload = await readCappedJson(response, 8 * 1024 * 1024); + if (payload === undefined) throw new Error('Routing response is invalid or too large'); + return payload; + } + /** * One question to a Valhalla. The TREK services' one is asked through the * relay, under the user's allowance there and with the installation key; a * relay that cannot answer hands the question to FOSSGIS through the paced - * queue, unless `publicFallback` is off. Every other Valhalla is asked - * through the queue directly. + * queue, unless `publicFallback` is off. A question the relay ran out of + * time on fails as it is, without FOSSGIS. Every other Valhalla is asked + * through `request` directly. */ private async askValhalla( userId: number, @@ -391,22 +428,24 @@ export class RoadtripRouterService { ): Promise { if (!this.relay.serves(valhalla)) return this.request(`${valhalla}/${action}`, body, timeoutMs); const answer = await this.relay.ask(userId, action, body, timeoutMs).catch((err: unknown) => { + if (err instanceof DomainError && err.getStatus() === 504) throw new Error(err.message); if (err instanceof DomainError) return null; throw err; }); if (answer && !relayCannotAnswer(answer.status)) return payloadOf(answer); if (!publicFallback) throw new Error('The TREK routing service cannot answer'); + if (locationCount(body) > FOSSGIS_MAX_LOCATIONS) throw new TooManyForFossgis(); return this.request(`${FOSSGIS_VALHALLA}/${action}`, body, timeoutMs); } /** - * Locations one route request carries: ten wherever it may end up at - * FOSSGIS, twenty at the TREK Valhalla asked alone and at an own one, as the - * planner splits a day (`locationsPerRequest`). + * Locations one route request carries: ten at FOSSGIS, twenty at the TREK + * Valhalla and at an own one, as the planner splits a day + * (`locationsPerRequest`). A piece cut for the relay that ends up at FOSSGIS + * has the day cut again (`valhallaLegs`). */ - private locationsPerRequest(valhalla: string, publicFallback: boolean): number { - const mayReachPublic = valhalla === FOSSGIS_VALHALLA || (this.relay.serves(valhalla) && publicFallback); - return mayReachPublic ? FOSSGIS_MAX_LOCATIONS : VALHALLA_RELAY_MAX_LOCATIONS; + private locationsPerRequest(valhalla: string): number { + return valhalla === FOSSGIS_VALHALLA ? FOSSGIS_MAX_LOCATIONS : VALHALLA_RELAY_MAX_LOCATIONS; } async route( @@ -612,9 +651,11 @@ export class RoadtripRouterService { /** * Valhalla's legs through the points, as many locations a request as the - * host takes (`locationsPerRequest`), one after another through the paced - * queue, each piece starting on the point the one before ended at. CW-12: a - * walk on foot binds no point to a ferry's edge (`offBoatLines`). + * host takes (`locationsPerRequest`), one after another, each piece starting + * on the point the one before ended at. Where the relay cannot answer a + * piece of twenty and FOSSGIS has to, the whole day is asked again in + * pieces of ten. CW-12: a walk on foot binds no point to a ferry's edge + * (`offBoatLines`). */ private async valhallaLegs( userId: number, @@ -622,10 +663,27 @@ export class RoadtripRouterService { points: Point[], costing: ValhallaCosting, options: Record, - { offBoatLines = false, publicFallback = true }: { offBoatLines?: boolean; publicFallback?: boolean } = {}, + ask: { offBoatLines?: boolean; publicFallback?: boolean } = {}, + ): Promise { + const size = this.locationsPerRequest(valhalla); + try { + return await this.legsInPieces(userId, valhalla, points, costing, options, ask, size); + } catch (err) { + if (!(err instanceof TooManyForFossgis)) throw err; + return this.legsInPieces(userId, valhalla, points, costing, options, ask, FOSSGIS_MAX_LOCATIONS); + } + } + + private async legsInPieces( + userId: number, + valhalla: string, + points: Point[], + costing: ValhallaCosting, + options: Record, + { offBoatLines = false, publicFallback = true }: { offBoatLines?: boolean; publicFallback?: boolean }, + size: number, ): Promise { const legs: ValhallaLegs = []; - const size = this.locationsPerRequest(valhalla, publicFallback); for (let start = 0; start < points.length - 1; start += size - 1) { const body = { locations: points.slice(start, start + size).map((p) => ({ @@ -637,6 +695,8 @@ export class RoadtripRouterService { costing, costing_options: { [costing]: options }, directions_options: { units: 'kilometers' }, + // The maneuvers, where a walk boards a ferry, without the narrative TREK never reads. + directions_type: 'maneuvers', }; const payload = await this.askValhalla(userId, valhalla, 'route', body, { publicFallback }); legs.push(...valhallaSchema.parse(payload).trip.legs); @@ -767,6 +827,7 @@ export class RoadtripRouterService { costing: 'pedestrian', costing_options: { pedestrian: { ...costingOptions } }, directions_options: { units: 'kilometers' }, + directions_type: 'maneuvers', ...(count > 0 ? { alternates: count } : {}), }); const parsed = alternatesSchema.parse(payload); diff --git a/server/src/nest/routing-relay/routing-relay.service.ts b/server/src/nest/routing-relay/routing-relay.service.ts index 4ffb7a6b0..094a546e9 100644 --- a/server/src/nest/routing-relay/routing-relay.service.ts +++ b/server/src/nest/routing-relay/routing-relay.service.ts @@ -23,11 +23,16 @@ const QUESTIONS_PER_MINUTE = 60; */ const INSTALLATION_QUESTIONS_PER_MINUTE = 100; const RATE_WINDOW_MS = 60_000; -/** Short, because FOSSGIS still has to answer behind it while the planner waits. */ +/** + * Short, because FOSSGIS still has to answer behind it while the planner waits. A + * question that runs past it is answered 504 and leaves the host asked: a bike route + * of 300 km takes the TREK Valhalla about twelve seconds, and that says nothing about + * the next question. + */ const TIMEOUT_MS = 10_000; /** Matching a long line takes Valhalla a while, on FOSSGIS as much as here. */ const TRACE_TIMEOUT_MS = 30_000; -/** How long every question goes to FOSSGIS after the TREK Valhalla failed one. */ +/** How long every question goes to FOSSGIS after the TREK Valhalla could not be reached or failed one. */ const REST_MS = 60_000; /** The longest wait a Retry-After is followed for; the first question after it finds out whether the allowance is back. */ const MAX_REST_MS = 60 * 60_000; @@ -53,6 +58,8 @@ export interface ValhallaRelayAnswer { * A host that failed a question, or refused it for a spent allowance, is left * alone for a while: the relay then answers 503 at once and every caller asks * FOSSGIS, instead of each leg of a long day waiting out the deadline again. + * A question that only took too long is no such failure: it is answered 504, + * and the next one is asked as before. */ @Injectable() export class RoutingRelayService { @@ -130,11 +137,14 @@ export class RoutingRelayService { 0, undefined, MAX_ANSWER_BYTES, + // One host asked many times in a row: its connection is kept rather than opened per question. + true, ); this.identity.observe(url, response); const retryAfter = response.headers.get('retry-after'); if (response.status === 429) this.rest(waitOf(retryAfter) ?? REST_MS); - else if (response.status >= 500) this.rest(REST_MS); + // A 504 of the host's own proxy is a slow question as much as the relay's deadline is. + else if (response.status >= 500 && response.status !== 504) this.rest(REST_MS); const { bytes, truncated } = await readCapped(response, MAX_ANSWER_BYTES); if (truncated) throw new Error('the answer is larger than the relay passes on'); return { @@ -144,9 +154,14 @@ export class RoutingRelayService { ...(retryAfter ? { retryAfter } : {}), }; } catch (err) { - this.rest(REST_MS); + // A question that ran out its deadline is a slow question, not a host that is down: + // only a host that could not be reached, or sent more than is passed on, rests. + const timedOut = err instanceof DOMException && err.name === 'TimeoutError'; + if (!timedOut) this.rest(REST_MS); logWarn(`[Routing relay] ${url.pathname}: ${err instanceof Error ? err.message : String(err)}`); - throw new DomainError(502, 'The TREK routing service did not answer'); + throw timedOut + ? new DomainError(504, 'The TREK routing service took too long for this route') + : new DomainError(502, 'The TREK routing service did not answer'); } } diff --git a/server/src/utils/ssrfGuard.ts b/server/src/utils/ssrfGuard.ts index 20a5204ca..a90daf896 100644 --- a/server/src/utils/ssrfGuard.ts +++ b/server/src/utils/ssrfGuard.ts @@ -261,6 +261,10 @@ function isLinkLocal(ip: string): boolean { * legitimately wait longer (see safeFetchLlm). Left unset, the caller's own * signal governs, and a call without one waits DEFAULT_RESPONSE_TIMEOUT_MS for * the response headers. `maxBytes` caps the body as in SafeFetchOptions. + * + * `reuseConnections` keeps the dispatcher for the next call to the same vetted + * addresses (see keptDispatcher), so a caller that asks one host many times in + * a row, the routing relay, does not pay DNS, TCP and TLS on every question. */ export async function safeFetchAdminConfigured( url: string, @@ -268,6 +272,7 @@ export async function safeFetchAdminConfigured( maxRedirects = 5, responseTimeoutMs?: number, maxBytes: number | null = DEFAULT_MAX_RESPONSE_BYTES, + reuseConnections = false, ): Promise { let currentUrl = url; let hopInit = init; @@ -298,7 +303,14 @@ export async function safeFetchAdminConfigured( throw new SsrfBlockedError('Requests to link-local / cloud-metadata addresses are not allowed'); } - const dispatcher = createOutboundDispatcher(currentUrl, usableIps, true, responseTimeoutMs, headersTimeoutMs); + const dispatcher = createOutboundDispatcher( + currentUrl, + usableIps, + true, + responseTimeoutMs, + headersTimeoutMs, + reuseConnections, + ); const response = await fetch(currentUrl, { ...hopInit, redirect: 'manual', dispatcher } as any); // Only a 3xx WITH a Location header is a redirect we follow; anything else @@ -683,14 +695,57 @@ export function createOutboundDispatcher( rejectUnauthorized = true, responseTimeoutMs?: number, headersTimeoutMs?: number, + reuseConnections = false, ): Dispatcher { const proxy = proxyFor(url); - if (!proxy) return createPinnedDispatcher(resolved, rejectUnauthorized, responseTimeoutMs, headersTimeoutMs); - return new ProxyAgent({ - uri: proxy, - requestTls: { rejectUnauthorized }, - ...dispatcherTimeouts(responseTimeoutMs, headersTimeoutMs), - }); + const keepAliveMs = reuseConnections ? KEPT_ALIVE_MS : undefined; + const create = (): Dispatcher => + proxy + ? new ProxyAgent({ + uri: proxy, + requestTls: { rejectUnauthorized }, + ...dispatcherTimeouts(responseTimeoutMs, headersTimeoutMs), + ...(keepAliveMs ? { keepAliveTimeout: keepAliveMs } : {}), + }) + : createPinnedDispatcher(resolved, rejectUnauthorized, responseTimeoutMs, headersTimeoutMs, keepAliveMs); + if (!reuseConnections) return create(); + // Through a proxy the proxy connects, so the addresses play no part in what the dispatcher does. + const addresses = proxy ? [] : typeof resolved === 'string' ? [resolved] : [...resolved]; + const key = JSON.stringify([ + proxy, + addresses, + rejectUnauthorized, + responseTimeoutMs ?? null, + headersTimeoutMs ?? null, + ]); + return keptDispatcher(key, create); +} + +/** + * Dispatchers kept for the callers that reuse their connections, by everything + * that makes one: the proxy, the vetted addresses, the TLS check and the + * timeouts. The addresses are part of the key, so the pin is as strict as a + * new agent's: a name that now resolves elsewhere gets a dispatcher of its own + * and never a connection to the old address. At most MAX_KEPT_DISPATCHERS are + * kept; the oldest is closed, after its requests finish, when one more is needed. + */ +const keptDispatchers = new Map(); +const MAX_KEPT_DISPATCHERS = 32; +/** How long a kept connection may sit idle, where undici's own default is four seconds. */ +const KEPT_ALIVE_MS = 10_000; + +function keptDispatcher(key: string, create: () => Dispatcher): Dispatcher { + const kept = keptDispatchers.get(key); + if (kept) return kept; + const dispatcher = create(); + keptDispatchers.set(key, dispatcher); + const [oldestKey] = keptDispatchers.keys(); + if (keptDispatchers.size > MAX_KEPT_DISPATCHERS && oldestKey !== undefined) { + const oldest = keptDispatchers.get(oldestKey); + keptDispatchers.delete(oldestKey); + void oldest?.close().catch(() => {}); + } + return dispatcher; } /** @@ -717,12 +772,15 @@ function dispatcherTimeouts( * `all: true` and walks the list itself, moving on after its attempt timeout * when one family does not answer. * A caller that still names one address gets exactly the old behaviour. + * `keepAliveMs` holds an idle connection open longer than undici's default, + * for a dispatcher that is kept (keptDispatcher). */ export function createPinnedDispatcher( resolved: string | readonly string[], rejectUnauthorized = true, responseTimeoutMs?: number, headersTimeoutMs?: number, + keepAliveMs?: number, ): Agent { const addresses = (typeof resolved === 'string' ? [resolved] : [...resolved]).map((address) => ({ address, @@ -733,6 +791,7 @@ export function createPinnedDispatcher( // that cap is invisible from the call site: an AbortController set to // fifteen still dies at five. ...dispatcherTimeouts(responseTimeoutMs, headersTimeoutMs), + ...(keepAliveMs ? { keepAliveTimeout: keepAliveMs } : {}), connect: { rejectUnauthorized, lookup: (_hostname: string, opts: Record, callback: Function) => { diff --git a/server/tests/unit/nest/roadtrip-router.test.ts b/server/tests/unit/nest/roadtrip-router.test.ts index 4eb07459f..a2fdffc24 100644 --- a/server/tests/unit/nest/roadtrip-router.test.ts +++ b/server/tests/unit/nest/roadtrip-router.test.ts @@ -747,6 +747,22 @@ describe('server Roadtrip router through the TREK services (routing relay)', () }); }); + it('ROUTER-RELAY-007: a question the relay ran out of time on is not handed to FOSSGIS, and OSRM routes the day', async () => { + const slow = [ + () => Promise.reject(new DomainError(504, 'The TREK routing service took too long for this route')), + () => Promise.resolve(answer(504, { error: 'gateway timeout' })), + ]; + await oneByOne([...slow.entries()], async ([userId, timedOut]) => { + vi.mocked(safeFetchAdminConfigured).mockClear(); + relay.ask.mockReset().mockImplementationOnce(timedOut); + const route = await settled(setup().router.route(userId, 1, 1, points.slice(0, 2), 'driving', ['toll'])); + expect(route.avoidMissed).toEqual(['toll']); + const asked = vi.mocked(safeFetchAdminConfigured).mock.calls.map(([url]) => String(url)); + expect(asked).toHaveLength(1); + expect(asked[0]).toContain('routing.openstreetmap.de/routed-car'); + }); + }); + it('ROUTER-RELAY-003: reads the TREK Valhalla saying no way joins the points as a refusal, and fails on any other error', async () => { const costingOptions = { max_hiking_difficulty: 3 }; relay.ask.mockResolvedValueOnce(answer(400, { error_code: 442, error: 'No path could be found for input' })); @@ -908,13 +924,22 @@ describe('server Roadtrip router: every day through Valhalla where it routes the ); }); - it('ROUTER-DAY-005: a day that avoids something keeps FOSSGIS behind the relay and its ten locations', async () => { + it('ROUTER-DAY-011: a day that avoids something asks the relay twenty locations at a time, with only the maneuvers', async () => { + const route = await settled(setup().router.route(4, 1, 1, day(12), 'driving', ['toll'])); + expect(relayBodies().map((body) => body.locations.length)).toEqual([12]); + expect(relay.ask.mock.calls[0][2]).toMatchObject({ directions_type: 'maneuvers' }); + expect(route.parts).toHaveLength(11); + expect(safeFetchAdminConfigured).not.toHaveBeenCalled(); + }); + + it('ROUTER-DAY-005: a day that avoids something keeps FOSSGIS behind the relay, cut again in its ten locations', async () => { relay.ask.mockReset().mockResolvedValue(answer(503, { error: 'The TREK routing service is resting' })); vi.mocked(safeFetchAdminConfigured).mockImplementation( async (_url, init) => new Response(JSON.stringify(legsFor(bodyOf(init)))), ); const route = await settled(setup().router.route(4, 1, 1, day(12), 'driving', ['toll'])); - expect(relayBodies().map((body) => body.locations.length)).toEqual([10, 3]); + // Twelve for the relay, then the day cut again: FOSSGIS refuses an eleventh location. + expect(relayBodies().map((body) => body.locations.length)).toEqual([12, 10, 3]); expect(vi.mocked(safeFetchAdminConfigured).mock.calls.map(([url]) => url)).toEqual( Array(2).fill('https://valhalla1.openstreetmap.de/route'), ); @@ -986,6 +1011,42 @@ describe('server Roadtrip router: every day through Valhalla where it routes the expect(relay.ask).not.toHaveBeenCalled(); }); + it('ROUTER-DAY-012: paces the public hosts a request every 1.1 s, an own Valhalla and an own OSRM not at all', async () => { + const own = (url: unknown) => String(url).startsWith('https://valhalla.internal'); + vi.mocked(safeFetchAdminConfigured).mockImplementation(async (url, init) => + own(url) ? new Response(JSON.stringify(legsFor(bodyOf(init)))) : osrm(), + ); + const ownValhalla = setup({ valhalla_base_url: 'https://valhalla.internal' }).router.route( + 1, + 1, + 1, + day(21), + 'cycling', + [], + ); + const ownOsrm = setup({ routing_base_url: 'http://router.internal' }).router; + const twoDays = Promise.all([ + ownOsrm.route(2, 1, 1, points, 'driving', []), + ownOsrm.route(2, 1, 2, points, 'driving', []), + ]); + await vi.advanceTimersByTimeAsync(1000); + expect(await ownValhalla).toMatchObject({ parts: expect.any(Array) }); + expect(await twoDays).toHaveLength(2); + expect(safeFetchAdminConfigured).toHaveBeenCalledTimes(4); + + vi.mocked(safeFetchAdminConfigured).mockClear(); + const { router } = setup(); + const publicDays = Promise.all([ + router.route(3, 1, 1, points, 'driving', []), + router.route(3, 1, 2, points, 'driving', []), + ]); + await vi.advanceTimersByTimeAsync(1000); + expect(safeFetchAdminConfigured).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(200); + await publicDays; + expect(safeFetchAdminConfigured).toHaveBeenCalledTimes(2); + }); + it('ROUTER-DAY-007: with FOSSGIS chosen, a day that avoids nothing stays with OSRM as before', async () => { relay.trekValhalla.mockReturnValue(TREK_VALHALLA); const { router } = setup({ routing_valhalla_provider: 'fossgis' }); diff --git a/server/tests/unit/nest/routing-relay/routing-relay.service.test.ts b/server/tests/unit/nest/routing-relay/routing-relay.service.test.ts index c9630b966..93a726548 100644 --- a/server/tests/unit/nest/routing-relay/routing-relay.service.test.ts +++ b/server/tests/unit/nest/routing-relay/routing-relay.service.test.ts @@ -81,9 +81,10 @@ describe('RoutingRelayService.ask', () => { contentType: 'application/json', body: Buffer.from('{"trip":{"legs":[]}}'), }); - const [url, init, redirects] = vi.mocked(safeFetchAdminConfigured).mock.calls[0]; + const [url, init, redirects, , , reuseConnections] = vi.mocked(safeFetchAdminConfigured).mock.calls[0]; expect(url).toBe('https://valhalla.liketrek.com/route'); expect(redirects).toBe(0); + expect(reuseConnections).toBe(true); expect(init?.signal).toBeInstanceOf(AbortSignal); expect((init?.headers as Record).Authorization).toBe('Bearer tk1.k.p.s'); expect(JSON.parse(String(init?.body))).toEqual(ROUTE); @@ -158,12 +159,10 @@ describe('RoutingRelayService.ask', () => { await expect(relay.ask(1, 'route', ROUTE)).rejects.toMatchObject({ status: 502 }); }); - it('RELAY-009: after a host that hung, the next question is answered 503 at once, and asked again after the rest', async () => { + it('RELAY-009: after a host that could not be reached, the next question is answered 503 at once, and asked again after the rest', async () => { vi.useFakeTimers({ toFake: ['Date'] }); vi.setSystemTime(new Date('2026-10-29T10:00:00Z')); - vi.mocked(safeFetchAdminConfigured).mockRejectedValueOnce( - new DOMException('The operation was aborted due to timeout', 'TimeoutError'), - ); + vi.mocked(safeFetchAdminConfigured).mockRejectedValueOnce(new TypeError('fetch failed')); await expect(relay.ask(1, 'height', { shape: [{ lat: 47, lon: 11 }] })).rejects.toMatchObject({ status: 502 }); const resting = await relay.ask(1, 'height', { shape: [{ lat: 47.1, lon: 11 }] }); @@ -177,12 +176,27 @@ describe('RoutingRelayService.ask', () => { }); it('RELAY-010: passes an error of the host on and rests after it', async () => { - vi.mocked(safeFetchAdminConfigured).mockResolvedValueOnce(upstream(504, '{"error":"gateway"}')); - expect((await relay.ask(1, 'route', ROUTE)).status).toBe(504); + vi.mocked(safeFetchAdminConfigured).mockResolvedValueOnce(upstream(502, '{"error":"gateway"}')); + expect((await relay.ask(1, 'route', ROUTE)).status).toBe(502); expect((await relay.ask(1, 'route', ROUTE)).status).toBe(503); expect(safeFetchAdminConfigured).toHaveBeenCalledTimes(1); }); + it('RELAY-016: answers 504 for a question that ran out its deadline, and asks the host the next one', async () => { + vi.mocked(safeFetchAdminConfigured).mockRejectedValueOnce( + new DOMException('The operation was aborted due to timeout', 'TimeoutError'), + ); + await expect(relay.ask(1, 'route', ROUTE)).rejects.toMatchObject({ + status: 504, + message: 'The TREK routing service took too long for this route', + }); + vi.mocked(safeFetchAdminConfigured).mockResolvedValueOnce(upstream(504, '{"error":"gateway timeout"}')); + expect((await relay.ask(1, 'route', ROUTE)).status).toBe(504); + vi.mocked(safeFetchAdminConfigured).mockResolvedValueOnce(upstream(200, '{}')); + expect((await relay.ask(1, 'route', ROUTE)).status).toBe(200); + expect(safeFetchAdminConfigured).toHaveBeenCalledTimes(3); + }); + it("RELAY-011: follows a spent allowance's Retry-After, at most an hour of it, and passes it on", async () => { vi.useFakeTimers({ toFake: ['Date'] }); vi.setSystemTime(new Date('2026-10-29T10:00:00Z')); diff --git a/server/tests/unit/utils/ssrf-proxy.test.ts b/server/tests/unit/utils/ssrf-proxy.test.ts index 86554d5b5..67ce637b6 100644 --- a/server/tests/unit/utils/ssrf-proxy.test.ts +++ b/server/tests/unit/utils/ssrf-proxy.test.ts @@ -5,13 +5,21 @@ */ import { afterEach, describe, expect, it, vi } from 'vitest'; -const built = vi.hoisted(() => ({ agents: [] as unknown[], proxies: [] as Array> })); +const built = vi.hoisted(() => ({ + agents: [] as unknown[], + proxies: [] as Array>, + closed: [] as unknown[], +})); vi.mock('undici', () => ({ Agent: class { - constructor(opts: unknown) { + constructor(readonly opts: unknown) { built.agents.push(opts); } + close() { + built.closed.push(this.opts); + return Promise.resolve(); + } }, ProxyAgent: class { constructor(opts: Record) { @@ -33,6 +41,7 @@ afterEach(() => { for (const key of PROXY_KEYS) delete process.env[key]; built.agents.length = 0; built.proxies.length = 0; + built.closed.length = 0; }); describe('proxyFor', () => { @@ -87,3 +96,81 @@ describe('createOutboundDispatcher', () => { }); }); }); + +describe('createOutboundDispatcher, connections kept (the routing relay)', () => { + it('SSRF-KEEP-001: hands the same dispatcher back for the same vetted addresses, held open ten seconds', async () => { + const { createOutboundDispatcher } = await loadGuard({}); + const first = createOutboundDispatcher( + 'https://valhalla.liketrek.com/route', + ['203.0.113.7'], + true, + undefined, + undefined, + true, + ); + const again = createOutboundDispatcher( + 'https://valhalla.liketrek.com/height', + ['203.0.113.7'], + true, + undefined, + undefined, + true, + ); + expect(again).toBe(first); + expect(built.agents).toHaveLength(1); + expect(built.agents[0]).toMatchObject({ keepAliveTimeout: 10_000 }); + }); + + it('SSRF-KEEP-002: a name that resolves elsewhere, or other timeouts, get a dispatcher of their own', async () => { + const { createOutboundDispatcher } = await loadGuard({}); + const first = createOutboundDispatcher( + 'https://valhalla.liketrek.com/', + ['203.0.113.7'], + true, + undefined, + undefined, + true, + ); + const moved = createOutboundDispatcher( + 'https://valhalla.liketrek.com/', + ['203.0.113.8'], + true, + undefined, + undefined, + true, + ); + const slower = createOutboundDispatcher( + 'https://valhalla.liketrek.com/', + ['203.0.113.7'], + true, + 60_000, + undefined, + true, + ); + expect(new Set([first, moved, slower]).size).toBe(3); + // The new address is the one the socket gets, never the old one. + const lookup = ( + built.agents[1] as { connect: { lookup: (h: string, o: object, cb: (...a: unknown[]) => void) => void } } + ).connect.lookup; + const seen: unknown[] = []; + lookup('valhalla.liketrek.com', {}, (...args: unknown[]) => seen.push(...args)); + expect(seen).toContain('203.0.113.8'); + expect(seen).not.toContain('203.0.113.7'); + }); + + it('SSRF-KEEP-003: keeps at most 32 and closes the oldest; a caller that does not ask keeps none', async () => { + const { createOutboundDispatcher } = await loadGuard({}); + const at = (n: number) => + createOutboundDispatcher('https://h.example/', [`203.0.113.${n}`], true, undefined, undefined, true); + const oldest = at(0); + for (let n = 1; n <= 32; n++) at(n); + expect(built.closed).toEqual([built.agents[0]]); + expect(at(0)).not.toBe(oldest); + + built.agents.length = 0; + createOutboundDispatcher('https://h.example/', ['203.0.113.1']); + createOutboundDispatcher('https://h.example/', ['203.0.113.1']); + expect(built.agents).toHaveLength(2); + expect(built.agents[0]).not.toHaveProperty('keepAliveTimeout'); + }); +}); diff --git a/shared/src/routing-relay/routing-relay.schema.spec.ts b/shared/src/routing-relay/routing-relay.schema.spec.ts index 3d4d65a8b..bc4fe4a49 100644 --- a/shared/src/routing-relay/routing-relay.schema.spec.ts +++ b/shared/src/routing-relay/routing-relay.schema.spec.ts @@ -30,6 +30,9 @@ describe('valhallaRouteRequestSchema', () => { it('takes the route the planner asks for, its costing options included', () => { expect(valhallaRouteRequestSchema.parse(route)).toEqual(route); expect(valhallaRouteRequestSchema.parse({ ...route, alternates: 3 }).alternates).toBe(3); + expect(valhallaRouteRequestSchema.parse({ ...route, directions_type: 'maneuvers' }).directions_type).toBe( + 'maneuvers', + ); const bike = { ...route, costing: 'bicycle', @@ -45,6 +48,8 @@ describe('valhallaRouteRequestSchema', () => { const many = Array.from({ length: VALHALLA_RELAY_MAX_LOCATIONS + 1 }, () => route.locations[0]); expect(valhallaRouteRequestSchema.safeParse({ ...route, locations: many }).success).toBe(false); expect(valhallaRouteRequestSchema.safeParse({ ...route, costing: 'truck' }).success).toBe(false); + // Only the maneuvers: `none` would drop the ferry boarding the planner reads off them. + expect(valhallaRouteRequestSchema.safeParse({ ...route, directions_type: 'none' }).success).toBe(false); const off = [{ lat: 91, lon: 0 }, route.locations[1]]; expect(valhallaRouteRequestSchema.safeParse({ ...route, locations: off }).success).toBe(false); }); diff --git a/shared/src/routing-relay/routing-relay.schema.ts b/shared/src/routing-relay/routing-relay.schema.ts index 0f05839a7..317bb6737 100644 --- a/shared/src/routing-relay/routing-relay.schema.ts +++ b/shared/src/routing-relay/routing-relay.schema.ts @@ -66,6 +66,8 @@ export const valhallaRouteRequestSchema = z costing: costingSchema, costing_options: costingOptionsSchema.optional(), directions_options: z.object({ units: z.enum(['kilometers', 'miles']).optional() }).optional(), + // The maneuvers without their narrative: TREK reads where a route boards a ferry, never the instructions. + directions_type: z.enum(['maneuvers']).optional(), alternates: z.number().int().min(0).max(VALHALLA_RELAY_MAX_ALTERNATES).optional(), }) .refine(withinRelayBytes.check, withinRelayBytes.message);