From a74a2cf9b5abc14e171b7381ef0abe678b12553e Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 18 Oct 2026 12:42:50 +0200 Subject: [PATCH] fix(map): the day route waits out a busy router instead of keeping straight lines The public OSRM hosts answer 429 above about one request a second. The trip overview and the road trip waited that out and asked again; the day route took the first refusal as final and drew straight lines between the stops until the next edit, which is what a day with a change of mode or a quick switch between days ran into. Both the day route and the overview now go through one retry helper: a rate limit is waited out, as long as the host asks when it says, and a refusal the host meant still ends at once. --- client/src/components/Map/routeRetry.test.ts | 85 +++++++++++++++++++ client/src/components/Map/routeRetry.ts | 48 +++++++++++ .../src/components/Map/tripRouteGeometry.ts | 44 +++------- client/src/hooks/useRouteCalculation.ts | 5 +- 4 files changed, 148 insertions(+), 34 deletions(-) create mode 100644 client/src/components/Map/routeRetry.test.ts create mode 100644 client/src/components/Map/routeRetry.ts diff --git a/client/src/components/Map/routeRetry.test.ts b/client/src/components/Map/routeRetry.test.ts new file mode 100644 index 000000000..fb7fc9454 --- /dev/null +++ b/client/src/components/Map/routeRetry.test.ts @@ -0,0 +1,85 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { calculateRouteWithLegs, RoutingRefusedError } from './RouteCalculator'; +import { calculateRouteWithRetries, ROUTE_RETRY_DELAYS_MS } from './routeRetry'; + +vi.mock('./RouteCalculator', async (importActual) => { + const actual = await importActual(); + return { ...actual, calculateRouteWithLegs: vi.fn() }; +}); + +const WAYPOINTS = [ + { lat: 47.5, lng: 19.04 }, + { lat: 47.49, lng: 19.05 }, +]; +const ROUTED = { + coordinates: [ + [47.5, 19.04], + [47.49, 19.05], + ], + legs: [], +} as unknown as Awaited>; + +beforeEach(() => { + vi.useFakeTimers(); + vi.mocked(calculateRouteWithLegs).mockReset(); +}); +afterEach(() => vi.useRealTimers()); + +describe('calculateRouteWithRetries', () => { + it('waits out a rate limit and answers with the road (Discord, 16.10.2026)', async () => { + vi.mocked(calculateRouteWithLegs) + .mockRejectedValueOnce(new RoutingRefusedError(429, null)) + .mockResolvedValueOnce(ROUTED); + + const answer = calculateRouteWithRetries(WAYPOINTS, { profile: 'driving' }); + await vi.advanceTimersByTimeAsync(ROUTE_RETRY_DELAYS_MS[0]); + + await expect(answer).resolves.toBe(ROUTED); + expect(calculateRouteWithLegs).toHaveBeenCalledTimes(2); + }); + + it('waits as long as the host asks when that is longer', async () => { + vi.mocked(calculateRouteWithLegs) + .mockRejectedValueOnce(new RoutingRefusedError(503, 3000)) + .mockResolvedValueOnce(ROUTED); + + const answer = calculateRouteWithRetries(WAYPOINTS, {}); + await vi.advanceTimersByTimeAsync(ROUTE_RETRY_DELAYS_MS[0]); + expect(calculateRouteWithLegs).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(3000 - ROUTE_RETRY_DELAYS_MS[0]); + + await expect(answer).resolves.toBe(ROUTED); + }); + + it('gives up after the last delay and throws the refusal', async () => { + vi.mocked(calculateRouteWithLegs).mockRejectedValue(new RoutingRefusedError(429, null)); + + const answer = calculateRouteWithRetries(WAYPOINTS, {}); + const settled = expect(answer).rejects.toBeInstanceOf(RoutingRefusedError); + for (const ms of ROUTE_RETRY_DELAYS_MS) await vi.advanceTimersByTimeAsync(ms); + + await settled; + expect(calculateRouteWithLegs).toHaveBeenCalledTimes(ROUTE_RETRY_DELAYS_MS.length + 1); + }); + + it('throws a refusal the host meant at once', async () => { + vi.mocked(calculateRouteWithLegs).mockRejectedValue(new RoutingRefusedError(400, null)); + + await expect(calculateRouteWithRetries(WAYPOINTS, {})).rejects.toBeInstanceOf(RoutingRefusedError); + expect(calculateRouteWithLegs).toHaveBeenCalledTimes(1); + }); + + it('stops waiting when the day changes', async () => { + const controller = new AbortController(); + vi.mocked(calculateRouteWithLegs) + .mockRejectedValueOnce(new RoutingRefusedError(429, null)) + .mockRejectedValueOnce(new DOMException('aborted', 'AbortError')); + + const answer = calculateRouteWithRetries(WAYPOINTS, { signal: controller.signal }); + const settled = expect(answer).rejects.toMatchObject({ name: 'AbortError' }); + await vi.advanceTimersByTimeAsync(10); + controller.abort(); + + await settled; + }); +}); diff --git a/client/src/components/Map/routeRetry.ts b/client/src/components/Map/routeRetry.ts new file mode 100644 index 000000000..982396c06 --- /dev/null +++ b/client/src/components/Map/routeRetry.ts @@ -0,0 +1,48 @@ +import { calculateRouteWithLegs, RoutingRefusedError } from './RouteCalculator'; + +// The profile a caller hands in, so a hook routing through here needs nothing else from Map/. +export type { RouteProfileKey } from './RouteCalculator'; + +/** How often a rate-limited route is asked for again, and how long after. */ +export const ROUTE_RETRY_DELAYS_MS = [1500, 4000]; + +/** A pause that ends early, without throwing, when the signal aborts. */ +export const sleepUnlessAborted = (ms: number, signal?: AbortSignal): Promise => + new Promise((resolve) => { + if (signal?.aborted || ms <= 0) { + resolve(); + return; + } + const timer = setTimeout(done, ms); + function done(): void { + clearTimeout(timer); + signal?.removeEventListener('abort', done); + resolve(); + } + signal?.addEventListener('abort', done, { once: true }); + }); + +/** + * `calculateRouteWithLegs`, asked again while the host says it is busy. + * + * The public OSRM hosts allow about one request a second and answer 429 above that. A + * day with a change of mode, or a quick switch between days, asks more often than that, + * and the day route used to give up on the first refusal and keep its straight lines + * until the next edit (reported on Discord, 16.10.2026). A rate limit is waited out, for + * as long as the host asks when it says; a refusal the host meant is thrown at once, + * because the same request earns the same answer. An abort ends the wait, and the next + * attempt then throws the AbortError the caller already handles. + */ +export async function calculateRouteWithRetries( + ...[waypoints, opts]: Parameters +): ReturnType { + for (let attempt = 0; ; attempt++) { + try { + return await calculateRouteWithLegs(waypoints, opts); + } catch (err) { + const rateLimit = err instanceof RoutingRefusedError && err.isRateLimit ? err : null; + if (!rateLimit || attempt >= ROUTE_RETRY_DELAYS_MS.length || opts?.signal?.aborted) throw err; + await sleepUnlessAborted(Math.max(ROUTE_RETRY_DELAYS_MS[attempt], rateLimit.retryAfterMs ?? 0), opts?.signal); + } + } +} diff --git a/client/src/components/Map/tripRouteGeometry.ts b/client/src/components/Map/tripRouteGeometry.ts index 36017c598..56e6ede60 100644 --- a/client/src/components/Map/tripRouteGeometry.ts +++ b/client/src/components/Map/tripRouteGeometry.ts @@ -1,4 +1,5 @@ -import { calculateRouteWithLegs, RoutingRefusedError, type RouteProfileKey } from './RouteCalculator' +import type { RouteProfileKey } from './RouteCalculator' +import { calculateRouteWithRetries, sleepUnlessAborted as sleep } from './routeRetry' import { buildDayRouteRuns, type DayRouteInputs, type DayRoutePoint } from './dayRoutePlan' import { dayViaLookup, foldViaLegs, threadVias, viasOnDayLeg, type DayViaLookup } from '../../utils/dayRouteVias' import { projectDayItinerary } from './dayTourProjection' @@ -185,20 +186,6 @@ export function summariseTripRoute(days: TripOverviewDay[]): TripRouteSummary { const REQUEST_SPACING_MS = 1100 /** Anything answered faster than this came out of RouteCalculator's cache, not the network. */ const CACHE_HIT_MS = 60 -/** How often a rate-limited leg is tried again, and how long after. */ -const RETRY_DELAYS_MS = [1500, 4000] - -const sleep = (ms: number, signal?: AbortSignal): Promise => - new Promise(resolve => { - if (signal?.aborted || ms <= 0) { resolve(); return } - const timer = setTimeout(done, ms) - function done(): void { - clearTimeout(timer) - signal?.removeEventListener('abort', done) - resolve() - } - signal?.addEventListener('abort', done, { once: true }) - }) /** * Ask the router for every leg of every day, one at a time. @@ -233,23 +220,16 @@ export async function routeTripLegs( runs.forEach((chunks, r) => { chunks.forEach((chunk, c) => { tasks.push(async () => { - for (let attempt = 0; attempt <= RETRY_DELAYS_MS.length; attempt++) { - if (signal?.aborted) return - try { - const answer = await calculateRouteWithLegs( - chunk.waypoints, - { signal, profile: chunk.mode, tripId, dayId: day.id }, - ) - routed[d][r][c] = { coordinates: answer.coordinates, legs: foldViaLegs(answer.legs, chunk.stopAt, unit) } - onAnswer?.(routed) - return - } catch (err) { - if (signal?.aborted) return - const rateLimit = err instanceof RoutingRefusedError && err.isRateLimit ? err : null - if (!rateLimit || attempt === RETRY_DELAYS_MS.length) return - // When the host says how long to wait, waiting less is just a second refusal. - await sleep(Math.max(RETRY_DELAYS_MS[attempt], rateLimit.retryAfterMs ?? 0), signal) - } + if (signal?.aborted) return + try { + const answer = await calculateRouteWithRetries( + chunk.waypoints, + { signal, profile: chunk.mode, tripId, dayId: day.id }, + ) + routed[d][r][c] = { coordinates: answer.coordinates, legs: foldViaLegs(answer.legs, chunk.stopAt, unit) } + onAnswer?.(routed) + } catch { + // Refused for good, or aborted: the leg keeps its straight line. } }) }) diff --git a/client/src/hooks/useRouteCalculation.ts b/client/src/hooks/useRouteCalculation.ts index 94d2979ad..77c9b90ea 100644 --- a/client/src/hooks/useRouteCalculation.ts +++ b/client/src/hooks/useRouteCalculation.ts @@ -4,7 +4,7 @@ import { useState, useCallback, useRef, useEffect, useMemo } from 'react' import { useTripStore } from '../store/tripStore' import { useSettingsStore } from '../store/settingsStore' import { useAddonStore } from '../store/addonStore' -import { calculateRouteWithLegs, type RouteProfileKey } from '../components/Map/RouteCalculator' +import { calculateRouteWithRetries, type RouteProfileKey } from '../components/Map/routeRetry' import { buildDayRouteRuns, hotelBookendOf, TRANSPORT_TYPES } from '../components/Map/dayRoutePlan' import { dayViaLookup, foldViaLegs, threadVias, viaKeyOf, viasOnDayLeg } from '../utils/dayRouteVias' import { resolveLegMode } from '../components/Planner/legMode' @@ -131,7 +131,8 @@ export function useRouteCalculation(tripStore: Pick chunk.map(p => [p.lat, p.lng] as [number, number]) const { waypoints, stopAt } = threadVias(chunk, (from, to) => viasOnDayLeg(viaLookup, dayId, mode, from, to)) try { - const r = await calculateRouteWithLegs(waypoints, { signal: controller.signal, profile: mode, tripId, dayId }) + // A busy router is waited out rather than left as straight lines until the next edit. + const r = await calculateRouteWithRetries(waypoints, { signal: controller.signal, profile: mode, tripId, dayId }) pushCoords(r.coordinates.length >= 2 ? r.coordinates : straight(), mode === 'walking') // Leg k runs from chunk[k] to chunk[k + 1]; a hotel bookend says so on the // segment, which is how the phone timeline finds it (#2501).