diff --git a/hyperbrowser/client/managers/async_manager/sandbox.py b/hyperbrowser/client/managers/async_manager/sandbox.py index 746eb83e..0dbb4427 100644 --- a/hyperbrowser/client/managers/async_manager/sandbox.py +++ b/hyperbrowser/client/managers/async_manager/sandbox.py @@ -57,8 +57,11 @@ from ....sandbox_common import ( RuntimeConnection, ensure_response_ok, + get_retry_delay_seconds, normalize_network_error, + request_context, parse_json_response, + should_retry_get, ) from ..sandboxes.shared import ( _build_sandbox_exposed_url, @@ -942,19 +945,31 @@ async def _request( params: Optional[Dict[str, object]] = None, data: Optional[Dict[str, object]] = None, ): - try: - response = await self._client.transport.client.request( - method, - self._client._build_url(path), - params={k: v for k, v in (params or {}).items() if v is not None}, - json=data, - ) - except BaseException as error: - raise normalize_network_error( - error, - "control", - "Unknown error occurred", - ) + failed_attempt = 1 + while True: + try: + response = await self._client.transport.client.request( + method, + self._client._build_url(path), + params={ + key: value + for key, value in (params or {}).items() + if value is not None + }, + json=data, + ) + ensure_response_ok(response, "control") + except BaseException as cause: + error = normalize_network_error( + cause, + "control", + "Unknown error occurred", + request_context(method, path), + ) + if not should_retry_get(method, error, failed_attempt): + raise error + await asyncio.sleep(get_retry_delay_seconds(failed_attempt)) + failed_attempt += 1 + continue - ensure_response_ok(response, "control") - return parse_json_response(response, "control") + return parse_json_response(response, "control") diff --git a/hyperbrowser/client/managers/async_manager/sandboxes/sandbox_transport.py b/hyperbrowser/client/managers/async_manager/sandboxes/sandbox_transport.py index d2b10ad3..96e96ee1 100644 --- a/hyperbrowser/client/managers/async_manager/sandboxes/sandbox_transport.py +++ b/hyperbrowser/client/managers/async_manager/sandboxes/sandbox_transport.py @@ -9,6 +9,7 @@ ensure_response_ok, normalize_network_error, parse_json_response, + request_context, resolve_runtime_transport_target, ) from ...sandboxes.shared import _build_query_path, _is_replayable_http_content @@ -275,6 +276,7 @@ async def _send( error, "runtime", "Unknown runtime request error", + request_context(method, path), ) await response.aread() @@ -309,6 +311,7 @@ async def _send_binary_stream( error, "runtime", "Unknown runtime request error", + request_context(method, path), ) async def _send_stream( @@ -341,4 +344,5 @@ async def _send_stream( error, "runtime", "Unknown runtime request error", + request_context("GET", path), ) diff --git a/hyperbrowser/client/managers/sync_manager/sandbox.py b/hyperbrowser/client/managers/sync_manager/sandbox.py index 17771a7f..bde5000e 100644 --- a/hyperbrowser/client/managers/sync_manager/sandbox.py +++ b/hyperbrowser/client/managers/sync_manager/sandbox.py @@ -55,8 +55,11 @@ from ....sandbox_common import ( RuntimeConnection, ensure_response_ok, + get_retry_delay_seconds, normalize_network_error, + request_context, parse_json_response, + should_retry_get, ) from ..sandboxes.shared import ( _build_sandbox_exposed_url, @@ -925,19 +928,31 @@ def _request( params: Optional[Dict[str, object]] = None, data: Optional[Dict[str, object]] = None, ): - try: - response = self._client.transport.client.request( - method, - self._client._build_url(path), - params={k: v for k, v in (params or {}).items() if v is not None}, - json=data, - ) - except BaseException as error: - raise normalize_network_error( - error, - "control", - "Unknown error occurred", - ) + failed_attempt = 1 + while True: + try: + response = self._client.transport.client.request( + method, + self._client._build_url(path), + params={ + key: value + for key, value in (params or {}).items() + if value is not None + }, + json=data, + ) + ensure_response_ok(response, "control") + except BaseException as cause: + error = normalize_network_error( + cause, + "control", + "Unknown error occurred", + request_context(method, path), + ) + if not should_retry_get(method, error, failed_attempt): + raise error + time.sleep(get_retry_delay_seconds(failed_attempt)) + failed_attempt += 1 + continue - ensure_response_ok(response, "control") - return parse_json_response(response, "control") + return parse_json_response(response, "control") diff --git a/hyperbrowser/client/managers/sync_manager/sandboxes/sandbox_transport.py b/hyperbrowser/client/managers/sync_manager/sandboxes/sandbox_transport.py index 1204b819..74573e50 100644 --- a/hyperbrowser/client/managers/sync_manager/sandboxes/sandbox_transport.py +++ b/hyperbrowser/client/managers/sync_manager/sandboxes/sandbox_transport.py @@ -9,6 +9,7 @@ ensure_response_ok, normalize_network_error, parse_json_response, + request_context, resolve_runtime_transport_target, ) from ...sandboxes.shared import _build_query_path, _is_replayable_http_content @@ -273,6 +274,7 @@ def _send( error, "runtime", "Unknown runtime request error", + request_context(method, path), ) response.read() @@ -307,6 +309,7 @@ def _send_binary_stream( error, "runtime", "Unknown runtime request error", + request_context(method, path), ) def _send_stream( @@ -339,4 +342,5 @@ def _send_stream( error, "runtime", "Unknown runtime request error", + request_context("GET", path), ) diff --git a/hyperbrowser/sandbox_common.py b/hyperbrowser/sandbox_common.py index a05b1e2a..e17310e4 100644 --- a/hyperbrowser/sandbox_common.py +++ b/hyperbrowser/sandbox_common.py @@ -1,4 +1,5 @@ import json +import random from dataclasses import dataclass from typing import Any, Dict, Optional, Tuple from urllib.parse import urljoin, urlsplit, urlunsplit @@ -8,6 +9,9 @@ from .exceptions import HyperbrowserError, HyperbrowserService RETRYABLE_STATUS_CODES = {429, 502, 503, 504} +GET_RETRY_MAX_ATTEMPTS = 3 +GET_RETRY_INITIAL_DELAY_SECONDS = 0.25 +GET_RETRY_MAX_DELAY_SECONDS = 1.0 RUNTIME_SESSION_REFRESH_BUFFER_MS = 60_000 @@ -45,6 +49,26 @@ def is_retryable_network_error(error: BaseException) -> bool: ) +def should_retry_get( + method: str, + error: HyperbrowserError, + failed_attempt: int, +) -> bool: + return ( + method.upper() == "GET" + and error.retryable + and failed_attempt < GET_RETRY_MAX_ATTEMPTS + ) + + +def get_retry_delay_seconds(failed_attempt: int) -> float: + maximum_delay = min( + GET_RETRY_INITIAL_DELAY_SECONDS * (2 ** (failed_attempt - 1)), + GET_RETRY_MAX_DELAY_SECONDS, + ) + return random.uniform(maximum_delay / 2, maximum_delay) + + def parse_error_payload( raw_text: str, fallback_message: str ) -> Tuple[str, Optional[str], Any]: @@ -230,16 +254,55 @@ def to_websocket_transport_target( ) +def request_context(method: Optional[str], path_or_url: Optional[str]) -> str: + """Render "[POST /sandbox]" for error messages. + + Accepts either a bare path or a full URL. The query string is dropped so + request parameters never reach error text or logs. + """ + normalized_method = (method or "").strip().upper() + raw_target = (path_or_url or "").strip() + normalized_path = urlsplit(raw_target).path or raw_target.split("?", 1)[0] + if normalized_method and normalized_path: + return f"[{normalized_method} {normalized_path}]" + if normalized_path: + return f"[{normalized_path}]" + return f"[{normalized_method}]" if normalized_method else "" + + +def describe_network_error( + error: BaseException, + default_message: str, + context: str = "", +) -> str: + suffix = f" {context}" if context else "" + detail = str(error).strip() + if detail: + return f"{detail}{suffix}" + # Several httpx transport exceptions are raised with no arguments, so str() + # is empty and the caller's fallback alone would not say which one failed. + name = type(error).__name__ + base = f"{default_message} ({name})" if default_message else name + return f"{base}{suffix}" + + def normalize_network_error( error: BaseException, service: HyperbrowserService, default_message: str, + context: str = "", ) -> HyperbrowserError: if isinstance(error, HyperbrowserError): return error + if not isinstance(error, Exception): + # CancelledError, KeyboardInterrupt and SystemExit are control flow, not + # transport failures. Callers catch BaseException around their requests, + # so wrapping these would strand the cancellation and report a request + # the caller itself abandoned as a network error. + raise error return HyperbrowserError( - str(error) if str(error) else default_message, + describe_network_error(error, default_message, context), retryable=is_retryable_network_error(error), service=service, cause=error, diff --git a/hyperbrowser/transport/async_transport.py b/hyperbrowser/transport/async_transport.py index 8bf40338..10f3474a 100644 --- a/hyperbrowser/transport/async_transport.py +++ b/hyperbrowser/transport/async_transport.py @@ -3,6 +3,15 @@ from typing import Optional from hyperbrowser.exceptions import HyperbrowserError +from hyperbrowser.sandbox_common import ( + RETRYABLE_STATUS_CODES, + get_request_id, + get_retry_delay_seconds, + is_retryable_network_error, + normalize_network_error, + request_context, + should_retry_get, +) from .base import TransportStrategy, APIResponse @@ -49,6 +58,9 @@ async def _handle_response(self, response: httpx.Response) -> APIResponse: status_code=response.status_code, response=response, original_error=e, + request_id=get_request_id(response), + retryable=response.status_code in RETRYABLE_STATUS_CODES, + service="control", ) return APIResponse.from_status(response.status_code) except httpx.HTTPStatusError as e: @@ -62,9 +74,17 @@ async def _handle_response(self, response: httpx.Response) -> APIResponse: status_code=response.status_code, response=response, original_error=e, + request_id=get_request_id(response), + retryable=response.status_code in RETRYABLE_STATUS_CODES, + service="control", ) except httpx.RequestError as e: - raise HyperbrowserError("Request failed", original_error=e) + raise HyperbrowserError( + "Request failed", + original_error=e, + retryable=is_retryable_network_error(e), + service="control", + ) async def post( self, @@ -82,25 +102,34 @@ async def post( else: response = await self.client.post(url, json=data, **kwargs) return await self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Post request failed", original_error=e) + except BaseException as e: + raise normalize_network_error( + e, "control", "Post request failed", request_context("POST", url) + ) async def get( self, url: str, params: Optional[dict] = None, follow_redirects: bool = False ) -> APIResponse: if params: params = {k: v for k, v in params.items() if v is not None} - try: - response = await self.client.get( - url, params=params, follow_redirects=follow_redirects - ) - return await self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Get request failed", original_error=e) + failed_attempt = 1 + while True: + try: + response = await self.client.get( + url, params=params, follow_redirects=follow_redirects + ) + return await self._handle_response(response) + except BaseException as cause: + error = normalize_network_error( + cause, + "control", + "Get request failed", + request_context("GET", url), + ) + if not should_retry_get("GET", error, failed_attempt): + raise error + await asyncio.sleep(get_retry_delay_seconds(failed_attempt)) + failed_attempt += 1 async def put(self, url: str, data: Optional[dict] = None) -> APIResponse: try: diff --git a/hyperbrowser/transport/sync.py b/hyperbrowser/transport/sync.py index b4af6e7f..29067c14 100644 --- a/hyperbrowser/transport/sync.py +++ b/hyperbrowser/transport/sync.py @@ -1,7 +1,18 @@ +import time + import httpx from typing import Optional from hyperbrowser.exceptions import HyperbrowserError +from hyperbrowser.sandbox_common import ( + RETRYABLE_STATUS_CODES, + get_request_id, + get_retry_delay_seconds, + is_retryable_network_error, + normalize_network_error, + request_context, + should_retry_get, +) from .base import TransportStrategy, APIResponse @@ -25,6 +36,9 @@ def _handle_response(self, response: httpx.Response) -> APIResponse: status_code=response.status_code, response=response, original_error=e, + request_id=get_request_id(response), + retryable=response.status_code in RETRYABLE_STATUS_CODES, + service="control", ) return APIResponse.from_status(response.status_code) except httpx.HTTPStatusError as e: @@ -38,9 +52,17 @@ def _handle_response(self, response: httpx.Response) -> APIResponse: status_code=response.status_code, response=response, original_error=e, + request_id=get_request_id(response), + retryable=response.status_code in RETRYABLE_STATUS_CODES, + service="control", ) except httpx.RequestError as e: - raise HyperbrowserError("Request failed", original_error=e) + raise HyperbrowserError( + "Request failed", + original_error=e, + retryable=is_retryable_network_error(e), + service="control", + ) def close(self) -> None: self.client.close() @@ -61,25 +83,34 @@ def post( else: response = self.client.post(url, json=data, **kwargs) return self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Post request failed", original_error=e) + except BaseException as e: + raise normalize_network_error( + e, "control", "Post request failed", request_context("POST", url) + ) def get( self, url: str, params: Optional[dict] = None, follow_redirects: bool = False ) -> APIResponse: if params: params = {k: v for k, v in params.items() if v is not None} - try: - response = self.client.get( - url, params=params, follow_redirects=follow_redirects - ) - return self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Get request failed", original_error=e) + failed_attempt = 1 + while True: + try: + response = self.client.get( + url, params=params, follow_redirects=follow_redirects + ) + return self._handle_response(response) + except BaseException as cause: + error = normalize_network_error( + cause, + "control", + "Get request failed", + request_context("GET", url), + ) + if not should_retry_get("GET", error, failed_attempt): + raise error + time.sleep(get_retry_delay_seconds(failed_attempt)) + failed_attempt += 1 def put(self, url: str, data: Optional[dict] = None) -> APIResponse: try: diff --git a/tests/test_control_get_retries.py b/tests/test_control_get_retries.py new file mode 100644 index 00000000..20f98b9c --- /dev/null +++ b/tests/test_control_get_retries.py @@ -0,0 +1,216 @@ +from types import SimpleNamespace + +import httpx +import pytest + +import hyperbrowser.client.managers.async_manager.sandbox as async_sandbox_module +import hyperbrowser.client.managers.sync_manager.sandbox as sync_sandbox_module +import hyperbrowser.transport.async_transport as async_transport_module +import hyperbrowser.transport.sync as sync_transport_module +from hyperbrowser.client.managers.async_manager.sandbox import ( + SandboxManager as AsyncSandboxManager, +) +from hyperbrowser.client.managers.sync_manager.sandbox import SandboxManager +from hyperbrowser.exceptions import HyperbrowserError +from hyperbrowser.transport.async_transport import AsyncTransport +from hyperbrowser.transport.sync import SyncTransport + + +def make_response(status_code, payload=None, text=None): + request = httpx.Request("GET", "https://api.hyperbrowser.ai/api/test") + if payload is not None: + return httpx.Response(status_code, json=payload, request=request) + return httpx.Response(status_code, text=text or "", request=request) + + +class SequencedSyncClient: + def __init__(self, outcomes): + self.outcomes = list(outcomes) + self.calls = [] + + def _next(self, method): + self.calls.append(method) + outcome = self.outcomes.pop(0) + if isinstance(outcome, BaseException): + raise outcome + return outcome + + def get(self, url, **kwargs): + return self._next("GET") + + def post(self, url, **kwargs): + return self._next("POST") + + def request(self, method, url, **kwargs): + return self._next(method) + + +class SequencedAsyncClient: + def __init__(self, outcomes): + self.outcomes = list(outcomes) + self.calls = [] + + def _next(self, method): + self.calls.append(method) + outcome = self.outcomes.pop(0) + if isinstance(outcome, BaseException): + raise outcome + return outcome + + async def get(self, url, **kwargs): + return self._next("GET") + + async def post(self, url, **kwargs): + return self._next("POST") + + async def request(self, method, url, **kwargs): + return self._next(method) + + +def make_sync_transport(client): + transport = object.__new__(SyncTransport) + transport.client = client + return transport + + +def make_async_transport(client): + transport = object.__new__(AsyncTransport) + transport.client = client + transport._closed = True + return transport + + +def make_sync_sandbox_manager(client): + manager = object.__new__(SandboxManager) + manager._client = SimpleNamespace( + transport=SimpleNamespace(client=client), + _build_url=lambda path: f"https://api.hyperbrowser.ai/api{path}", + ) + return manager + + +def make_async_sandbox_manager(client): + manager = object.__new__(AsyncSandboxManager) + manager._client = SimpleNamespace( + transport=SimpleNamespace(client=client), + _build_url=lambda path: f"https://api.hyperbrowser.ai/api{path}", + ) + return manager + + +def test_sync_transport_get_retries_transient_status(monkeypatch): + client = SequencedSyncClient( + [make_response(502, text="Bad Gateway"), make_response(200, {"ok": True})] + ) + delays = [] + monkeypatch.setattr(sync_transport_module.time, "sleep", delays.append) + + result = make_sync_transport(client).get("https://example.test") + + assert result.data == {"ok": True} + assert client.calls == ["GET", "GET"] + assert len(delays) == 1 + + +def test_sync_transport_get_stops_after_three_attempts(monkeypatch): + client = SequencedSyncClient([make_response(503)] * 3) + monkeypatch.setattr(sync_transport_module.time, "sleep", lambda _: None) + + with pytest.raises(HyperbrowserError) as exc_info: + make_sync_transport(client).get("https://example.test") + + assert exc_info.value.status_code == 503 + assert exc_info.value.retryable is True + assert client.calls == ["GET", "GET", "GET"] + + +def test_sync_transport_post_does_not_retry(monkeypatch): + client = SequencedSyncClient( + [make_response(502, text="Bad Gateway"), make_response(200, {"ok": True})] + ) + delays = [] + monkeypatch.setattr(sync_transport_module.time, "sleep", delays.append) + + with pytest.raises(HyperbrowserError) as exc_info: + make_sync_transport(client).post("https://example.test", {"value": 1}) + + assert exc_info.value.status_code == 502 + assert exc_info.value.retryable is True + assert client.calls == ["POST"] + assert delays == [] + + +@pytest.mark.anyio +async def test_async_transport_get_retries_transient_network_error(monkeypatch): + request = httpx.Request("GET", "https://example.test") + client = SequencedAsyncClient( + [httpx.ReadTimeout("", request=request), make_response(200, {"ok": True})] + ) + delays = [] + + async def record_sleep(delay): + delays.append(delay) + + monkeypatch.setattr(async_transport_module.asyncio, "sleep", record_sleep) + + result = await make_async_transport(client).get("https://example.test") + + assert result.data == {"ok": True} + assert client.calls == ["GET", "GET"] + assert len(delays) == 1 + + +def test_sync_sandbox_get_retries_transient_status(monkeypatch): + client = SequencedSyncClient( + [make_response(502, text="Bad Gateway"), make_response(200, {"ok": True})] + ) + delays = [] + monkeypatch.setattr(sync_sandbox_module.time, "sleep", delays.append) + + result = make_sync_sandbox_manager(client)._request("GET", "/sandbox/test") + + assert result == {"ok": True} + assert client.calls == ["GET", "GET"] + assert len(delays) == 1 + + +@pytest.mark.anyio +async def test_async_sandbox_get_retries_transient_status(monkeypatch): + client = SequencedAsyncClient( + [make_response(504, text="Gateway Timeout"), make_response(200, {"ok": True})] + ) + delays = [] + + async def record_sleep(delay): + delays.append(delay) + + monkeypatch.setattr(async_sandbox_module.asyncio, "sleep", record_sleep) + + result = await make_async_sandbox_manager(client)._request("GET", "/sandbox/test") + + assert result == {"ok": True} + assert client.calls == ["GET", "GET"] + assert len(delays) == 1 + + +@pytest.mark.anyio +async def test_async_sandbox_post_does_not_retry(monkeypatch): + client = SequencedAsyncClient( + [make_response(502, text="Bad Gateway"), make_response(200, {"ok": True})] + ) + delays = [] + + async def record_sleep(delay): + delays.append(delay) + + monkeypatch.setattr(async_sandbox_module.asyncio, "sleep", record_sleep) + + with pytest.raises(HyperbrowserError) as exc_info: + await make_async_sandbox_manager(client)._request( + "POST", "/sandbox", data={"image": "test"} + ) + + assert exc_info.value.status_code == 502 + assert exc_info.value.retryable is True + assert client.calls == ["POST"] + assert delays == [] diff --git a/tests/test_network_error_normalization.py b/tests/test_network_error_normalization.py new file mode 100644 index 00000000..f955b792 --- /dev/null +++ b/tests/test_network_error_normalization.py @@ -0,0 +1,70 @@ +import asyncio + +import httpx +import pytest + +from hyperbrowser.exceptions import HyperbrowserError +from hyperbrowser.sandbox_common import normalize_network_error, request_context + + +@pytest.mark.parametrize( + "error", + [ + asyncio.CancelledError(), + KeyboardInterrupt(), + SystemExit(), + ], +) +def test_control_flow_exceptions_propagate_instead_of_becoming_network_errors(error): + with pytest.raises(type(error)) as exc_info: + normalize_network_error(error, "control", "Unknown error occurred") + + assert exc_info.value is error + + +def test_transport_error_without_a_message_reports_its_type(): + error = normalize_network_error( + httpx.ReadTimeout(""), + "control", + "Unknown error occurred", + ) + + assert isinstance(error, HyperbrowserError) + assert str(error) == "Unknown error occurred (ReadTimeout)" + assert error.retryable is True + + +def test_transport_error_with_a_message_keeps_its_detail(): + error = normalize_network_error( + httpx.ConnectError("connection refused"), + "control", + "Unknown error occurred", + ) + + assert str(error) == "connection refused" + + +def test_request_context_is_appended_so_the_failing_call_is_identifiable(): + error = normalize_network_error( + httpx.ReadTimeout(""), + "control", + "Unknown error occurred", + request_context("post", "/sandbox"), + ) + + assert str(error) == "Unknown error occurred (ReadTimeout) [POST /sandbox]" + + +@pytest.mark.parametrize( + ("method", "target", "expected"), + [ + ("GET", "/images/builds/abc", "[GET /images/builds/abc]"), + ("GET", "https://api.hyperbrowser.ai/sandbox?token=secret", "[GET /sandbox]"), + ("GET", "/sandbox?token=secret", "[GET /sandbox]"), + ("", "/sandbox", "[/sandbox]"), + ("GET", "", "[GET]"), + ("", "", ""), + ], +) +def test_request_context_drops_query_strings_and_hosts(method, target, expected): + assert request_context(method, target) == expected