Skip to content

Commit 94b7121

Browse files
rgarciaclaude
andcommitted
feat: opt-in control-plane fallback on browser_gone 404
Rework the routing-layer control-plane fallback to a tight, opt-in design that pairs with metro-api kernel#2317 (routed requests to a deleted/gone browser return HTTP 404 with body {"code":"browser_gone"}). Replaces the previous broad trigger (fall back on any 5xx for any idempotent GET), which retried the dead VM then fell back, adding latency on transient errors. New semantics in routeRequest, applied only when the request was actually routed to the VM (allowlisted subresource + cached route): fall back IFF method is GET, the routed (subresource + suffix) path is in the fallback-eligible registry, and the VM returns HTTP 404 whose JSON body has code == "browser_gone". On fallback, evict the cached route and re-issue the ORIGINAL request to the control plane exactly once (original URL, restore Authorization, drop the jwt query param); return that response, never loop. Success, transient 5xx, network errors, other 4xx, and non-browser_gone 404s propagate unchanged. The 404 body is read via response.clone() so a non-fallback response is returned intact. Per kernel#2317 there is no special response header, so the gone check keys off the body code only (no content-type gate), matching the python SDK. Adds an isFallbackEligible(subresource, suffix) predicate backed by a registry that is default-OFF. Pre-registers only the prospective pull endpoint GET /browsers/{id}/telemetry/events; adding future eligible endpoints is a one-line registry edit. Scoped to the fallback only: does not modify the default routing subresource list (owned by the telemetry-default-routing PR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a0c377d commit 94b7121

2 files changed

Lines changed: 332 additions & 2 deletions

File tree

src/lib/browser-routing.ts

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,56 @@ const BROWSER_POOL_ACQUIRE_PATH = /^\/(?:v\d+\/)?browser_pools\/[^/]+\/acquire\/
3434
const BROWSER_DELETE_BY_ID_PATH = /^\/(?:v\d+\/)?browsers\/([^/]+)\/?$/;
3535
const BROWSER_POOL_RELEASE_PATH = /^\/(?:v\d+\/)?browser_pools\/[^/]+\/release\/?$/;
3636

37+
/**
38+
* Registry of routed (subresource + suffix) paths that are eligible for
39+
* control-plane fallback when the VM reports the browser is authoritatively
40+
* gone (HTTP 404 with body code "browser_gone"). Everything not listed here is
41+
* fallback-OFF by default.
42+
*
43+
* Adding a future eligible endpoint is intentionally a one-line edit: append
44+
* another `${subresource} ${suffix}` entry below.
45+
*/
46+
const FALLBACK_ELIGIBLE_ROUTES = new Set<string>([
47+
// PROSPECTIVE: GET /browsers/{id}/telemetry/events. This pull endpoint /
48+
// method does not exist on the SDK yet; this entry pre-wires the opt-in so
49+
// control-plane fallback works the moment the method ships. Remove this
50+
// comment once the method lands.
51+
fallbackRouteKey('telemetry', '/events'),
52+
]);
53+
54+
const BROWSER_GONE_CODE = 'browser_gone';
55+
56+
function fallbackRouteKey(subresource: string, suffix: string): string {
57+
return `${subresource} ${suffix}`;
58+
}
59+
60+
/**
61+
* Whether a routed path (parsed subresource + suffix) is opted in to
62+
* control-plane fallback. Suffix is the portion after the subresource (e.g.
63+
* "/events"), or "" when the request targets the bare subresource.
64+
*/
65+
export function isFallbackEligible(subresource: string, suffix: string): boolean {
66+
return FALLBACK_ELIGIBLE_ROUTES.has(fallbackRouteKey(subresource, suffix));
67+
}
68+
69+
async function isBrowserGone404(response: Response): Promise<boolean> {
70+
if (response.status !== 404) {
71+
return false;
72+
}
73+
// Key off the body code only (per kernel#2317: there is NO special response
74+
// header). We do not gate on content-type so behavior matches the spec and
75+
// the python SDK, which simply attempts response.json(). A non-JSON body just
76+
// fails to parse and returns false.
77+
try {
78+
const body = await response.clone().json();
79+
return (
80+
!!body && typeof body === 'object' && (body as Record<string, unknown>)['code'] === BROWSER_GONE_CODE
81+
);
82+
} catch {
83+
return false;
84+
}
85+
}
86+
3787
export function browserRoutingSubresourcesFromEnv(): string[] {
3888
const raw = readBrowserRoutingSubresourcesEnv();
3989
if (raw === undefined) {
@@ -229,6 +279,7 @@ async function routeRequest(
229279

230280
const sessionId = decodeURIComponent(match[1] ?? '');
231281
const subresource = match[2] ?? '';
282+
const suffix = match[3] ?? '';
232283
if (!sessionId || !allowed.has(subresource)) {
233284
return innerFetch(input, init);
234285
}
@@ -237,7 +288,7 @@ async function routeRequest(
237288
return innerFetch(input, init);
238289
}
239290

240-
const target = new URL(joinURL(route.baseURL, `/${subresource}${match[3] ?? ''}`));
291+
const target = new URL(joinURL(route.baseURL, `/${subresource}${suffix}`));
241292
url.searchParams.forEach((value, key) => {
242293
if (key !== 'jwt') {
243294
target.searchParams.append(key, value);
@@ -249,7 +300,32 @@ async function routeRequest(
249300

250301
const headers = new Headers(request.headers);
251302
headers.delete('authorization');
252-
return innerFetch(target.toString(), buildRoutedInit(request, init, headers));
303+
const response = await innerFetch(target.toString(), buildRoutedInit(request, init, headers));
304+
305+
// Control-plane fallback: the request was actually routed to the VM, so this
306+
// is the only place we attempt it. Fall back IFF the method is GET, the routed
307+
// path is opted in, and the VM authoritatively reports the browser is gone
308+
// (HTTP 404 with body code "browser_gone"). Everything else — success,
309+
// transient 5xx, network errors, other 4xx, or a 404 without that code —
310+
// propagates unchanged.
311+
const method = request.method.toUpperCase();
312+
if (method !== 'GET' || !isFallbackEligible(subresource, suffix)) {
313+
return response;
314+
}
315+
if (!(await isBrowserGone404(response))) {
316+
return response;
317+
}
318+
319+
// The browser is authoritatively gone: evict the now-stale route and re-issue
320+
// the ORIGINAL request to the control plane exactly once. Restore the original
321+
// Authorization (still present on `request.headers`) and drop the jwt query
322+
// param so we hit the CP, not the VM. Never loops back through routing.
323+
cache.delete(sessionId);
324+
325+
const cpURL = new URL(request.url);
326+
cpURL.searchParams.delete('jwt');
327+
const cpHeaders = new Headers(request.headers);
328+
return innerFetch(cpURL.toString(), buildRoutedInit(request, init, cpHeaders));
253329
}
254330

255331
function buildRoutedInit(

tests/lib/browser-routing.test.ts

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
BrowserRouteCache,
55
browserRoutingSubresourcesFromEnv,
66
createRoutingFetch,
7+
isFallbackEligible,
78
} from '../../src/lib/browser-routing';
89

910
describe('browser routing', () => {
@@ -425,3 +426,256 @@ describe('browser routing', () => {
425426
});
426427
});
427428
});
429+
430+
describe('browser routing control-plane fallback', () => {
431+
const ELIGIBLE_PATH = 'https://api.example/browsers/sess-1/telemetry/events';
432+
const VM_BASE = 'http://browser-session.test/browser/kernel';
433+
434+
const browserGone404 = () =>
435+
Response.json({ code: 'browser_gone', message: 'browser not found' }, { status: 404 });
436+
437+
const warmedCache = () => {
438+
const cache = new BrowserRouteCache();
439+
cache.set({ sessionId: 'sess-1', baseURL: VM_BASE, jwt: 'token-abc' });
440+
return cache;
441+
};
442+
443+
// The "telemetry" subresource must be routed; the SUT's default-subresources
444+
// constant is out of scope for this PR, so tests enable it explicitly here.
445+
const makeFetch = (
446+
cache: BrowserRouteCache,
447+
inner: (input: any, init?: RequestInit) => Promise<Response>,
448+
) =>
449+
createRoutingFetch(inner as any, {
450+
apiBaseURL: 'https://api.example/',
451+
subresources: ['telemetry'],
452+
cache,
453+
});
454+
455+
const urlOf = (input: any) =>
456+
typeof input === 'string' ? input
457+
: input instanceof URL ? input.toString()
458+
: input.url;
459+
460+
test('isFallbackEligible only matches the pre-registered telemetry/events route', () => {
461+
expect(isFallbackEligible('telemetry', '/events')).toBe(true);
462+
expect(isFallbackEligible('telemetry', '/stream')).toBe(false);
463+
expect(isFallbackEligible('telemetry', '')).toBe(false);
464+
expect(isFallbackEligible('curl', '/events')).toBe(false);
465+
});
466+
467+
test('eligible GET + 404 browser_gone falls back to the control plane', async () => {
468+
const cache = warmedCache();
469+
const calls: Array<{ url: string; method: string; auth: string | null }> = [];
470+
const wrapped = makeFetch(cache, async (input, init) => {
471+
const url = urlOf(input);
472+
const headers = new Headers(init?.headers);
473+
calls.push({ url, method: (init?.method ?? 'GET').toUpperCase(), auth: headers.get('authorization') });
474+
if (url.startsWith(VM_BASE)) {
475+
return browserGone404();
476+
}
477+
return Response.json({ events: [] }, { status: 200 });
478+
});
479+
480+
const res = await wrapped(ELIGIBLE_PATH, {
481+
method: 'GET',
482+
headers: { authorization: 'Bearer original-key' },
483+
});
484+
485+
expect(res.status).toBe(200);
486+
expect(await res.json()).toEqual({ events: [] });
487+
// Exactly one VM attempt + exactly one CP re-issue. No loop.
488+
expect(calls).toHaveLength(2);
489+
// VM attempt: routed, Authorization stripped, jwt added.
490+
expect(calls[0]?.url).toBe(`${VM_BASE}/telemetry/events?jwt=token-abc`);
491+
expect(calls[0]?.auth).toBeNull();
492+
// CP re-issue: original URL, Authorization restored, jwt dropped.
493+
expect(calls[1]?.url).toBe(ELIGIBLE_PATH);
494+
expect(calls[1]?.method).toBe('GET');
495+
expect(calls[1]?.auth).toBe('Bearer original-key');
496+
// Route evicted as authoritatively gone.
497+
expect(cache.get('sess-1')).toBeUndefined();
498+
});
499+
500+
test('eligible GET + 404 browser_gone where CP also errors returns CP response, no loop', async () => {
501+
const cache = warmedCache();
502+
const calls: string[] = [];
503+
const wrapped = makeFetch(cache, async (input) => {
504+
const url = urlOf(input);
505+
calls.push(url);
506+
if (url.startsWith(VM_BASE)) {
507+
return browserGone404();
508+
}
509+
return new Response('boom', { status: 500, headers: { 'content-type': 'text/plain' } });
510+
});
511+
512+
const res = await wrapped(ELIGIBLE_PATH, { method: 'GET', headers: { authorization: 'Bearer k' } });
513+
514+
expect(res.status).toBe(500);
515+
expect(await res.text()).toBe('boom');
516+
// One VM attempt + one CP re-issue only; the CP 500 is returned as-is.
517+
expect(calls).toHaveLength(2);
518+
expect(calls[0]).toBe(`${VM_BASE}/telemetry/events?jwt=token-abc`);
519+
expect(calls[1]).toBe(ELIGIBLE_PATH);
520+
});
521+
522+
test('not-eligible routed path + 404 browser_gone does NOT fall back', async () => {
523+
const cache = warmedCache();
524+
const calls: string[] = [];
525+
const wrapped = makeFetch(cache, async (input) => {
526+
calls.push(urlOf(input));
527+
return browserGone404();
528+
});
529+
530+
const res = await wrapped('https://api.example/browsers/sess-1/telemetry/stream', {
531+
method: 'GET',
532+
headers: { authorization: 'Bearer k' },
533+
});
534+
535+
expect(res.status).toBe(404);
536+
expect(await res.json()).toEqual({ code: 'browser_gone', message: 'browser not found' });
537+
expect(calls).toEqual([`${VM_BASE}/telemetry/stream?jwt=token-abc`]);
538+
expect(cache.get('sess-1')).toMatchObject({ sessionId: 'sess-1' });
539+
});
540+
541+
test('eligible GET + transient 502 does NOT fall back (propagated)', async () => {
542+
const cache = warmedCache();
543+
const calls: string[] = [];
544+
const wrapped = makeFetch(cache, async (input) => {
545+
calls.push(urlOf(input));
546+
return new Response('bad gateway', { status: 502, headers: { 'content-type': 'text/plain' } });
547+
});
548+
549+
const res = await wrapped(ELIGIBLE_PATH, { method: 'GET', headers: { authorization: 'Bearer k' } });
550+
551+
expect(res.status).toBe(502);
552+
expect(calls).toEqual([`${VM_BASE}/telemetry/events?jwt=token-abc`]);
553+
expect(cache.get('sess-1')).toMatchObject({ sessionId: 'sess-1' });
554+
});
555+
556+
test('eligible GET + connection error does NOT fall back (propagated)', async () => {
557+
const cache = warmedCache();
558+
let attempts = 0;
559+
const wrapped = makeFetch(cache, async () => {
560+
attempts++;
561+
throw new TypeError('network down');
562+
});
563+
564+
await expect(
565+
wrapped(ELIGIBLE_PATH, { method: 'GET', headers: { authorization: 'Bearer k' } }),
566+
).rejects.toThrow(/network down/);
567+
expect(attempts).toBe(1);
568+
expect(cache.get('sess-1')).toMatchObject({ sessionId: 'sess-1' });
569+
});
570+
571+
test('eligible GET + 200 does NOT fall back', async () => {
572+
const cache = warmedCache();
573+
const calls: string[] = [];
574+
const wrapped = makeFetch(cache, async (input) => {
575+
calls.push(urlOf(input));
576+
return Response.json({ events: [{ seq: 1 }] }, { status: 200 });
577+
});
578+
579+
const res = await wrapped(ELIGIBLE_PATH, { method: 'GET', headers: { authorization: 'Bearer k' } });
580+
581+
expect(res.status).toBe(200);
582+
expect(await res.json()).toEqual({ events: [{ seq: 1 }] });
583+
expect(calls).toEqual([`${VM_BASE}/telemetry/events?jwt=token-abc`]);
584+
expect(cache.get('sess-1')).toMatchObject({ sessionId: 'sess-1' });
585+
});
586+
587+
test('eligible path but POST does NOT fall back even on 404 browser_gone', async () => {
588+
const cache = warmedCache();
589+
const calls: Array<{ url: string; method: string }> = [];
590+
const wrapped = makeFetch(cache, async (input, init) => {
591+
calls.push({ url: urlOf(input), method: (init?.method ?? 'GET').toUpperCase() });
592+
return browserGone404();
593+
});
594+
595+
const res = await wrapped(ELIGIBLE_PATH, {
596+
method: 'POST',
597+
headers: { authorization: 'Bearer k' },
598+
body: '{}',
599+
});
600+
601+
expect(res.status).toBe(404);
602+
expect(calls).toHaveLength(1);
603+
expect(calls[0]?.method).toBe('POST');
604+
expect(cache.get('sess-1')).toMatchObject({ sessionId: 'sess-1' });
605+
});
606+
607+
test('a 404 whose body code is not browser_gone does NOT fall back', async () => {
608+
const cache = warmedCache();
609+
const calls: string[] = [];
610+
const wrapped = makeFetch(cache, async (input) => {
611+
calls.push(urlOf(input));
612+
return Response.json({ code: 'not_found', message: 'no such event' }, { status: 404 });
613+
});
614+
615+
const res = await wrapped(ELIGIBLE_PATH, { method: 'GET', headers: { authorization: 'Bearer k' } });
616+
617+
expect(res.status).toBe(404);
618+
expect(await res.json()).toEqual({ code: 'not_found', message: 'no such event' });
619+
expect(calls).toEqual([`${VM_BASE}/telemetry/events?jwt=token-abc`]);
620+
expect(cache.get('sess-1')).toMatchObject({ sessionId: 'sess-1' });
621+
});
622+
623+
test('keys off the body code only: browser_gone 404 without a JSON content-type still falls back', async () => {
624+
const cache = warmedCache();
625+
const calls: string[] = [];
626+
const wrapped = makeFetch(cache, async (input) => {
627+
const url = urlOf(input);
628+
calls.push(url);
629+
if (url.startsWith(VM_BASE)) {
630+
// Body is JSON but the proxy omitted the application/json content-type.
631+
// Per kernel#2317 we key off the body code only, so this must fall back.
632+
return new Response(JSON.stringify({ code: 'browser_gone', message: 'gone' }), {
633+
status: 404,
634+
headers: { 'content-type': 'text/plain' },
635+
});
636+
}
637+
return Response.json({ events: [] }, { status: 200 });
638+
});
639+
640+
const res = await wrapped(ELIGIBLE_PATH, { method: 'GET', headers: { authorization: 'Bearer k' } });
641+
642+
expect(res.status).toBe(200);
643+
expect(await res.json()).toEqual({ events: [] });
644+
expect(calls).toEqual([`${VM_BASE}/telemetry/events?jwt=token-abc`, ELIGIBLE_PATH]);
645+
expect(cache.get('sess-1')).toBeUndefined();
646+
});
647+
648+
test('a non-JSON 404 body does NOT fall back', async () => {
649+
const cache = warmedCache();
650+
const calls: string[] = [];
651+
const wrapped = makeFetch(cache, async (input) => {
652+
calls.push(urlOf(input));
653+
return new Response('plain not found', {
654+
status: 404,
655+
headers: { 'content-type': 'text/plain' },
656+
});
657+
});
658+
659+
const res = await wrapped(ELIGIBLE_PATH, { method: 'GET', headers: { authorization: 'Bearer k' } });
660+
661+
expect(res.status).toBe(404);
662+
expect(await res.text()).toBe('plain not found');
663+
expect(calls).toEqual([`${VM_BASE}/telemetry/events?jwt=token-abc`]);
664+
expect(cache.get('sess-1')).toMatchObject({ sessionId: 'sess-1' });
665+
});
666+
667+
test('non-routed request (cache miss) is untouched and never falls back', async () => {
668+
const cache = new BrowserRouteCache(); // no route warmed -> not routed to VM
669+
const calls: string[] = [];
670+
const wrapped = makeFetch(cache, async (input) => {
671+
calls.push(urlOf(input));
672+
return browserGone404();
673+
});
674+
675+
const res = await wrapped(ELIGIBLE_PATH, { method: 'GET', headers: { authorization: 'Bearer k' } });
676+
677+
expect(res.status).toBe(404);
678+
// Hit the original CP URL directly (not routed to a VM), and no re-issue.
679+
expect(calls).toEqual([ELIGIBLE_PATH]);
680+
});
681+
});

0 commit comments

Comments
 (0)