diff --git a/actions/__init__.py b/actions/__init__.py index 4317e09..733246b 100644 --- a/actions/__init__.py +++ b/actions/__init__.py @@ -28,4 +28,4 @@ # ├── test_summarize_pr.py # └── ... -__version__ = "0.3.23" +__version__ = "0.3.24" diff --git a/actions/utils/openai_utils.py b/actions/utils/openai_utils.py index 66bd851..c92ac36 100644 --- a/actions/utils/openai_utils.py +++ b/actions/utils/openai_utils.py @@ -327,9 +327,8 @@ def _post_openai_response( return ( _poll_openai_response(response_json, headers) if response_json.get("status") else response_json ), elapsed - except (requests.exceptions.ConnectionError, json.JSONDecodeError): - # ConnectTimeout subclasses ConnectionError so it stays retryable; a ReadTimeout propagates instead, - # because the request may have completed server-side and re-POSTing it would double-bill. + except (requests.exceptions.ConnectionError, requests.exceptions.Timeout, json.JSONDecodeError): + # Retry TLS/read timeouts too, accepting that a response timeout may repeat an already-billed request. if attempt < retries: print(f"Retrying API request in {2**attempt}s (attempt {attempt + 1}/{retries + 1})...") time.sleep(2**attempt) @@ -666,9 +665,8 @@ def get_response( return content - except (requests.exceptions.ConnectionError, json.JSONDecodeError) as e: - # ConnectTimeout subclasses ConnectionError so it stays retryable; a ReadTimeout propagates instead, - # because the request may have completed server-side and re-POSTing it would double-bill. + except (requests.exceptions.ConnectionError, requests.exceptions.Timeout, json.JSONDecodeError) as e: + # Retry TLS/read timeouts too, accepting that a response timeout may repeat an already-billed request. if attempt < retries: print(f"Retrying {e.__class__.__name__} in {2**attempt}s (attempt {attempt + 1}/{retries + 1})...") time.sleep(2**attempt) diff --git a/tests/test_openai_utils.py b/tests/test_openai_utils.py index 6a52065..e4d2999 100644 --- a/tests/test_openai_utils.py +++ b/tests/test_openai_utils.py @@ -1,7 +1,7 @@ # Ultralytics 🚀 AGPL-3.0 License - https://ultralytics.com/license import json -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch import pytest import requests @@ -149,19 +149,31 @@ def test_get_response(mock_post): assert mock_post.call_args.kwargs["json"]["prompt_cache_options"] == {"mode": "explicit"} +@pytest.mark.parametrize("agent", [False, True]) +@pytest.mark.parametrize("failures", [1, 3]) @patch("time.sleep") @patch("requests.post") -def test_get_response_read_timeout_propagates(mock_post, mock_sleep): - """Test a read timeout is NOT retried: the request may have completed server-side and re-POSTing double-bills.""" - mock_post.side_effect = requests.exceptions.ReadTimeout() - +def test_response_read_timeout_retries(mock_post, mock_sleep, agent, failures): + """Read timeouts recover with bounded backoff in both completion and agent requests.""" + response = MagicMock(status_code=200) + response.elapsed.total_seconds.return_value = 1.0 + response.json.return_value = { + "output": [{"type": "message", "content": [{"type": "output_text", "text": "recovered"}]}] + } + mock_post.side_effect = [requests.exceptions.ReadTimeout()] * failures + [response] + kwargs = {"tools": [], "tool_handlers": {}} if agent else {"check_links": False} + generate = get_agent_response if agent else get_response with patch("actions.utils.openai_utils.OPENAI_API_KEY", "test-key"): - try: - get_response([{"role": "user", "content": "Hello"}], check_links=False, retries=2) - raise AssertionError("ReadTimeout should propagate") - except requests.exceptions.ReadTimeout: - pass - assert mock_post.call_count == 1 # no re-POST of a possibly-billed request + if failures > 2: + with pytest.raises(requests.exceptions.ReadTimeout): + generate([{"role": "user", "content": "Hello"}], model=OPENAI_MODEL_DEFAULT, retries=2, **kwargs) + else: + assert ( + generate([{"role": "user", "content": "Hello"}], model=OPENAI_MODEL_DEFAULT, retries=2, **kwargs) + == "recovered" + ) + assert mock_post.call_count == min(failures + 1, 3) + assert mock_sleep.call_args_list == [call(2**attempt) for attempt in range(min(failures, 2))] @patch("time.sleep")