From e5ceba00f3de9c8c1949180a8b0ee28f5ce1854f Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 7 Oct 2026 21:39:48 +0200 Subject: [PATCH] fix(tours): wait for the public Valhalla's turn before every request The public instance allows about one request a second per client, and the Tours elevation questions went out right behind the route request with no pause, so they could be refused with a 429 and leave the tour without the elevation it needs to be saved. Every request to that host now waits for its turn, across callers; an operator's own Valhalla is not paced. --- .../src/components/Map/valhallaRoute.test.ts | 53 ++++++++++++++++++- client/src/components/Map/valhallaRoute.ts | 31 +++++++++++ .../Tours/planner/tourRouting.test.ts | 31 ++++++++++- .../components/Tours/planner/tourRouting.ts | 7 ++- 4 files changed, 118 insertions(+), 4 deletions(-) diff --git a/client/src/components/Map/valhallaRoute.test.ts b/client/src/components/Map/valhallaRoute.test.ts index 7e0198a9e..86c2b8f45 100644 --- a/client/src/components/Map/valhallaRoute.test.ts +++ b/client/src/components/Map/valhallaRoute.test.ts @@ -1,8 +1,8 @@ -import { describe, it, expect, afterEach } from 'vitest' +import { describe, it, expect, afterEach, vi } from 'vitest' import { http, HttpResponse } from 'msw' import { server } from '../../../tests/helpers/msw/server' import { useSettingsStore } from '../../store/settingsStore' -import { valhallaBase, valhallaAvailable, valhallaRouteAvoiding, valhallaRun, valhallaAlternates, alternatesFrom, runFrom, legAvoids } from './valhallaRoute' +import { valhallaBase, valhallaAvailable, valhallaTurn, resetValhallaTurns, valhallaRouteAvoiding, valhallaRun, valhallaAlternates, alternatesFrom, runFrom, legAvoids } from './valhallaRoute' const FOSSGIS_VALHALLA = 'https://valhalla1.openstreetmap.de/route' @@ -37,6 +37,55 @@ const setSettings = (patch: Record) => afterEach(() => { setSettings({ routing_base_url: '', valhalla_base_url: '' }) + resetValhallaTurns() +}) + +describe('valhallaTurn', () => { + const PUBLIC = 'https://valhalla1.openstreetmap.de' + + afterEach(() => vi.useRealTimers()) + + it('spaces questions to the public instance a little over a second apart, across callers', async () => { + vi.useFakeTimers() + const done: number[] = [] + const start = Date.now() + await valhallaTurn(PUBLIC) + done.push(Date.now() - start) + const second = valhallaTurn(PUBLIC).then(() => done.push(Date.now() - start)) + const third = valhallaTurn(PUBLIC).then(() => done.push(Date.now() - start)) + + await vi.advanceTimersByTimeAsync(5000) + await Promise.all([second, third]) + + expect(done[0]).toBe(0) + expect(done[1]).toBeGreaterThanOrEqual(1100) + expect(done[2] - done[1]).toBeGreaterThanOrEqual(1100) + }) + + it("does not pace an operator's own host", async () => { + vi.useFakeTimers() + await valhallaTurn('https://valhalla.example.org') + let through = false + void valhallaTurn('https://valhalla.example.org').then(() => { through = true }) + await vi.advanceTimersByTimeAsync(0) + expect(through).toBe(true) + }) + + it('lets a caller that gives up go without taking the turn', async () => { + vi.useFakeTimers() + await valhallaTurn(PUBLIC) + const controller = new AbortController() + const waiting = valhallaTurn(PUBLIC, controller.signal) + controller.abort() + await vi.advanceTimersByTimeAsync(0) + await waiting + // The abandoned wait claimed nothing, so the next caller only waits out the first turn. + const start = Date.now() + const next = valhallaTurn(PUBLIC) + await vi.advanceTimersByTimeAsync(1100) + await next + expect(Date.now() - start).toBeLessThanOrEqual(1100) + }) }) const HAMBURG = { lat: 53.5511, lng: 9.9937 } diff --git a/client/src/components/Map/valhallaRoute.ts b/client/src/components/Map/valhallaRoute.ts index a8deff865..987b26db5 100644 --- a/client/src/components/Map/valhallaRoute.ts +++ b/client/src/components/Map/valhallaRoute.ts @@ -254,6 +254,35 @@ function pause(ms: number, signal?: AbortSignal): Promise { }) } +/** When this tab last put a question to the public instance. */ +let lastPublicAskAt = 0 + +/** + * Waits for this tab's turn at the public instance, which allows about one request a + * second per client. Every request to it goes through here, the route and the Tours + * elevation alike, so a tour's route and the height question right behind it, or two + * features asking at once, no longer reach it inside the same second and come back 429. + * The pauses the callers keep themselves still stand; this only adds the wait they + * cannot see, the one between unrelated callers. + * + * Another host is not paced: an operator's own Valhalla sets its own limits. + */ +export async function valhallaTurn(base: string, signal?: AbortSignal): Promise { + if (base !== FOSSGIS_VALHALLA) return + for (let wait = lastPublicAskAt + CHUNK_PAUSE_MS - Date.now(); wait > 0; wait = lastPublicAskAt + CHUNK_PAUSE_MS - Date.now()) { + await pause(wait, signal) + if (signal?.aborted) return + } + // Taken in the same synchronous step as the check, so two callers woken together + // cannot both see a free turn. + lastPublicAskAt = Date.now() +} + +/** Forget the last turn, for tests that start from a quiet host. */ +export function resetValhallaTurns(): void { + lastPublicAskAt = 0 +} + /** * Waits out the host's one request a second before the next question is put. * @@ -306,6 +335,8 @@ function routeBody( /** The parsed answer to one POST, or null for every failure. */ async function postRoute(base: string, body: Record, signal?: AbortSignal): Promise { try { + await valhallaTurn(base, signal) + if (signal?.aborted) return null const response = await fetch(`${base}/route`, { method: 'POST', // A browser cannot set User-Agent, so this is the whole of what FOSSGIS asks a diff --git a/client/src/components/Tours/planner/tourRouting.test.ts b/client/src/components/Tours/planner/tourRouting.test.ts index f37c5f2bb..d829d966f 100644 --- a/client/src/components/Tours/planner/tourRouting.test.ts +++ b/client/src/components/Tours/planner/tourRouting.test.ts @@ -1,13 +1,15 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { valhallaBase, valhallaRun } = vi.hoisted(() => ({ +const { valhallaBase, valhallaRun, valhallaTurn } = vi.hoisted(() => ({ valhallaBase: vi.fn(), valhallaRun: vi.fn(), + valhallaTurn: vi.fn(async () => {}), })) vi.mock('../../Map/valhallaRoute', () => ({ valhallaBase, valhallaRun, + valhallaTurn, })) import { enrichTourElevations, parseValhallaElevation, routeWalkingTour } from './tourRouting' @@ -67,6 +69,33 @@ describe('routeWalkingTour', () => { ]) }) + it('waits for its turn at the host before every height question', async () => { + const order: string[] = [] + valhallaTurn.mockImplementation(async () => { order.push('turn') }) + vi.stubGlobal('fetch', vi.fn(async (_url: string, init: { body: string }) => { + order.push('fetch') + const shape = (JSON.parse(init.body) as { shape: unknown[] }).shape + return { ok: true, json: async () => ({ range_height: shape.map((_, i) => [i, 100]) }) } + })) + const coordinates = Array.from({ length: 600 }, (_, i) => [48 + i / 1000, 11] as [number, number]) + + const result = await enrichTourElevations(coordinates) + + expect(order).toEqual(['turn', 'fetch', 'turn', 'fetch']) + expect(valhallaTurn).toHaveBeenCalledWith('https://valhalla.test', undefined) + expect(result!.every(point => point.length === 3)).toBe(true) + }) + + it('asks nothing once the caller gave up while waiting for its turn', async () => { + const controller = new AbortController() + valhallaTurn.mockImplementation(async () => { controller.abort() }) + const fetchSpy = vi.fn() + vi.stubGlobal('fetch', fetchSpy) + + await expect(enrichTourElevations([[48, 11], [48.01, 11.01]], controller.signal)).resolves.toBeNull() + expect(fetchSpy).not.toHaveBeenCalled() + }) + it('keeps the full route as 2D geometry when height enrichment is unavailable', async () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, json: async () => ({}) })) const coordinates: [number, number][] = [[48, 11], [48.01, 11.01]] diff --git a/client/src/components/Tours/planner/tourRouting.ts b/client/src/components/Tours/planner/tourRouting.ts index 393bc1f5e..975af0f5a 100644 --- a/client/src/components/Tours/planner/tourRouting.ts +++ b/client/src/components/Tours/planner/tourRouting.ts @@ -1,5 +1,5 @@ import type { Waypoint } from '../../../types' -import { valhallaBase, valhallaRun } from '../../Map/valhallaRoute' +import { valhallaBase, valhallaRun, valhallaTurn } from '../../Map/valhallaRoute' export interface TourRouteResult { coordinates: [number, number][] @@ -68,6 +68,11 @@ export async function enrichTourElevations( const chunk = coordinates.slice(offset, offset + HEIGHT_CHUNK_SIZE) if (chunk.length === 1 && offset > 0) break try { + // The route came from the same host a moment ago, and the public one allows a + // request a second: without the wait the first height question is refused and + // the tour cannot be saved for want of elevation. + await valhallaTurn(base, signal) + if (signal?.aborted) return null const response = await fetch(`${base}/height`, { method: 'POST', headers: { 'Content-Type': 'application/json', 'X-Client-Id': 'trek' },