mirror of
https://github.com/usestrix/strix.git
synced 2026-11-14 09:01:25 +00:00
Merge 8598f6a735 into f6d9790ecb
This commit is contained in:
commit
070f7933e9
7 changed files with 144 additions and 34 deletions
|
|
@ -160,6 +160,21 @@ def is_content_guardrail_error(exc: BaseException) -> bool:
|
|||
return any(marker in text for marker in _GUARDRAIL_MARKERS)
|
||||
|
||||
|
||||
_USAGE_LIMIT_MARKERS = (
|
||||
"usage_limit_reached",
|
||||
"usage limit has been reached",
|
||||
)
|
||||
|
||||
|
||||
def is_usage_limit_error(exc: BaseException) -> bool:
|
||||
"""A plan/subscription usage window is exhausted (e.g. ChatGPT's 5-hour or
|
||||
weekly cap), not an ordinary throttle. Terminal for the run: retrying only
|
||||
burns more of the same exhausted quota and cannot succeed until it resets,
|
||||
so the run should stop resumably instead of thrashing the backoff."""
|
||||
text = str(exc).lower()
|
||||
return any(marker in text for marker in _USAGE_LIMIT_MARKERS)
|
||||
|
||||
|
||||
def _b64url(raw: bytes) -> str:
|
||||
return base64.urlsafe_b64encode(raw).rstrip(b"=").decode("ascii")
|
||||
|
||||
|
|
|
|||
|
|
@ -545,6 +545,15 @@ class StrixProvider(MultiProvider):
|
|||
)
|
||||
|
||||
|
||||
def _not_usage_limit_error(context: RetryPolicyContext) -> bool:
|
||||
# A usage-limit rejection (a plan/subscription window is exhausted, e.g.
|
||||
# ChatGPT's 5-hour or weekly cap) is terminal for the run: retrying only
|
||||
# burns more of the same exhausted quota and cannot succeed until it resets.
|
||||
# Composed with ``all`` below so the run fails fast and stops resumably
|
||||
# instead of thrashing the backoff on a limit that will not clear for hours.
|
||||
return not codex.is_usage_limit_error(context.error)
|
||||
|
||||
|
||||
DEFAULT_MODEL_RETRY = ModelRetrySettings(
|
||||
max_retries=5,
|
||||
backoff=ModelRetryBackoffSettings(
|
||||
|
|
@ -553,11 +562,14 @@ DEFAULT_MODEL_RETRY = ModelRetrySettings(
|
|||
multiplier=2.0,
|
||||
jitter=False,
|
||||
),
|
||||
policy=retry_policies.any(
|
||||
retry_policies.provider_suggested(),
|
||||
retry_policies.network_error(),
|
||||
retry_policies.http_status((429, 500, 502, 503, 504)),
|
||||
_retry_statusless_provider_errors,
|
||||
policy=retry_policies.all(
|
||||
retry_policies.any(
|
||||
retry_policies.provider_suggested(),
|
||||
retry_policies.network_error(),
|
||||
retry_policies.http_status((429, 500, 502, 503, 504)),
|
||||
_retry_statusless_provider_errors,
|
||||
),
|
||||
_not_usage_limit_error,
|
||||
),
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -132,6 +132,8 @@ def _model_error_status_code(exc: BaseException) -> int | None:
|
|||
def _is_transient_model_error(exc: BaseException) -> bool:
|
||||
if codex.is_content_guardrail_error(exc):
|
||||
return False
|
||||
if codex.is_usage_limit_error(exc):
|
||||
return False
|
||||
if isinstance(
|
||||
exc, APITimeoutError | APIConnectionError | TimeoutError | ConnectionError | OSError
|
||||
):
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ from openai import RateLimitError
|
|||
|
||||
from strix.agents.factory import build_strix_agent, make_child_factory
|
||||
from strix.agents.prompt import render_system_prompt
|
||||
from strix.config import load_settings
|
||||
from strix.config import codex, load_settings
|
||||
from strix.config.models import (
|
||||
StrixProvider,
|
||||
configure_sdk_model_defaults,
|
||||
|
|
@ -614,24 +614,36 @@ async def run_strix_scan(
|
|||
with contextlib.suppress(Exception):
|
||||
await coordinator.set_status(root_id, "stopped")
|
||||
return None
|
||||
except RateLimitError as exc:
|
||||
logger.warning(
|
||||
"Scan %s stopped: persistent rate limit from the LLM provider (%s). "
|
||||
"Resume with 'strix --resume %s' once the limit clears.",
|
||||
scan_id,
|
||||
exc,
|
||||
scan_id,
|
||||
)
|
||||
if root_id is not None:
|
||||
with contextlib.suppress(Exception):
|
||||
await coordinator.set_status(root_id, "stopped")
|
||||
return None
|
||||
except (asyncio.CancelledError, KeyboardInterrupt):
|
||||
logger.info("Scan %s interrupted by the user", scan_id)
|
||||
if root_id is not None:
|
||||
with contextlib.suppress(Exception):
|
||||
await coordinator.set_status(root_id, "running")
|
||||
raise
|
||||
except Exception as exc:
|
||||
# A usage-limit rejection is resumable regardless of the provider's
|
||||
# exception class: OpenAI raises RateLimitError, but a LiteLLM-routed
|
||||
# provider may surface the same exhausted-window error as a different
|
||||
# type. Both retry layers already stop retrying it (see
|
||||
# codex.is_usage_limit_error), so route it to the same resumable stop
|
||||
# instead of the generic failure path.
|
||||
if isinstance(exc, RateLimitError) or codex.is_usage_limit_error(exc):
|
||||
logger.warning(
|
||||
"Scan %s stopped: persistent rate limit from the LLM provider (%s). "
|
||||
"Resume with 'strix --resume %s' once the limit clears.",
|
||||
scan_id,
|
||||
exc,
|
||||
scan_id,
|
||||
)
|
||||
if root_id is not None:
|
||||
with contextlib.suppress(Exception):
|
||||
await coordinator.set_status(root_id, "stopped")
|
||||
return None
|
||||
logger.exception("Strix scan %s failed", scan_id)
|
||||
if root_id is not None:
|
||||
with contextlib.suppress(Exception):
|
||||
await coordinator.set_status(root_id, "failed")
|
||||
raise
|
||||
except BaseException:
|
||||
logger.exception("Strix scan %s failed", scan_id)
|
||||
if root_id is not None:
|
||||
|
|
|
|||
|
|
@ -172,3 +172,23 @@ async def test_run_cycle_does_not_retry_permanent_error(
|
|||
streams = [_FakeStream(exc=bad_request), _FakeStream()]
|
||||
with pytest.raises(BadRequestError):
|
||||
await _run_once(monkeypatch, streams)
|
||||
|
||||
|
||||
def test_usage_limit_error_is_terminal_not_transient() -> None:
|
||||
usage_limited = RateLimitError(
|
||||
"Error code: 429 - {'error': {'type': 'usage_limit_reached', "
|
||||
"'message': 'The usage limit has been reached'}}",
|
||||
response=httpx.Response(429, request=_request()),
|
||||
body={"type": "usage_limit_reached"},
|
||||
)
|
||||
assert codex.is_usage_limit_error(usage_limited) is True
|
||||
# A usage-limit rejection must not be retried like an ordinary throttle.
|
||||
assert execution._is_transient_model_error(usage_limited) is False
|
||||
|
||||
|
||||
def test_ordinary_rate_limit_is_not_usage_limit() -> None:
|
||||
rate_limited = RateLimitError(
|
||||
"slow down", response=httpx.Response(429, request=_request()), body=None
|
||||
)
|
||||
assert codex.is_usage_limit_error(rate_limited) is False
|
||||
assert execution._is_transient_model_error(rate_limited) is True
|
||||
|
|
|
|||
|
|
@ -88,3 +88,12 @@ def test_policy_helper_matches_statusless_only() -> None:
|
|||
)
|
||||
is False
|
||||
)
|
||||
|
||||
|
||||
def test_usage_limit_error_is_not_retried() -> None:
|
||||
# A usage-limit rejection carries a 429, but the window will not clear for
|
||||
# hours; retrying only burns more of the same quota, so it must be terminal
|
||||
# even though an ordinary 429 is retried.
|
||||
usage_limited = RuntimeError("Error code: 429 - {'error': {'type': 'usage_limit_reached'}}")
|
||||
assert codex.is_usage_limit_error(usage_limited) is True
|
||||
assert _retries(ModelRetryNormalizedError(status_code=429), error=usage_limited) is False
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
"""Tests for graceful handling of persistent RateLimitError in run_strix_scan."""
|
||||
"""Tests for graceful handling of persistent usage/rate-limit errors in run_strix_scan."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
|
|
@ -24,11 +24,22 @@ def _make_rate_limit_error() -> RateLimitError:
|
|||
return RateLimitError("rate limited", response=response, body=None)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persistent_rate_limit_stops_gracefully(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Any, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A persistent RateLimitError stops the scan (root -> 'stopped') without raising."""
|
||||
class _ProviderUsageLimitError(Exception):
|
||||
"""A non-OpenAI (e.g. LiteLLM-routed) provider exception whose text carries the
|
||||
usage-limit marker but which is *not* an ``openai.RateLimitError``."""
|
||||
|
||||
|
||||
def _make_litellm_usage_limit_error() -> Exception:
|
||||
return _ProviderUsageLimitError(
|
||||
"litellm.RateLimitError: 429 - {'error': {'type': 'usage_limit_reached', "
|
||||
"'message': 'The usage limit has been reached'}}"
|
||||
)
|
||||
|
||||
|
||||
async def _run_scan_raising(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Any, error: BaseException
|
||||
) -> tuple[Any, AgentCoordinator]:
|
||||
"""Drive run_strix_scan with a fully mocked scan whose agent loop raises ``error``."""
|
||||
monkeypatch.setattr(runner, "run_dir_for", lambda _scan_id: tmp_path)
|
||||
monkeypatch.setattr(runner, "runtime_state_dir", lambda _run_dir: tmp_path)
|
||||
monkeypatch.setattr(runner, "setup_scan_logging", lambda _run_dir: lambda: None)
|
||||
|
|
@ -70,21 +81,24 @@ async def test_persistent_rate_limit_stops_gracefully(
|
|||
monkeypatch.setattr(runner, "make_child_factory", lambda **_kwargs: lambda **_k: object())
|
||||
monkeypatch.setattr(runner, "open_agent_session", lambda _root_id, _db: object())
|
||||
|
||||
async def _raise_rate_limit(*_args: Any, **_kwargs: Any) -> None:
|
||||
raise _make_rate_limit_error()
|
||||
async def _raise(*_args: Any, **_kwargs: Any) -> None:
|
||||
raise error
|
||||
|
||||
monkeypatch.setattr(runner, "run_agent_loop", _raise_rate_limit)
|
||||
monkeypatch.setattr(runner, "run_agent_loop", _raise)
|
||||
|
||||
coordinator = AgentCoordinator()
|
||||
result = await runner.run_strix_scan(
|
||||
scan_config={"targets": [], "scan_mode": "deep"},
|
||||
scan_id="scan-test",
|
||||
image="img",
|
||||
coordinator=coordinator,
|
||||
)
|
||||
return result, coordinator
|
||||
|
||||
with caplog.at_level(logging.WARNING):
|
||||
result = await runner.run_strix_scan(
|
||||
scan_config={"targets": [], "scan_mode": "deep"},
|
||||
scan_id="scan-test",
|
||||
image="img",
|
||||
coordinator=coordinator,
|
||||
)
|
||||
|
||||
def _assert_stopped_resumably(
|
||||
result: Any, coordinator: AgentCoordinator, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
assert result is None
|
||||
root_ids = [aid for aid, parent in coordinator.parent_of.items() if parent is None]
|
||||
assert len(root_ids) == 1
|
||||
|
|
@ -92,3 +106,29 @@ async def test_persistent_rate_limit_stops_gracefully(
|
|||
# the resume hint must carry the real scan id, not a literal placeholder
|
||||
assert "strix --resume scan-test" in caplog.text
|
||||
assert "<run_name>" not in caplog.text
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persistent_rate_limit_stops_gracefully(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Any, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A persistent OpenAI RateLimitError stops the scan (root -> 'stopped') without raising."""
|
||||
with caplog.at_level(logging.WARNING):
|
||||
result, coordinator = await _run_scan_raising(
|
||||
monkeypatch, tmp_path, _make_rate_limit_error()
|
||||
)
|
||||
_assert_stopped_resumably(result, coordinator, caplog)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persistent_usage_limit_non_openai_stops_gracefully(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Any, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A usage-limit error from a non-OpenAI (LiteLLM-routed) provider — not an
|
||||
``openai.RateLimitError`` — must also land on the resumable stop path, not the
|
||||
generic failure path."""
|
||||
with caplog.at_level(logging.WARNING):
|
||||
result, coordinator = await _run_scan_raising(
|
||||
monkeypatch, tmp_path, _make_litellm_usage_limit_error()
|
||||
)
|
||||
_assert_stopped_resumably(result, coordinator, caplog)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue