From 16eb96a3b440199863505fbebcb6a261fc68e198 Mon Sep 17 00:00:00 2001 From: Jack Decker <24392469+jackowfish@users.noreply.github.com> Date: Wed, 15 Jul 2026 13:47:52 +0200 Subject: [PATCH] Exec runs unbounded by default and is never retried --- porter_sandbox/_async_base_client.py | 23 ++++++-- porter_sandbox/_base_client.py | 23 ++++++-- porter_sandbox/_errors.py | 8 +++ porter_sandbox/resources/sandboxes.py | 8 +-- tests/test_timeout.py | 75 +++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 tests/test_timeout.py diff --git a/porter_sandbox/_async_base_client.py b/porter_sandbox/_async_base_client.py index d975660..2ceb4e5 100644 --- a/porter_sandbox/_async_base_client.py +++ b/porter_sandbox/_async_base_client.py @@ -5,9 +5,10 @@ from typing import Any import httpx +from httpx._client import UseClientDefault from ._config import Config -from ._errors import SandboxError, error_for_status +from ._errors import SandboxError, SandboxTimeoutError, error_for_status from ._retries import DEFAULT_MAX_RETRIES, should_retry, sleep_for_attempt USER_AGENT = "porter-sandbox-python/0.0.1" @@ -75,20 +76,34 @@ async def _request( path: str, params: Mapping[str, Any] | None = None, json: Any = None, + timeout: float | None | UseClientDefault = httpx.USE_CLIENT_DEFAULT, + retry: bool = True, ) -> Any: + # `timeout=None` disables the timeout entirely, used for long-running + # calls like exec, where the API works for the full duration of the + # request. `retry=False` is for calls that must not be re-sent (exec): + # a failed attempt may have executed server-side, so retrying could + # run the command again. + max_retries = self._max_retries if retry else 0 last_error: Exception | None = None - for attempt in range(self._max_retries + 1): + for attempt in range(max_retries + 1): try: response = await self._http.request( method=method, url=path, params=params, json=json, + timeout=timeout, ) + except httpx.TimeoutException as exc: + # A timed-out request may have executed server-side, so + # retrying could run it again. Surface the timeout instead. + effective = self._config.timeout if timeout is httpx.USE_CLIENT_DEFAULT else timeout + raise SandboxTimeoutError(f"Request timed out after {effective}s") from exc except httpx.HTTPError as exc: last_error = exc - if attempt < self._max_retries: + if attempt < max_retries: await sleep_for_attempt(attempt) continue raise SandboxError(f"Network error: {exc}") from exc @@ -96,7 +111,7 @@ async def _request( if 200 <= response.status_code < 300: return _decode_body(response) - if should_retry(response.status_code) and attempt < self._max_retries: + if should_retry(response.status_code) and attempt < max_retries: await sleep_for_attempt(attempt) continue diff --git a/porter_sandbox/_base_client.py b/porter_sandbox/_base_client.py index 8a26182..9ba08f8 100644 --- a/porter_sandbox/_base_client.py +++ b/porter_sandbox/_base_client.py @@ -5,9 +5,10 @@ from typing import Any import httpx +from httpx._client import UseClientDefault from ._config import Config -from ._errors import SandboxError, error_for_status +from ._errors import SandboxError, SandboxTimeoutError, error_for_status from ._retries import DEFAULT_MAX_RETRIES, should_retry, sleep_for_attempt_sync USER_AGENT = "porter-sandbox-python/0.0.1" @@ -75,20 +76,34 @@ def _request( path: str, params: Mapping[str, Any] | None = None, json: Any = None, + timeout: float | None | UseClientDefault = httpx.USE_CLIENT_DEFAULT, + retry: bool = True, ) -> Any: + # `timeout=None` disables the timeout entirely, used for long-running + # calls like exec, where the API works for the full duration of the + # request. `retry=False` is for calls that must not be re-sent (exec): + # a failed attempt may have executed server-side, so retrying could + # run the command again. + max_retries = self._max_retries if retry else 0 last_error: Exception | None = None - for attempt in range(self._max_retries + 1): + for attempt in range(max_retries + 1): try: response = self._http.request( method=method, url=path, params=params, json=json, + timeout=timeout, ) + except httpx.TimeoutException as exc: + # A timed-out request may have executed server-side, so + # retrying could run it again. Surface the timeout instead. + effective = self._config.timeout if timeout is httpx.USE_CLIENT_DEFAULT else timeout + raise SandboxTimeoutError(f"Request timed out after {effective}s") from exc except httpx.HTTPError as exc: last_error = exc - if attempt < self._max_retries: + if attempt < max_retries: sleep_for_attempt_sync(attempt) continue raise SandboxError(f"Network error: {exc}") from exc @@ -96,7 +111,7 @@ def _request( if 200 <= response.status_code < 300: return _decode_body(response) - if should_retry(response.status_code) and attempt < self._max_retries: + if should_retry(response.status_code) and attempt < max_retries: sleep_for_attempt_sync(attempt) continue diff --git a/porter_sandbox/_errors.py b/porter_sandbox/_errors.py index cc00605..42000e3 100644 --- a/porter_sandbox/_errors.py +++ b/porter_sandbox/_errors.py @@ -30,6 +30,14 @@ class ServerError(SandboxError): """Raised on 5xx responses from the API.""" +class SandboxTimeoutError(SandboxError): + """Raised when a request exceeds its timeout. + + Never retried by the client: the server may still be executing the request + (e.g. a long-running exec), so a retry could run it again. + """ + + def error_for_status(status_code: int, body: object | None, message: str) -> SandboxError: """Map an HTTP status code to the appropriate SandboxError subclass.""" if status_code == 401: diff --git a/porter_sandbox/resources/sandboxes.py b/porter_sandbox/resources/sandboxes.py index 03cae7e..3c4a90d 100644 --- a/porter_sandbox/resources/sandboxes.py +++ b/porter_sandbox/resources/sandboxes.py @@ -146,14 +146,14 @@ def get_sandbox_logs(self, id: str, limit: int | None = None, since: str | None response = self._client._request(method="GET", path=path, params=params) return _coerce(LogsResponse, response) - def exec_sandbox(self, id: str, body: ExecRequest) -> ExecResponse: + def exec_sandbox(self, id: str, body: ExecRequest, timeout: float | None = None) -> ExecResponse: """ Exec in sandbox Run a command in a running sandbox and return its output and exit code """ path = f"/v1/sandbox/{id}/exec" - response = self._client._request(method="POST", path=path, json=body.model_dump(by_alias=True, exclude_none=True) if hasattr(body, "model_dump") else body) + response = self._client._request(method="POST", path=path, json=body.model_dump(by_alias=True, exclude_none=True) if hasattr(body, "model_dump") else body, timeout=timeout, retry=False) return _coerce(ExecResponse, response) @@ -273,12 +273,12 @@ async def get_sandbox_logs(self, id: str, limit: int | None = None, since: str | response = await self._client._request(method="GET", path=path, params=params) return _coerce(LogsResponse, response) - async def exec_sandbox(self, id: str, body: ExecRequest) -> ExecResponse: + async def exec_sandbox(self, id: str, body: ExecRequest, timeout: float | None = None) -> ExecResponse: """ Exec in sandbox Run a command in a running sandbox and return its output and exit code """ path = f"/v1/sandbox/{id}/exec" - response = await self._client._request(method="POST", path=path, json=body.model_dump(by_alias=True, exclude_none=True) if hasattr(body, "model_dump") else body) + response = await self._client._request(method="POST", path=path, json=body.model_dump(by_alias=True, exclude_none=True) if hasattr(body, "model_dump") else body, timeout=timeout, retry=False) return _coerce(ExecResponse, response) diff --git a/tests/test_timeout.py b/tests/test_timeout.py new file mode 100644 index 0000000..98c91cd --- /dev/null +++ b/tests/test_timeout.py @@ -0,0 +1,75 @@ +from __future__ import annotations + +import httpx +import pytest +import respx + +from porter_sandbox import Porter +from porter_sandbox._errors import SandboxError, SandboxTimeoutError +from porter_sandbox._models import ExecRequest + + +def test_exec_runs_without_client_timeout() -> None: + with respx.mock(base_url="https://sandbox.example") as mock: + mock.post("/v1/sandbox/run").respond(202, json={"id": "sb_123", "name": "sb_123"}) + exec_route = mock.post("/v1/sandbox/sb_123/exec").respond( + 200, json={"stdout": "hi", "stderr": "", "exit_code": 0} + ) + + porter = Porter(api_key="test", base_url="https://sandbox.example") + try: + sandbox = porter.sandboxes.create(image="python:3.11-alpine") + response = sandbox.exec(["echo", "hi"]) + assert response.exit_code == 0 + + request_timeout = exec_route.calls.last.request.extensions["timeout"] + assert request_timeout["read"] is None + finally: + porter.close() + + +def test_exec_accepts_explicit_timeout_via_raw_resource() -> None: + with respx.mock(base_url="https://sandbox.example") as mock: + exec_route = mock.post("/v1/sandbox/sb_123/exec").respond( + 200, json={"stdout": "hi", "stderr": "", "exit_code": 0} + ) + + porter = Porter(api_key="test", base_url="https://sandbox.example") + try: + porter.sandboxes.raw.exec_sandbox( + id="sb_123", body=ExecRequest(command=["echo", "hi"]), timeout=120 + ) + request_timeout = exec_route.calls.last.request.extensions["timeout"] + assert request_timeout["read"] == 120 + finally: + porter.close() + + +def test_timed_out_request_raises_without_retrying() -> None: + with respx.mock(base_url="https://sandbox.example") as mock: + route = mock.get("/v1/sandbox/sb_123") + route.side_effect = httpx.ReadTimeout("timed out") + + porter = Porter(api_key="test", base_url="https://sandbox.example") + try: + with pytest.raises(SandboxTimeoutError): + porter.sandboxes.raw.get_sandbox(id="sb_123") + assert route.call_count == 1 + finally: + porter.close() + + +def test_exec_network_error_is_not_retried() -> None: + with respx.mock(base_url="https://sandbox.example") as mock: + exec_route = mock.post("/v1/sandbox/sb_123/exec") + exec_route.side_effect = httpx.ConnectError("connection reset") + + porter = Porter(api_key="test", base_url="https://sandbox.example") + try: + with pytest.raises(SandboxError): + porter.sandboxes.raw.exec_sandbox( + id="sb_123", body=ExecRequest(command=["sleep", "60"]) + ) + assert exec_route.call_count == 1 + finally: + porter.close()