alishahryar1--free-claude-code
e22a38b2c2
## Problem Provider SDK classification, retry policy, canonical failures, and downstream wire errors shared exception types across layers. That blurred ownership and let cleanup or provisional Responses tool failures mask the real provider diagnostic. ## Changes | Before | After | | --- | --- | | Provider failures carried Anthropic wire types and core code classified OpenAI/httpx errors. | Protocol-neutral `ExecutionFailure` values cross layers, providers classify SDK errors, and protocol packages map wire types. | | Provider adapters could author terminal wire events. | The HTTP commit boundary selects non-2xx JSON or a protocol terminal event with one ingress request ID. | | Retry policy and diagnostic handling were spread across core and provider modules. | Providers own the unchanged retry budgets while neutral core utilities own bounded credential redaction. | | Stream cleanup could replace an already-mapped provider failure. | Cleanup records safe metadata and preserves the canonical failure, status, and diagnostic. | | An incomplete Responses tool could preempt a later provider failure. | Tool-finalization errors remain provisional so canonical provider failures take precedence. | | Readiness failures reused provider exception types. | Application-owned errors represent deterministic validation and availability phases without terminal retry headers. | | Legacy exception and recovery owners remained importable. | Obsolete modules are deleted without shims, architecture rules enforce the boundaries, and package version is 3.4.21. | <!-- greptile_comment --> <details open><summary><h3>Greptile Summary</h3></summary> This PR canonicalizes provider failure handling across the API boundary. The main changes are: - Adds protocol-neutral execution failure values and safe diagnostics. - Moves SDK and HTTP failure classification into provider-owned policy. - Lets Messages and Responses choose their own wire error payloads. - Preserves canonical failures across stream cleanup and committed stream failures. - Makes incomplete Responses tool errors provisional until finalization. </details> <h3>Confidence Score: 5/5</h3> This looks safe to merge. No blocking issues found in the changed code. No files need attention. <details><summary><h3><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="20" align="absmiddle"></a> T-Rex Logs</h3></summary> **What T-Rex did** - Ran the API failure contract suite and related tests (tests/api/test\_execution\_failure\_contract.py, tests/core/test\_failure\_protocol\_mapping.py, tests/providers/test\_execution\_failure\_boundary.py, tests/providers/test\_failure\_policy.py); 48 passed in 3.36s. - Ran the streaming boundaries tests including response streams, stream recovery, and streaming errors; 70 passed in 5.36s. - Ran the OpenAI responses tests; 20 passed in 4.42s. <a href="https://app.greptile.com/trex/runs/14071383/artifacts"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifactsDark.svg?v=4"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"><img alt="View all artifacts" src="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"></picture></a> <sub><a href="https://www.greptile.com/trex"><img alt="T-Rex" src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg" height="14" align="absmiddle"></a> Ran code and verified through T-Rex</sub> </details> <details open><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | src/free_claude_code/providers/transports/http.py | Adds cleanup-safe stream closing that preserves established outcomes. | | src/free_claude_code/core/openai_responses/stream.py | Preserves canonical execution failures when committed Responses streams fail. | | src/free_claude_code/core/openai_responses/streaming/assembler.py | Keeps malformed tool-call errors provisional so later provider failures can win. | | src/free_claude_code/core/failures.py | Defines neutral failure kinds and exception-group lookup for execution failures. | </details> <sub>Reviews (2): Last reviewed commit: ["Preserve canonical outcomes in grouped a..."](https://github.com/alishahryar1/free-claude-code/commit/f57f21241dbe582985627ed4fb40734b2c656809) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=43468297)</sub> <!-- /greptile_comment -->
153 行
5.1 KiB
Python
153 行
5.1 KiB
Python
"""Protocol-neutral diagnostic redaction and detail contracts."""
|
|
|
|
from httpx import ConnectError, HTTPStatusError, Request, Response
|
|
|
|
from free_claude_code.core.diagnostics import (
|
|
ERROR_DETAIL_DISPLAY_CAP_BYTES,
|
|
UpstreamErrorDetail,
|
|
attach_upstream_error_body,
|
|
exception_cause_types,
|
|
extract_upstream_error_detail,
|
|
format_execution_failure_message,
|
|
format_user_error_preview,
|
|
redact_sensitive_error_text,
|
|
safe_exception_message,
|
|
)
|
|
from free_claude_code.core.failures import ExecutionFailure, FailureKind
|
|
|
|
|
|
def test_redaction_preserves_context_and_covers_recognizable_credentials() -> None:
|
|
sanitized = redact_sensitive_error_text(
|
|
'{"authorization":"Bearer AUTH_TOKEN","api_key":"sk-live-secret-key",'
|
|
'"client_secret":"CLIENT_SECRET"} raw=nvapi-standalone-secret '
|
|
"token=PLAIN_TOKEN"
|
|
)
|
|
|
|
assert sanitized == (
|
|
'{"authorization":"<redacted>","api_key":"<redacted>",'
|
|
'"client_secret":"<redacted>"} raw=<redacted> token=<redacted>'
|
|
)
|
|
|
|
|
|
def test_safe_exception_message_is_detailed_redacted_and_non_empty() -> None:
|
|
assert (
|
|
safe_exception_message(
|
|
RuntimeError("gateway failed api_key=SECRET useful detail")
|
|
)
|
|
== "gateway failed api_key=<redacted> useful detail"
|
|
)
|
|
assert safe_exception_message(RuntimeError()) == (
|
|
"Provider request failed unexpectedly."
|
|
)
|
|
assert format_user_error_preview(ValueError("x" * 500), max_len=20) == "x" * 20
|
|
|
|
|
|
def test_extract_upstream_error_detail_compacts_json_and_redacts_secrets() -> None:
|
|
response = Response(
|
|
status_code=400,
|
|
request=Request("POST", "https://provider.test/v1/messages"),
|
|
json={
|
|
"error": {
|
|
"type": "BadRequest",
|
|
"message": "bad field api_key=SECRET authorization: Bearer TOKEN",
|
|
}
|
|
},
|
|
)
|
|
error = HTTPStatusError(
|
|
"Bad Request",
|
|
request=response.request,
|
|
response=response,
|
|
)
|
|
|
|
detail = extract_upstream_error_detail(error)
|
|
|
|
assert isinstance(detail, UpstreamErrorDetail)
|
|
assert detail.status_code == 400
|
|
assert detail.body_text == (
|
|
'{"error":{"type":"BadRequest","message":'
|
|
'"bad field api_key=<redacted> authorization: <redacted>"}}'
|
|
)
|
|
assert detail.exception_text == "Bad Request"
|
|
assert detail.cause_chain_text is None
|
|
assert detail.category_hint == "BadRequest"
|
|
assert not detail.body_truncated
|
|
assert "SECRET" not in repr(detail)
|
|
assert "TOKEN" not in repr(detail)
|
|
|
|
|
|
def test_attached_upstream_body_is_capped_after_redaction() -> None:
|
|
assert ERROR_DETAIL_DISPLAY_CAP_BYTES == 16_384
|
|
response = Response(
|
|
status_code=500,
|
|
request=Request("POST", "https://provider.test/v1/messages"),
|
|
content=b"",
|
|
)
|
|
error = HTTPStatusError(
|
|
"Server Error",
|
|
request=response.request,
|
|
response=response,
|
|
)
|
|
attach_upstream_error_body(
|
|
error,
|
|
"token=SECRET " + "x" * ERROR_DETAIL_DISPLAY_CAP_BYTES,
|
|
)
|
|
|
|
detail = extract_upstream_error_detail(error)
|
|
|
|
assert detail.body_text is not None
|
|
assert detail.body_truncated
|
|
assert "SECRET" not in detail.body_text
|
|
assert "token=<redacted>" in detail.body_text
|
|
assert f"truncated after {ERROR_DETAIL_DISPLAY_CAP_BYTES} bytes" in detail.body_text
|
|
|
|
|
|
def test_cause_chain_is_redacted_capped_and_has_safe_type_metadata() -> None:
|
|
request = Request("POST", "https://provider.test/v1/messages")
|
|
error = RuntimeError("provider connection failed")
|
|
error.__cause__ = ConnectError(
|
|
"connect failed authorization: Bearer SECRET token=ALSO_SECRET "
|
|
+ "x" * ERROR_DETAIL_DISPLAY_CAP_BYTES,
|
|
request=request,
|
|
)
|
|
|
|
detail = extract_upstream_error_detail(error)
|
|
|
|
assert exception_cause_types(error) == ("ConnectError",)
|
|
assert detail.cause_chain_text is not None
|
|
assert "ConnectError: connect failed authorization: <redacted>" in (
|
|
detail.cause_chain_text
|
|
)
|
|
assert "SECRET" not in detail.cause_chain_text
|
|
assert f"truncated after {ERROR_DETAIL_DISPLAY_CAP_BYTES} bytes" in (
|
|
detail.cause_chain_text
|
|
)
|
|
|
|
|
|
def test_execution_failure_format_uses_semantic_category_and_request_id() -> None:
|
|
failure = ExecutionFailure(
|
|
kind=FailureKind.INVALID_REQUEST,
|
|
status_code=400,
|
|
message="Invalid request sent to provider.",
|
|
retryable=False,
|
|
)
|
|
detail = UpstreamErrorDetail(
|
|
status_code=400,
|
|
body_text='{"error":{"message":"bad field token=<redacted>"}}',
|
|
exception_text="Bad Request",
|
|
category_hint=None,
|
|
)
|
|
|
|
message = format_execution_failure_message(
|
|
failure,
|
|
detail,
|
|
upstream_name="ACME",
|
|
request_id="req_diagnostic",
|
|
)
|
|
|
|
assert "Upstream provider ACME returned HTTP 400." in message
|
|
assert "Category: invalid_request" in message
|
|
assert "Mapped message: Invalid request sent to provider." in message
|
|
assert '{"error":{"message":"bad field token=<redacted>"}}' in message
|
|
assert "Request ID: req_diagnostic" in message
|
|
assert "invalid_request_error" not in message
|