mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix: a body that stops mid-read is transient, and a round that dies still says so
Round 5 died on `http.client.IncompleteRead(0 bytes read)` and left no record at all. Two defects, both ours. The retry classified only connection-level failures. IncompleteRead and RemoteDisconnected arrive while the *body* is being read, after the connection succeeded, so they are not URLError -- they escaped both the retry and the error wrapper and walked out of the round. The network is allowed to be unreliable; losing a paid round to it is not the provider's defect. And the exception path was wrapped around the stages that come *after* the proposal is applied, because those are the ones with something to revert. The stages before it had already spent money, and a round that leaves no record is a round nobody can account for. Every outcome is journalled now, with the exception type in the reason. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
This commit is contained in:
co-authored by
Claude Opus 5
parent
cde57e4293
commit
3ebcc6729a
@@ -406,10 +406,25 @@ def run_learning_round(
|
|||||||
spent = spent_usd() if spent_usd is not None else 0.0
|
spent = spent_usd() if spent_usd is not None else 0.0
|
||||||
return not within_budget(spent, cost_cap_usd)
|
return not within_budget(spent, cost_cap_usd)
|
||||||
|
|
||||||
|
record = _round_record(round_number, family, None, taxonomy, config)
|
||||||
|
try:
|
||||||
before_scan = scan_factory(config)
|
before_scan = scan_factory(config)
|
||||||
before_train = _score(batch, before_scan, family)
|
before_train = _score(batch, before_scan, family)
|
||||||
before_hold = _score(holdout, before_scan, family)
|
before_hold = _score(holdout, before_scan, family)
|
||||||
record = _round_record(round_number, family, None, taxonomy, config)
|
except Exception as exc:
|
||||||
|
# These stages run before anything is applied, so there is nothing to revert — but they have already spent
|
||||||
|
# money, and a round that leaves no record is a round nobody can account for. Measured: round 5 died on a
|
||||||
|
# network read during its first stage and vanished.
|
||||||
|
_finish(
|
||||||
|
journal,
|
||||||
|
directory,
|
||||||
|
record,
|
||||||
|
outcome="reverted",
|
||||||
|
reason=f"stage_failed: {type(exc).__name__}",
|
||||||
|
cost=spent_usd() if spent_usd is not None else 0.0,
|
||||||
|
trajectories=trajectories,
|
||||||
|
)
|
||||||
|
raise
|
||||||
if _over_cap():
|
if _over_cap():
|
||||||
return _finish(
|
return _finish(
|
||||||
journal, directory, record, outcome="reverted_cost", reason="cost_cap_exceeded", cost=spent, trajectories=trajectories
|
journal, directory, record, outcome="reverted_cost", reason="cost_cap_exceeded", cost=spent, trajectories=trajectories
|
||||||
@@ -551,7 +566,7 @@ def run_learning_round(
|
|||||||
version=version,
|
version=version,
|
||||||
trajectories=trajectories,
|
trajectories=trajectories,
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
# A stage that raises has already spent money and already changed the tree. Both facts belong in the
|
# A stage that raises has already spent money and already changed the tree. Both facts belong in the
|
||||||
# journal before the exception leaves, or the next round starts from a state nothing describes.
|
# journal before the exception leaves, or the next round starts from a state nothing describes.
|
||||||
snapshot.restore()
|
snapshot.restore()
|
||||||
@@ -560,7 +575,7 @@ def run_learning_round(
|
|||||||
directory,
|
directory,
|
||||||
record,
|
record,
|
||||||
outcome="reverted",
|
outcome="reverted",
|
||||||
reason="stage_failed",
|
reason=f"stage_failed: {type(exc).__name__}",
|
||||||
proposal=proposal,
|
proposal=proposal,
|
||||||
cost=spent,
|
cost=spent,
|
||||||
trajectories=trajectories,
|
trajectories=trajectories,
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import http.client
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
@@ -20,10 +21,14 @@ _T = TypeVar("_T")
|
|||||||
|
|
||||||
|
|
||||||
def _is_transient(exc: Exception) -> bool:
|
def _is_transient(exc: Exception) -> bool:
|
||||||
"""A network failure worth retrying (rate-limit, 5xx, connection error, timeout)."""
|
"""A network failure worth retrying: rate-limit, 5xx, connection error, timeout, or a body that stopped.
|
||||||
|
|
||||||
|
`http.client.IncompleteRead` and `RemoteDisconnected` arrive while the *body* is being read, after the
|
||||||
|
connection succeeded, so they are not `URLError`. Measured: one of them ended an evolve round that had already
|
||||||
|
been paid for."""
|
||||||
if isinstance(exc, urllib.error.HTTPError):
|
if isinstance(exc, urllib.error.HTTPError):
|
||||||
return exc.code in _TRANSIENT_STATUS
|
return exc.code in _TRANSIENT_STATUS
|
||||||
return isinstance(exc, urllib.error.URLError | TimeoutError)
|
return isinstance(exc, urllib.error.URLError | TimeoutError | http.client.HTTPException)
|
||||||
|
|
||||||
|
|
||||||
def call_with_retry(
|
def call_with_retry(
|
||||||
@@ -108,8 +113,8 @@ class OpenRouterChatClient:
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
return call_with_retry(_do, attempts=self.max_attempts, base_delay=self.retry_base_delay)
|
return call_with_retry(_do, attempts=self.max_attempts, base_delay=self.retry_base_delay)
|
||||||
except (urllib.error.URLError, TimeoutError, json.JSONDecodeError) as exc:
|
except (urllib.error.URLError, TimeoutError, http.client.HTTPException, json.JSONDecodeError) as exc:
|
||||||
raise OpenRouterError(f"OpenRouter chat request failed: {exc}") from exc
|
raise OpenRouterError(f"OpenRouter chat request failed: {type(exc).__name__}: {exc}") from exc
|
||||||
|
|
||||||
def complete_json(self, *, model: str, messages: list[dict[str, str]], timeout_seconds: int = 60) -> object:
|
def complete_json(self, *, model: str, messages: list[dict[str, str]], timeout_seconds: int = 60) -> object:
|
||||||
message = self.complete_chat(model=model, messages=messages, timeout_seconds=timeout_seconds)
|
message = self.complete_chat(model=model, messages=messages, timeout_seconds=timeout_seconds)
|
||||||
@@ -151,8 +156,8 @@ class OpenRouterEmbeddingClient:
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
response_payload = call_with_retry(_do, attempts=self.max_attempts, base_delay=self.retry_base_delay)
|
response_payload = call_with_retry(_do, attempts=self.max_attempts, base_delay=self.retry_base_delay)
|
||||||
except (urllib.error.URLError, TimeoutError, json.JSONDecodeError) as exc:
|
except (urllib.error.URLError, TimeoutError, http.client.HTTPException, json.JSONDecodeError) as exc:
|
||||||
raise OpenRouterError(f"OpenRouter embedding request failed: {exc}") from exc
|
raise OpenRouterError(f"OpenRouter embedding request failed: {type(exc).__name__}: {exc}") from exc
|
||||||
return parse_embedding_response(response_payload)
|
return parse_embedding_response(response_payload)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -224,7 +224,7 @@ def test_a_stage_that_raises_is_reverted_journalled_and_re_raised(world) -> None
|
|||||||
_run(world, _proposal(), build)
|
_run(world, _proposal(), build)
|
||||||
assert (world["configs"] / "injection" / "checklist.md").read_text() == "- old\n"
|
assert (world["configs"] / "injection" / "checklist.md").read_text() == "- old\n"
|
||||||
outcomes = [(item.outcome, item.reason) for item in world["journal"].rounds()]
|
outcomes = [(item.outcome, item.reason) for item in world["journal"].rounds()]
|
||||||
assert outcomes == [("reverted", "stage_failed")]
|
assert outcomes == [("reverted", "stage_failed: RuntimeError")] # the exception type is part of the record
|
||||||
assert (world["out"] / "rounds" / "1" / "scores.json").is_file()
|
assert (world["out"] / "rounds" / "1" / "scores.json").is_file()
|
||||||
|
|
||||||
|
|
||||||
@@ -406,3 +406,23 @@ def test_a_round_journals_why_the_proposer_had_nothing_to_say(world) -> None: #
|
|||||||
)
|
)
|
||||||
assert record.outcome == "rejected"
|
assert record.outcome == "rejected"
|
||||||
assert record.reason == "no_proposal: meta_agent_failed: RuntimeError"
|
assert record.reason == "no_proposal: meta_agent_failed: RuntimeError"
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_round_that_dies_before_it_applies_anything_is_still_journalled(world) -> None: # type: ignore[no-untyped-def]
|
||||||
|
"""Measured: round 5 died on a network read during its *first* stage and left no record at all.
|
||||||
|
|
||||||
|
The exception path was wrapped around the stages after the proposal is applied, because those are the ones that
|
||||||
|
have something to revert. The ones before it had already spent money."""
|
||||||
|
|
||||||
|
def build(config: FamilyConfig): # type: ignore[no-untyped-def]
|
||||||
|
def scan(root: Path) -> list[StaticFinding]:
|
||||||
|
raise ConnectionError("the body stopped arriving")
|
||||||
|
|
||||||
|
return scan
|
||||||
|
|
||||||
|
with pytest.raises(ConnectionError):
|
||||||
|
_run(world, _proposal(), build)
|
||||||
|
outcomes = [(item.outcome, item.reason) for item in world["journal"].rounds()]
|
||||||
|
assert outcomes == [("reverted", "stage_failed: ConnectionError")]
|
||||||
|
assert (world["out"] / "rounds" / "1").is_dir()
|
||||||
|
assert (world["configs"] / "injection" / "checklist.md").read_text() == "- old\n"
|
||||||
|
|||||||
@@ -63,3 +63,53 @@ def test_non_retryable_value_error_propagates_immediately() -> None:
|
|||||||
|
|
||||||
with pytest.raises(OpenRouterError):
|
with pytest.raises(OpenRouterError):
|
||||||
call_with_retry(op, attempts=3, base_delay=0, sleep=_no_sleep)
|
call_with_retry(op, attempts=3, base_delay=0, sleep=_no_sleep)
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_body_that_stops_mid_read_is_transient() -> None:
|
||||||
|
"""Measured: an evolve round died on `http.client.IncompleteRead(0 bytes read)`.
|
||||||
|
|
||||||
|
The retry classified only connection-level failures, so a response whose *body* stopped arriving escaped both
|
||||||
|
the retry and the error wrapper, and the exception walked out of the round. The network is allowed to be
|
||||||
|
unreliable; losing a paid round to it is our defect, not the provider's."""
|
||||||
|
import http.client
|
||||||
|
|
||||||
|
from openultrasast.provider.openrouter import OpenRouterError, _is_transient, call_with_retry
|
||||||
|
|
||||||
|
assert _is_transient(http.client.IncompleteRead(b"")) is True
|
||||||
|
assert _is_transient(http.client.RemoteDisconnected("closed")) is True
|
||||||
|
assert _is_transient(ValueError("a bug in our own code")) is False
|
||||||
|
|
||||||
|
attempts = []
|
||||||
|
|
||||||
|
def flaky() -> str:
|
||||||
|
attempts.append(1)
|
||||||
|
if len(attempts) < 3:
|
||||||
|
raise http.client.IncompleteRead(b"")
|
||||||
|
return "ok"
|
||||||
|
|
||||||
|
assert call_with_retry(flaky, attempts=3, base_delay=0.0, sleep=lambda _s: None) == "ok"
|
||||||
|
assert len(attempts) == 3
|
||||||
|
assert issubclass(OpenRouterError, Exception)
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_read_failure_is_reported_as_a_provider_error_not_a_raw_http_exception() -> None:
|
||||||
|
import http.client
|
||||||
|
import urllib.error
|
||||||
|
|
||||||
|
from openultrasast.provider.openrouter import OpenRouterChatClient, OpenRouterError
|
||||||
|
|
||||||
|
client = OpenRouterChatClient(api_key="k", base_url="https://example.invalid", max_attempts=1)
|
||||||
|
|
||||||
|
def boom(*_args: object, **_kwargs: object) -> object:
|
||||||
|
raise http.client.IncompleteRead(b"")
|
||||||
|
|
||||||
|
import openultrasast.provider.openrouter as module
|
||||||
|
|
||||||
|
original = module.urllib.request.urlopen
|
||||||
|
module.urllib.request.urlopen = boom # type: ignore[assignment]
|
||||||
|
try:
|
||||||
|
with pytest.raises(OpenRouterError, match="IncompleteRead"):
|
||||||
|
client.complete_chat_raw(model="m", messages=[{"role": "user", "content": "x"}])
|
||||||
|
finally:
|
||||||
|
module.urllib.request.urlopen = original # type: ignore[assignment]
|
||||||
|
assert urllib.error.URLError is not None
|
||||||
|
|||||||
Reference in New Issue
Block a user