Skip to content

Commit d1e21ed

Browse files
committed
Run the route eviction hook before caller response hooks
A response hook registered by the caller runs in registration order, so one that reads a failing 401/403 body or raises would skip the eviction hook appended after it and leave the stale route cached. Prepend the hook in both the sync and async installers; registration stays once per route cache.
1 parent d818b10 commit d1e21ed

2 files changed

Lines changed: 114 additions & 3 deletions

File tree

src/kernel/lib/browser_routing/routing.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,8 @@ def install_stale_direct_vm_auth_eviction(client: httpx.Client, *, cache: Browse
209209
whose body read fails — the read error surfaces from `send()` instead and the
210210
dead route would stay cached, wedging every later call for that session. A
211211
response event hook runs after the status is known and before any body is
212-
read, which keeps eviction independent of the body.
212+
read, which keeps eviction independent of the body. It is prepended so that a
213+
caller-supplied hook cannot pre-empt it by reading a failing body or raising.
213214
"""
214215
hooks = client.event_hooks.setdefault("response", [])
215216
if _has_eviction_hook(hooks, cache):
@@ -220,7 +221,7 @@ def evict(response: httpx.Response) -> None:
220221
maybe_evict_browser_route_from_response(response, cache=cache)
221222

222223
setattr(evict, _EVICTION_HOOK_CACHE_ATTR, cache)
223-
hooks.append(evict)
224+
hooks.insert(0, evict)
224225

225226

226227
def install_async_stale_direct_vm_auth_eviction(client: httpx.AsyncClient, *, cache: BrowserRouteCache) -> None:
@@ -234,7 +235,7 @@ async def evict(response: httpx.Response) -> None:
234235
maybe_evict_browser_route_from_response(response, cache=cache)
235236

236237
setattr(evict, _EVICTION_HOOK_CACHE_ATTR, cache)
237-
hooks.append(evict)
238+
hooks.insert(0, evict)
238239

239240

240241
def _has_eviction_hook(hooks: list[Any], cache: BrowserRouteCache) -> bool:

tests/test_browser_routing.py

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,3 +1353,113 @@ def test_copied_client_registers_one_route_eviction_hook() -> None:
13531353
assert copied.browser_route_cache is client.browser_route_cache
13541354
hooks = client._client.event_hooks["response"] # pyright: ignore[reportPrivateUsage]
13551355
assert len(hooks) == 1
1356+
1357+
1358+
def test_route_eviction_hook_runs_before_caller_response_hooks(
1359+
monkeypatch: pytest.MonkeyPatch,
1360+
) -> None:
1361+
monkeypatch.delenv("KERNEL_BROWSER_ROUTING_SUBRESOURCES", raising=False)
1362+
requests: list[httpx.Request] = []
1363+
caller_hook_statuses: list[int] = []
1364+
1365+
def handle_request(request: httpx.Request) -> httpx.Response:
1366+
requests.append(request)
1367+
if "browser-session.test" in str(request.url):
1368+
return httpx.Response(401, stream=_FailingSyncStream(), headers={"content-type": "text/plain"})
1369+
return httpx.Response(200, content=b"png", headers={"content-type": "image/png"})
1370+
1371+
def caller_hook(response: httpx.Response) -> None:
1372+
caller_hook_statuses.append(response.status_code)
1373+
if response.status_code in {401, 403}:
1374+
# Reading a failing body raises out of the hook chain, which would
1375+
# skip any eviction hook registered after this one.
1376+
response.read()
1377+
1378+
http_client = httpx.Client(
1379+
transport=httpx.MockTransport(handle_request),
1380+
event_hooks={"response": [caller_hook]},
1381+
)
1382+
with Kernel(
1383+
base_url=base_url,
1384+
api_key=api_key,
1385+
max_retries=0,
1386+
http_client=http_client,
1387+
_strict_response_validation=True,
1388+
) as client:
1389+
_cache_browser(client)
1390+
assert http_client.event_hooks["response"][-1] is caller_hook
1391+
with pytest.raises(APIConnectionError):
1392+
client.browsers.computer.capture_screenshot("sess-1")
1393+
assert caller_hook_statuses == [401]
1394+
assert client.browser_route_cache.get("sess-1") is None
1395+
1396+
client.browsers.computer.capture_screenshot("sess-1")
1397+
1398+
assert str(requests[0].url).startswith("http://browser-session.test/browser/kernel/computer/screenshot")
1399+
assert requests[1].url == httpx.URL(f"{base_url}/browsers/sess-1/computer/screenshot")
1400+
assert requests[1].url.params.get("jwt") is None
1401+
assert requests[1].headers.get("Authorization") == f"Bearer {api_key}"
1402+
1403+
1404+
@pytest.mark.asyncio
1405+
async def test_async_route_eviction_hook_runs_before_caller_response_hooks(
1406+
monkeypatch: pytest.MonkeyPatch,
1407+
) -> None:
1408+
monkeypatch.delenv("KERNEL_BROWSER_ROUTING_SUBRESOURCES", raising=False)
1409+
requests: list[httpx.Request] = []
1410+
caller_hook_statuses: list[int] = []
1411+
1412+
async def handle_request(request: httpx.Request) -> httpx.Response:
1413+
requests.append(request)
1414+
if "browser-session.test" in str(request.url):
1415+
return httpx.Response(403, stream=_FailingAsyncStream(), headers={"content-type": "text/plain"})
1416+
return httpx.Response(200, content=b"png", headers={"content-type": "image/png"})
1417+
1418+
async def caller_hook(response: httpx.Response) -> None:
1419+
caller_hook_statuses.append(response.status_code)
1420+
if response.status_code in {401, 403}:
1421+
await response.aread()
1422+
1423+
http_client = httpx.AsyncClient(
1424+
transport=httpx.MockTransport(handle_request),
1425+
event_hooks={"response": [caller_hook]},
1426+
)
1427+
async with AsyncKernel(
1428+
base_url=base_url,
1429+
api_key=api_key,
1430+
max_retries=0,
1431+
http_client=http_client,
1432+
_strict_response_validation=True,
1433+
) as client:
1434+
route = browser_route_from_browser(_fake_browser())
1435+
assert route is not None
1436+
client.browser_route_cache.set(route)
1437+
assert http_client.event_hooks["response"][-1] is caller_hook
1438+
with pytest.raises(APIConnectionError):
1439+
await client.browsers.computer.capture_screenshot("sess-1")
1440+
assert caller_hook_statuses == [403]
1441+
assert client.browser_route_cache.get("sess-1") is None
1442+
1443+
await client.browsers.computer.capture_screenshot("sess-1")
1444+
1445+
assert str(requests[0].url).startswith("http://browser-session.test/browser/kernel/computer/screenshot")
1446+
assert requests[1].url == httpx.URL(f"{base_url}/browsers/sess-1/computer/screenshot")
1447+
assert requests[1].url.params.get("jwt") is None
1448+
assert requests[1].headers.get("Authorization") == f"Bearer {api_key}"
1449+
1450+
1451+
def test_route_eviction_hook_is_registered_once_before_caller_hooks() -> None:
1452+
def caller_hook(_response: httpx.Response) -> None: # pragma: no cover - never invoked
1453+
return None
1454+
1455+
http_client = httpx.Client(event_hooks={"response": [caller_hook]})
1456+
with Kernel(
1457+
base_url=base_url,
1458+
api_key=api_key,
1459+
http_client=http_client,
1460+
_strict_response_validation=True,
1461+
) as client:
1462+
client.copy(api_key="sk-456")
1463+
hooks = http_client.event_hooks["response"]
1464+
assert len(hooks) == 2
1465+
assert hooks[1] is caller_hook

0 commit comments

Comments
 (0)