From d6eac772aa47766495bbd7793914781de455a285 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 4 Sep 2026 21:24:15 +0200 Subject: [PATCH] fix(maps): ask a router that refuses exclude only once Both public hosts TREK ships with reject the parameter with HTTP 400: their car profile is built without excludable classes, and only a self-hosted OSRM built through the MLD pipeline has them. Every long leg was therefore paying two extra requests, against a host that allows one per second, to be refused twice. The first real attempt is the probe. Only a 400 is remembered, because only a 400 is about the host: a 429 is a rate limit, an abort is the user moving on, and a NoRoute means this leg has no way round the motorway while the next one might. Kept per base URL, so pointing the instance at an own router that does support it starts asking again without a reload. The comment above the fallback said none of this and read as a working feature, which is why "only one sensible way to drive this" keeps appearing on legs that plainly have another road. --- .../components/Map/RouteCalculator.test.ts | 19 ++++++++ client/src/components/Map/RouteCalculator.ts | 44 ++++++++++++++++--- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/client/src/components/Map/RouteCalculator.test.ts b/client/src/components/Map/RouteCalculator.test.ts index 9496d08a8..1631ad525 100644 --- a/client/src/components/Map/RouteCalculator.test.ts +++ b/client/src/components/Map/RouteCalculator.test.ts @@ -778,6 +778,25 @@ describe('calculateAlternatives', () => { expect(routes).toHaveLength(1) }) + it('FE-COMP-ROUTECALCULATOR-032: a host that rejects exclude is only asked once', async () => { + // Both public hosts TREK ships with answer 400 here, so without this every long leg + // pays two extra requests against a one-per-second limit to be refused twice. + const asked: string[] = [] + server.use(http.get('*/route/v1/driving/*', ({ request }) => { + const exclude = new URL(request.url).searchParams.get('exclude') + if (!exclude) return HttpResponse.json({ code: 'Ok', routes: [twoRoutes.routes[0]] }) + asked.push(exclude) + return new HttpResponse(null, { status: 400 }) + })) + + await calculateAlternatives([{ lat: 53, lng: 10 }, { lat: 52, lng: 13 }], 'driving') + const afterFirst = asked.length + await calculateAlternatives([{ lat: 51, lng: 9 }, { lat: 50, lng: 12 }], 'driving') + + expect(afterFirst).toBe(1) + expect(asked).toHaveLength(1) + }) + it('FE-COMP-ROUTECALCULATOR-028: no route at all is an empty list, not a throw', async () => { server.use(http.get(`${FOSSGIS.driving}/:coords`, () => HttpResponse.json({ code: 'NoRoute', routes: [] }))) diff --git a/client/src/components/Map/RouteCalculator.ts b/client/src/components/Map/RouteCalculator.ts index 872b96782..4f35a5c2d 100644 --- a/client/src/components/Map/RouteCalculator.ts +++ b/client/src/components/Map/RouteCalculator.ts @@ -490,12 +490,30 @@ function sameRoad(a: RouteAlternative, b: RouteAlternative): boolean { return Math.abs(a.distance - b.distance) < 200 && Math.abs(a.duration - b.duration) < 60 } +/** + * Hosts that have already answered "I do not do exclude", by base URL. + * + * Both public hosts TREK ships with reject the parameter outright with HTTP 400: the + * upstream car profile is built without excludable classes, and only a self-hosted OSRM + * built through the MLD pipeline supports it. Without this, every long leg on a default + * install pays two extra requests to be told twice that it cannot have what it asked for, + * against a host that allows one request per second. + * + * The first real attempt is the probe; there is no separate one. Only a 400 is recorded, + * because only a 400 is about the host rather than about this particular question: a 429 + * is a rate limit, an abort is the user moving on, and a `NoRoute` means this leg has no + * way round the motorway while the next one might. + * + * Per base URL rather than global, so pointing the instance at an own router that does + * support it starts asking again without a reload. + */ +const excludeUnsupported = new Set() + /** * One route with a road class left out, or null when the router will not or cannot. * - * Every failure is a null: an instance built without excludable classes answers - * `InvalidOptions`, a rate limit answers 429, and a leg with no way round the motorway - * answers `NoRoute`. None of those is worth an error on screen — they all just mean this + * Every failure is a null: a rate limit answers 429, and a leg with no way round the + * motorway answers `NoRoute`. Neither is worth an error on screen — they just mean this * particular question had no answer. */ async function routeExcluding( @@ -504,10 +522,17 @@ async function routeExcluding( exclude: string, signal?: AbortSignal, ): Promise { + const base = routeBaseFor(profile) + if (excludeUnsupported.has(base)) return null try { - const url = `${routeBaseFor(profile)}/${coords}?exclude=${exclude}&overview=full&geometries=geojson` + const url = `${base}/${coords}?exclude=${exclude}&overview=full&geometries=geojson` const response = await fetch(url, { signal }) - if (!response.ok) return null + if (!response.ok) { + // 400 is the router saying the parameter itself is not available here, which is + // true of every leg from now on. Anything else is about this request alone. + if (response.status === 400) excludeUnsupported.add(base) + return null + } const data = await response.json() const route = data?.code === 'Ok' && Array.isArray(data.routes) ? data.routes[0] : null if (!route?.geometry?.coordinates?.length) return null @@ -560,8 +585,13 @@ export async function calculateAlternatives( // long legs. Rather than reporting "only one sensible way", ask a different question: // the same drive without the motorway, and failing that without the tolls. Tried one at // a time and stopped as soon as one lands, so a leg that does have a second road costs - // one extra request rather than three, and the public hosts' one-per-second limit is - // not spent on questions already answered. + // one extra request rather than three. + // + // Worth knowing before reading further: on the two public hosts TREK ships with, this + // never produces anything. Their car profile is built without excludable classes and + // the parameter comes back as HTTP 400, which is why long legs keep reporting a single + // way to drive them. It works against a self-hosted OSRM built through the MLD + // pipeline, and `excludeUnsupported` makes sure a host that refuses is only asked once. if (routes.length < 2 && profile === 'driving') { for (const exclude of EXCLUDABLE_CLASSES) { if (signal?.aborted) break