mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-13 06:42:23 +00:00
fix(routes): keep the 12 s budget from arrival and name the outage to voice
Starting the upstream timeout after the outbound queue let a request run up to about 19 s, past the Directions layer's 15 s; the budget again runs from arrival, queue wait included, and a budget spent before the request leaves the queue answers 429 "routing busy" rather than an outage. plan_route threw "Routing HTTP 502" for an outage, and voice passes only the message on; it now throws "Routing service unavailable (HTTP 502)". Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
536f371dde
commit
1bff1961b4
6 files changed
+53
-25
No files matched your search
@@ -1,4 +1,4 @@
|
|||||||
- A routing service outage (an error status, no answer or a non-JSON body)
|
- When the routing service is down (an error status, no answer or a non-JSON
|
||||||
is reported as "routing service unavailable" instead of "no route found",
|
body), voice route planning hears "Routing service unavailable" and the
|
||||||
so voice route planning and the Directions layer no longer blame the
|
Directions layer shows "Routing unavailable" instead of "no route found",
|
||||||
request (quanticsoul4772, #1509).
|
so neither blames the request (quanticsoul4772, #1509).
|
||||||
@@ -4376,13 +4376,15 @@ service's usage policy states; past a bounded queue the answer is a 429 with a
|
|||||||
upstream host is pinned and a redirect is refused rather than followed, and a
|
upstream host is pinned and a redirect is refused rather than followed, and a
|
||||||
rate limit from the routing service is reported as one instead of as "no route
|
rate limit from the routing service is reported as one instead of as "no route
|
||||||
found". So is an outage: any failure status but 400 (a 5xx, a 403 block, a
|
found". So is an outage: any failure status but 400 (a 5xx, a 403 block, a
|
||||||
404), no answer (a 12 s timeout counted from when the request leaves, not
|
404), no answer (timeout, network error) or a body that is not JSON answers
|
||||||
while it waits for an outbound slot; a network error) or a body that is not
|
`502 routing service unavailable`. `plan_route` throws `Routing service
|
||||||
JSON answers
|
unavailable (HTTP 502)`, which is what voice hears, and the layer shows
|
||||||
`502 routing service unavailable`, which `plan_route`
|
`Routing unavailable (HTTP 502)`; only OSRM's own refusal (a 400, or a 200
|
||||||
reports as `unavailable` and the layer as `Routing unavailable (HTTP 502)`;
|
without a route) reads "no route found". The 12 s timeout runs from the
|
||||||
only OSRM's own refusal (a 400, or a 200 without a route) reads "no route
|
request's arrival, queue wait included; a budget spent before the request
|
||||||
found".
|
leaves the queue answers 429 "routing busy". A request that waited in the
|
||||||
|
queue gives OSRM the rest of the 12 s, so a slow answer then still reads as
|
||||||
|
unavailable.
|
||||||
`src/data/routeSteps.js` turns OSRM maneuver type / modifier / exit /
|
`src/data/routeSteps.js` turns OSRM maneuver type / modifier / exit /
|
||||||
road name into one plain-English sentence per decision, folding
|
road name into one plain-English sentence per decision, folding
|
||||||
`exit roundabout` steps into the roundabout they leave, and caps one route at
|
`exit roundabout` steps into the roundabout they leave, and caps one route at
|
||||||
|
|||||||
@@ -178,14 +178,16 @@ async function fetchRoute({
|
|||||||
`${base.replace(/\/$/, '')}/route/v1/${osrmProfile}/${coords}` +
|
`${base.replace(/\/$/, '')}/route/v1/${osrmProfile}/${coords}` +
|
||||||
`?overview=full&geometries=geojson&alternatives=false&steps=${withSteps ? 'true' : 'false'}`;
|
`?overview=full&geometries=geojson&alternatives=false&steps=${withSteps ? 'true' : 'false'}`;
|
||||||
const controller = new AbortController();
|
const controller = new AbortController();
|
||||||
let timer;
|
// One 12 s budget from arrival, queue wait included, so callers sized to it
|
||||||
|
// (the Directions layer gives up at 15 s) hear back in time.
|
||||||
|
const timer = setTimeout(() => controller.abort(), 12000);
|
||||||
let osrm;
|
let osrm;
|
||||||
try {
|
try {
|
||||||
const upstreamRes = await throughUpstreamGate(async () => {
|
const upstreamRes = await throughUpstreamGate(async () => {
|
||||||
// The 12 s budget starts when the request leaves, not while it waits
|
// A budget spent waiting in our own queue is our backlog, not the
|
||||||
// for an outbound slot, so a wait in our own queue is never reported
|
// routing service failing.
|
||||||
// as the routing service being unavailable.
|
if (controller.signal.aborted)
|
||||||
timer = setTimeout(() => controller.abort(), 12000);
|
throw new RouteBusyError(_upstreamIntervalMs);
|
||||||
try {
|
try {
|
||||||
return await fetchImpl(upstream, {
|
return await fetchImpl(upstream, {
|
||||||
signal: controller.signal,
|
signal: controller.signal,
|
||||||
|
|||||||
@@ -311,15 +311,16 @@ test('callers sharing one failed upstream call all hear the outage, and it is no
|
|||||||
assert.equal(calls, 2, 'the next request asks the routing service again');
|
assert.equal(calls, 2, 'the next request asks the routing service again');
|
||||||
});
|
});
|
||||||
|
|
||||||
test('a wait for an outbound slot does not count toward the upstream timeout', async (t) => {
|
test('a budget spent in the outbound queue is reported as busy, not as an outage', async (t) => {
|
||||||
_resetRouteUpstreamForTest();
|
_resetRouteUpstreamForTest();
|
||||||
// The second request waits 13 s for its slot, longer than the 12 s budget.
|
// The second request waits 13 s for its slot, past the 12 s budget.
|
||||||
_setRouteUpstreamIntervalForTest(13_000);
|
_setRouteUpstreamIntervalForTest(13_000);
|
||||||
t.after(() => _resetRouteUpstreamForTest());
|
t.after(() => _resetRouteUpstreamForTest());
|
||||||
t.mock.timers.enable({ apis: ['setTimeout', 'Date'], now: Date.now() });
|
t.mock.timers.enable({ apis: ['setTimeout', 'Date'], now: Date.now() });
|
||||||
const request = install(installRouteMiddleware);
|
const request = install(installRouteMiddleware);
|
||||||
t.mock.method(globalThis, 'fetch', async (_url, { signal }) => {
|
let calls = 0;
|
||||||
signal.throwIfAborted();
|
t.mock.method(globalThis, 'fetch', async () => {
|
||||||
|
calls += 1;
|
||||||
return Response.json(osrmRouteWithSteps(2));
|
return Response.json(osrmRouteWithSteps(2));
|
||||||
});
|
});
|
||||||
const first = request(
|
const first = request(
|
||||||
@@ -333,8 +334,9 @@ test('a wait for an outbound slot does not count toward the upstream timeout', a
|
|||||||
assert.equal((await first).body.ok, true);
|
assert.equal((await first).body.ok, true);
|
||||||
t.mock.timers.tick(13_000);
|
t.mock.timers.tick(13_000);
|
||||||
const result = await second;
|
const result = await second;
|
||||||
assert.equal(result.statusCode, 200);
|
assert.equal(result.statusCode, 429);
|
||||||
assert.equal(result.body.ok, true);
|
assert.match(result.body.error, /routing busy/);
|
||||||
|
assert.equal(calls, 1, 'the spent request never reaches the routing service');
|
||||||
});
|
});
|
||||||
|
|
||||||
/** One OSRM route with `count` maneuvers, for the step tests. */
|
/** One OSRM route with `count` maneuvers, for the step tests. */
|
||||||
|
|||||||
+7
-3
@@ -190,9 +190,13 @@ export function createRouteService({
|
|||||||
/* the status below is authoritative */
|
/* the status below is authoritative */
|
||||||
}
|
}
|
||||||
// A rate limit or an outage (5xx) is the routing service's failure, not
|
// A rate limit or an outage (5xx) is the routing service's failure, not
|
||||||
// a missing route, whatever the body says.
|
// a missing route, whatever the body says. Voice hears this message.
|
||||||
if (response.status === 429 || response.status >= 500)
|
if (response.status === 429) throw httpFailure(response, 'Routing');
|
||||||
throw httpFailure(response, 'Routing');
|
if (response.status >= 500)
|
||||||
|
throw new ToolError(
|
||||||
|
'unavailable',
|
||||||
|
`Routing service unavailable (HTTP ${response.status})`,
|
||||||
|
);
|
||||||
if (payload?.ok === false || response.status === 400)
|
if (payload?.ok === false || response.status === 400)
|
||||||
return { ok: false, error: payload?.error || 'no route found' };
|
return { ok: false, error: payload?.error || 'no route found' };
|
||||||
if (!response.ok) throw httpFailure(response, 'Routing');
|
if (!response.ok) throw httpFailure(response, 'Routing');
|
||||||
|
|||||||
@@ -283,6 +283,24 @@ test('a routing outage is reported as unavailable, not as invalid arguments', as
|
|||||||
assert.equal(result.structuredContent.error, 'unavailable');
|
assert.equal(result.structuredContent.error, 'unavailable');
|
||||||
assert.doesNotMatch(result.content[0].text, /No walk route/);
|
assert.doesNotMatch(result.content[0].text, /No walk route/);
|
||||||
}
|
}
|
||||||
|
// Voice passes only the message on, so it names the outage itself.
|
||||||
|
const outage = createRouteService({
|
||||||
|
fetchImpl: async () =>
|
||||||
|
Response.json(
|
||||||
|
{ ok: false, error: 'routing service unavailable' },
|
||||||
|
{ status: 502 },
|
||||||
|
),
|
||||||
|
});
|
||||||
|
await assert.rejects(
|
||||||
|
outage.route(
|
||||||
|
[
|
||||||
|
{ lat: 30, lon: -97 },
|
||||||
|
{ lat: 30.01, lon: -97.01 },
|
||||||
|
],
|
||||||
|
'foot',
|
||||||
|
),
|
||||||
|
{ message: 'Routing service unavailable (HTTP 502)' },
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('place answers say when the search limits may have left places out', async () => {
|
test('place answers say when the search limits may have left places out', async () => {
|
||||||
|
|||||||
Reference in new issue
Block a user