Skip to content

Commit 92acf13

Browse files
committed
Prevent retries of unreplayable VM bodies
1 parent 5d8ec7f commit 92acf13

3 files changed

Lines changed: 112 additions & 7 deletions

File tree

src/kernel/_client.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
maybe_evict_browser_route_from_response,
4848
should_retry_direct_vm_connection_error,
4949
install_async_stale_direct_vm_auth_eviction,
50+
direct_vm_request_body_is_known_unreplayable,
5051
maybe_populate_browser_route_cache_from_response,
5152
)
5253

@@ -382,6 +383,8 @@ def _should_retry_on_connection_error(self, request: httpx.Request) -> bool:
382383

383384
@override
384385
def _should_retry(self, response: httpx.Response) -> bool:
386+
if direct_vm_request_body_is_known_unreplayable(response.request):
387+
return False
385388
if is_stale_direct_vm_auth_response(response):
386389
# The route was already evicted by the response hook; retry only when
387390
# the body can be rebuilt, otherwise the caller sees the original auth
@@ -778,6 +781,8 @@ def _should_retry_on_connection_error(self, request: httpx.Request) -> bool:
778781

779782
@override
780783
def _should_retry(self, response: httpx.Response) -> bool:
784+
if direct_vm_request_body_is_known_unreplayable(response.request):
785+
return False
781786
if is_stale_direct_vm_auth_response(response):
782787
# The route was already evicted by the response hook; retry only when
783788
# the body can be rebuilt, otherwise the caller sees the original auth

src/kernel/lib/browser_routing/routing.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -251,13 +251,9 @@ def _has_eviction_hook(hooks: list[Any], cache: BrowserRouteCache) -> bool:
251251

252252

253253
def should_retry_direct_vm_connection_error(request: httpx.Request) -> bool:
254-
"""Prevent generic retries from replaying a stream after stale VM auth.
255-
256-
httpx may raise while eagerly reading the error response body, before the
257-
SDK can pass the response to `_should_retry`. The response hook records the
258-
known stale-auth status on the request so this connection-error path can
259-
apply the same body replay check.
260-
"""
254+
"""Prevent connection retries from replaying an unreplayable VM request body."""
255+
if direct_vm_request_body_is_known_unreplayable(request):
256+
return False
261257
if not request.extensions.get(_STALE_DIRECT_VM_AUTH_REQUEST_EXTENSION):
262258
return True
263259
return direct_vm_request_body_is_replayable(request)
@@ -276,6 +272,10 @@ def should_retry_stale_direct_vm_auth(response: httpx.Response) -> bool:
276272
return direct_vm_request_body_is_replayable(response.request)
277273

278274

275+
def direct_vm_request_body_is_known_unreplayable(request: httpx.Request) -> bool:
276+
return request.extensions.get(_DIRECT_VM_BODY_REPLAYABLE_REQUEST_EXTENSION) is False
277+
278+
279279
def direct_vm_request_body_is_replayable(request: httpx.Request) -> bool:
280280
replayable = request.extensions.get(_DIRECT_VM_BODY_REPLAYABLE_REQUEST_EXTENSION)
281281
if isinstance(replayable, bool):

tests/test_browser_routing.py

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,6 +1468,106 @@ async def handle_async_request(self, request: httpx.Request) -> httpx.Response:
14681468
assert requests[1] == (httpx.URL(f"{base_url}/browsers/sess-1/fs/write_file?path=%2Ftmp%2Fx"), b"next")
14691469

14701470

1471+
@pytest.mark.parametrize(
1472+
("failure_type", "expected_error"),
1473+
[
1474+
(httpx.ReadError, APIConnectionError),
1475+
(httpx.ReadTimeout, APITimeoutError),
1476+
(None, InternalServerError),
1477+
],
1478+
)
1479+
def test_direct_vm_failure_does_not_retry_unreplayable_write(
1480+
monkeypatch: pytest.MonkeyPatch,
1481+
failure_type: type[httpx.TransportError] | None,
1482+
expected_error: type[Exception],
1483+
) -> None:
1484+
monkeypatch.delenv("KERNEL_BROWSER_ROUTING_SUBRESOURCES", raising=False)
1485+
monkeypatch.setattr("kernel._base_client.SyncAPIClient._sleep_for_retry", _skip_retry_sleep)
1486+
requests: list[tuple[httpx.URL, bytes]] = []
1487+
1488+
class Transport(httpx.BaseTransport):
1489+
@override
1490+
def handle_request(self, request: httpx.Request) -> httpx.Response:
1491+
body = b"".join(cast(Iterator[bytes], request.stream))
1492+
requests.append((request.url, body))
1493+
if failure_type is not None:
1494+
raise failure_type("connection failed after sending the request", request=request)
1495+
return httpx.Response(500, json={"error": "boom"})
1496+
1497+
http_client = httpx.Client(transport=Transport())
1498+
with Kernel(
1499+
base_url=base_url,
1500+
api_key=api_key,
1501+
http_client=http_client,
1502+
_strict_response_validation=True,
1503+
) as client:
1504+
_cache_browser(client)
1505+
with pytest.raises(expected_error):
1506+
client.browsers.fs.write_file("sess-1", _UnseekableFile(b"payload"), path="/tmp/x")
1507+
1508+
assert client.browser_route_cache.get("sess-1") is not None
1509+
1510+
assert requests == [
1511+
(
1512+
httpx.URL("http://browser-session.test/browser/kernel/fs/write_file?path=%2Ftmp%2Fx&jwt=token-abc"),
1513+
b"payload",
1514+
)
1515+
]
1516+
1517+
1518+
@pytest.mark.asyncio
1519+
@pytest.mark.parametrize(
1520+
("failure_type", "expected_error"),
1521+
[
1522+
(httpx.ReadError, APIConnectionError),
1523+
(httpx.ReadTimeout, APITimeoutError),
1524+
(None, InternalServerError),
1525+
],
1526+
)
1527+
async def test_async_direct_vm_failure_does_not_retry_unreplayable_write(
1528+
monkeypatch: pytest.MonkeyPatch,
1529+
failure_type: type[httpx.TransportError] | None,
1530+
expected_error: type[Exception],
1531+
) -> None:
1532+
monkeypatch.delenv("KERNEL_BROWSER_ROUTING_SUBRESOURCES", raising=False)
1533+
monkeypatch.setattr("kernel._base_client.AsyncAPIClient._sleep_for_retry", _skip_async_retry_sleep)
1534+
requests: list[tuple[httpx.URL, bytes]] = []
1535+
1536+
async def payload() -> AsyncIterator[bytes]:
1537+
yield b"payload"
1538+
1539+
class Transport(httpx.AsyncBaseTransport):
1540+
@override
1541+
async def handle_async_request(self, request: httpx.Request) -> httpx.Response:
1542+
body = b"".join([chunk async for chunk in cast(AsyncIterator[bytes], request.stream)])
1543+
requests.append((request.url, body))
1544+
if failure_type is not None:
1545+
raise failure_type("connection failed after sending the request", request=request)
1546+
return httpx.Response(500, json={"error": "boom"})
1547+
1548+
http_client = httpx.AsyncClient(transport=Transport())
1549+
async with AsyncKernel(
1550+
base_url=base_url,
1551+
api_key=api_key,
1552+
http_client=http_client,
1553+
_strict_response_validation=True,
1554+
) as client:
1555+
route = browser_route_from_browser(_fake_browser())
1556+
assert route is not None
1557+
client.browser_route_cache.set(route)
1558+
with pytest.raises(expected_error):
1559+
await client.browsers.fs.write_file("sess-1", cast(Any, payload()), path="/tmp/x")
1560+
1561+
assert client.browser_route_cache.get("sess-1") is not None
1562+
1563+
assert requests == [
1564+
(
1565+
httpx.URL("http://browser-session.test/browser/kernel/fs/write_file?path=%2Ftmp%2Fx&jwt=token-abc"),
1566+
b"payload",
1567+
)
1568+
]
1569+
1570+
14711571
def test_copied_client_registers_one_route_eviction_hook() -> None:
14721572
with Kernel(base_url=base_url, api_key=api_key, _strict_response_validation=True) as client:
14731573
copied = client.copy(api_key="sk-456")

0 commit comments

Comments
 (0)