From 4761031cb48fd3e280ab62286ef24abb09a7e260 Mon Sep 17 00:00:00 2001 From: Hao Zhe Date: Wed, 14 Oct 2026 20:35:08 +0800 Subject: [PATCH] fix(models): tighten transient retry classification --- openviking/utils/model_retry.py | 25 +++++++++++--- tests/unit/test_model_retry.py | 61 +++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/openviking/utils/model_retry.py b/openviking/utils/model_retry.py index 2caf88c82c..4f432b159d 100644 --- a/openviking/utils/model_retry.py +++ b/openviking/utils/model_retry.py @@ -155,6 +155,11 @@ def extract_metric_error_code(error: BaseException) -> str: "connectionerror", "connection refused", "connection reset", + # Node-based gateways can forward errno tokens instead of prose transport + # errors. These are equivalent transient connection failures. + "econnrefused", + "econnreset", + "etimedout", ) RETRYABLE_RATE_LIMIT_MARKERS = ( @@ -176,17 +181,25 @@ def extract_metric_error_code(error: BaseException) -> str: RATE_LIMIT_RETRY_BASE_DELAY_SECONDS = 5.0 RATE_LIMIT_RETRY_MAX_DELAY_SECONDS = 120.0 -# Pre-compile regex for numeric status-code patterns to avoid substring false positives -# (e.g. "413" matching inside request IDs like "d7c9130f344..." or "req-413-abcd"). +# Pre-compile regex for numeric status-code patterns to avoid matching unrelated +# values (for example request IDs, token limits, or batch sizes). _NUMERIC_PATTERN_RE: dict[str, re.Pattern] = {} +_HTTP_STATUS_SUFFIX = ( + r"(?:status(?:\s+code)?|bad\s+request|unauthorized|forbidden|payload\s+too\s+large|" + r"too\s+many\s+requests|internal\s+server\s+error|bad\s+gateway|" + r"service\s+unavailable|gateway\s+timeout)" +) def _get_numeric_pattern_re(pattern: str) -> re.Pattern: if pattern not in _NUMERIC_PATTERN_RE: escaped = re.escape(pattern) _NUMERIC_PATTERN_RE[pattern] = re.compile( - rf"(?:\b(?:error\s*code|status(?:\s*code)?|http(?:\s*status)?|code)" - rf"\s*[:=]?\s*{escaped}(?!\w)|(? bool: """Check if pattern matches in text, using token-aware matching for numeric patterns. Numeric-only patterns (e.g. ``"413"``) must look like HTTP status codes, not - request ID fragments. Non-numeric patterns use plain substring matching as before. + request IDs or unrelated values. A status code is accepted at the start of the + message, after an explicit status label, or before ``status code``. Non-numeric + patterns use plain substring matching as before. """ if pattern.isdigit(): return bool(_get_numeric_pattern_re(pattern).search(text_lower)) or bool( diff --git a/tests/unit/test_model_retry.py b/tests/unit/test_model_retry.py index cedf5a2d87..5a779b0e02 100644 --- a/tests/unit/test_model_retry.py +++ b/tests/unit/test_model_retry.py @@ -370,3 +370,64 @@ def test_numeric_status_code_with_compact_error_code_context_still_matches(): assert classify_api_error(RuntimeError("Error code:413-Payload Too Large")) == ( ERROR_CLASS_INPUT_TOO_LARGE ) + + +@pytest.mark.parametrize( + "message", + [ + "Validation error: max_tokens=500 is too high", + "Validation error: token budget 500 is too high", + "Configured batch size: 502 items", + "The response limit is 504 tokens", + ], +) +def test_numeric_values_without_http_status_context_stay_unknown(message): + assert classify_api_error(RuntimeError(message)) == ERROR_CLASS_UNKNOWN + + +@pytest.mark.parametrize( + ("message", "expected"), + [ + ("500 Internal Server Error", ERROR_CLASS_TRANSIENT), + ("HTTP 500 Internal Server Error", ERROR_CLASS_TRANSIENT), + ("OpenAI API error (500): request failed", ERROR_CLASS_TRANSIENT), + ("Upstream returned 500 Internal Server Error", ERROR_CLASS_TRANSIENT), + ("API error: 502 Bad Gateway", ERROR_CLASS_TRANSIENT), + ("status code: 503 Service Unavailable", ERROR_CLASS_TRANSIENT), + ("code: 503", ERROR_CLASS_TRANSIENT), + ("504 status code", ERROR_CLASS_TRANSIENT), + ("413 Payload Too Large", ERROR_CLASS_INPUT_TOO_LARGE), + ("403 AccountQuotaExceeded", ERROR_CLASS_AUTH), + ("Error code: 400 - invalid parameter", ERROR_CLASS_PERMANENT), + ], +) +def test_numeric_http_status_context_still_classifies(message, expected): + assert classify_api_error(RuntimeError(message)) == expected + + +@pytest.mark.parametrize("errno", ["ECONNREFUSED", "ECONNRESET", "ETIMEDOUT"]) +def test_node_transport_errno_is_transient(errno): + assert classify_api_error(RuntimeError(f"connect {errno} upstream")) == ERROR_CLASS_TRANSIENT + + +def test_retry_sync_uses_status_context_and_node_errno(): + validation_attempts = {"count": 0} + + def invalid_request(): + validation_attempts["count"] += 1 + raise RuntimeError("Validation error: max_tokens=500 is too high") + + with pytest.raises(RuntimeError): + retry_sync(invalid_request, max_retries=3, base_delay=0, jitter=False) + assert validation_attempts["count"] == 1 + + transport_attempts = {"count": 0} + + def transient_request(): + transport_attempts["count"] += 1 + if transport_attempts["count"] == 1: + raise RuntimeError("connect ECONNRESET upstream") + return "ok" + + assert retry_sync(transient_request, max_retries=3, base_delay=0, jitter=False) == "ok" + assert transport_attempts["count"] == 2