From cf6b5eec85b8524e8906a4711680c4aa84392e94 Mon Sep 17 00:00:00 2001 From: Christie Williams Date: Wed, 14 Oct 2026 16:17:20 -0700 Subject: [PATCH] fix(claude-agents): fail the run when the CLI reports an API error as success The Claude Code CLI does not fail a run when an Anthropic API call fails (bad key, no credits, rate limit after retries). Measured against Agent SDK 0.3.220, it emits a synthetic assistant message (model "", the error text as content, zero usage) and then a result with subtype "success", is_error true, api_error_status set, and the error text in result. The handler only checked subtype, so it returned "Credit balance is too low" as the model output, invoke tracked generation success, and the trace showed an OK "chat " span. A result now fails the run when its subtype is not "success" or is_error is true, the same test the agent SDK applies itself. A synthetic assistant message produces no chat span, joins no conversation, and adds no usage. The native graph adapter applies the same check to every node, which also stops it from returning "" for an error-subtype result. Co-Authored-By: Claude Opus 5.5 --- packages/claude-agents/agents.md | 12 ++ .../launchdarkly_ai_claude_agents/handler.py | 37 +++-- .../native_graph.py | 4 + .../launchdarkly_ai_claude_agents/spans.py | 18 ++- packages/claude-agents/tests/test_handler.py | 130 +++++++++++++++++ .../claude-agents/tests/test_native_graph.py | 133 +++++++++++++++++- 6 files changed, 319 insertions(+), 15 deletions(-) diff --git a/packages/claude-agents/agents.md b/packages/claude-agents/agents.md index bce87584..aa369b4a 100644 --- a/packages/claude-agents/agents.md +++ b/packages/claude-agents/agents.md @@ -108,12 +108,24 @@ gen = query_fn(prompt=prompt, options=options) try: async for message in gen: if isinstance(message, ResultMessage): + _raise_if_run_failed(message) # raises on a failed run # done — extract output and usage break finally: await gen.aclose() ``` +A `success` subtype is not enough to call the run a success. An error subtype (`error_max_turns`, +`error_during_execution`, ...) fails, and so does a `success` result with `is_error` set. The second +is how the CLI reports a failed Anthropic API call (bad key, no credits, rate limit after retries): +`api_error_status` is set and `result` holds the error text. `_raise_if_run_failed` raises on either +shape, so the caller tracks `$ld:ai:generation:error` instead of being handed the error text as the +model's answer. The native graph adapter applies the same check to every node's run. + +The CLI also writes its own `AssistantMessage`, with `model` set to the literal ``, when +an API call fails and after a response stops at `max_output_tokens`. It is not a model response: +it gets no `chat` span and never joins the conversation a later span reports. + **Important:** a bare `return` inside `async for` abandons the generator. Python's asyncio finalizer will try to `aclose()` it later and may raise `RuntimeError: aclose(): asynchronous generator is already running` if the generator is still suspended inside a real SDK `await`. Always use `break` + explicit `aclose()`. See **Appendix A.4** in `TESTING.md`. ### 4. Telemetry diff --git a/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py b/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py index 3fe55f3d..c12b3b65 100644 --- a/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py +++ b/packages/claude-agents/src/launchdarkly_ai_claude_agents/handler.py @@ -477,15 +477,30 @@ def _opening_of(prompt: str, system_prompt: str | None) -> Opening: ) -def _result_error(subtype: str, errors: list[str] | None) -> str: - """Builds the error for a non-success result message. +def _raise_if_run_failed(message: Any) -> None: + """Raises when a result message reports a failed run. - ``SDKResultError`` carries no ``result`` field, so a run that hit ``error_max_turns`` or - ``error_max_budget_usd`` genuinely failed and has to surface as a failure rather than being - reported OK with zeroed usage. + Two shapes fail. A non-``success`` subtype carries no ``result`` field, so a run that hit + ``error_max_turns`` or ``error_max_budget_usd`` genuinely failed and has to surface as a failure + rather than being reported OK with zeroed usage. + + The other is a ``success`` subtype with ``is_error`` set, which is how the CLI reports a failed + Anthropic API call. Measured against Agent SDK 0.3.220 with a key that had no credits: the run + ends ``success``, ``is_error`` true, ``api_error_status`` set, and the error text in ``result`` — + so a caller was handed "Credit balance is too low" as the model's answer. """ - detail = f": {'; '.join(errors)}" if errors else "" - return f"Claude agent run ended with {subtype}{detail}" + if message.subtype != "success": + detail = f": {'; '.join(message.errors)}" if message.errors else "" + raise RuntimeError(f"Claude agent run ended with {message.subtype}{detail}") + if message.is_error is not True: + return + # ``getattr``: the field arrived with CLI v2.1.110, and older SDKs' dataclass lacks it. + status = getattr(message, "api_error_status", None) + status_text = f" (HTTP {status})" if isinstance(status, int) else "" + detail = f": {message.result}" if message.result else "" + raise RuntimeError( + f"Claude agent run ended with an error result{status_text}{detail}" + ) def _build_query_options( @@ -636,10 +651,7 @@ async def _call_impl( # The CLI's own run-level total is authoritative, so the except clause below # must not overwrite it with the summed-per-response figure. root_usage_written = True - if message.subtype != "success": - raise RuntimeError( - _result_error(message.subtype, message.errors) - ) + _raise_if_run_failed(message) result_text = ( message.result if isinstance(message.result, str) @@ -903,8 +915,7 @@ async def _stream_gen( raw_usage: dict[str, Any] = dict(message.usage or {}) finish_root_span(span, config, raw_usage) root_usage_written = True - if message.subtype != "success": - raise RuntimeError(_result_error(message.subtype, message.errors)) + _raise_if_run_failed(message) final_output = ( message.result if message.result is not None else full_output ) diff --git a/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py b/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py index 9f92ab0b..6bdf787c 100644 --- a/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py +++ b/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py @@ -34,6 +34,7 @@ from launchdarkly_ai_claude_agents.handler import ( _build_hooks, + _raise_if_run_failed, build_prompt, build_query_prompt, build_tool_mcp, @@ -182,6 +183,9 @@ async def _run_query( try: async for message in gen: if isinstance(message, ResultMessage): + # Raising fails the graph at the root, and fails the tool call at a sub-agent, + # instead of answering with an empty string or with the API's error text. + _raise_if_run_failed(message) output = message.result or "" raw_usage = message.usage or {} break diff --git a/packages/claude-agents/src/launchdarkly_ai_claude_agents/spans.py b/packages/claude-agents/src/launchdarkly_ai_claude_agents/spans.py index 2921b297..f25adc3e 100644 --- a/packages/claude-agents/src/launchdarkly_ai_claude_agents/spans.py +++ b/packages/claude-agents/src/launchdarkly_ai_claude_agents/spans.py @@ -213,6 +213,18 @@ def record_native_tools( set_tool_definition_attributes(span, capture, catalog.current) +def _is_synthetic(message: AssistantMessage) -> bool: + """Whether an assistant message is one the CLI wrote itself rather than one the model returned. + + Measured against Agent SDK 0.3.220: a failed API call surfaces as an assistant message whose + ``model`` is the literal ````, whose content is the error text and whose usage is + zero, and the CLI emits the same after a response stops at ``max_output_tokens``. Keyed on + ``model`` rather than on the message's ``error`` field because the latter is set on both, and + only the run's result says whether the run failed. + """ + return message.model == "" + + def marks_local_work(message: Any) -> bool: """Whether a non-assistant message marks the end of local work, and so the start of the window that will produce the next response. @@ -444,7 +456,11 @@ def _thread_for(self, parent_tool_use_id: str | None) -> list[SpanMessage]: def record(self, message: Any) -> None: """Feed every message of the run, in order.""" if isinstance(message, AssistantMessage): - self._absorb(message) + # Not a model response, so no span, no turn in the conversation and no usage. It can + # share the real response's id, which would otherwise append the error text to that + # response's output. + if not _is_synthetic(message): + self._absorb(message) return if isinstance(message, UserMessage): self._absorb_user_turn(message) diff --git a/packages/claude-agents/tests/test_handler.py b/packages/claude-agents/tests/test_handler.py index 74f64e24..b4b67d01 100644 --- a/packages/claude-agents/tests/test_handler.py +++ b/packages/claude-agents/tests/test_handler.py @@ -144,6 +144,47 @@ def result_message( ) +# Measured against Agent SDK 0.3.220 with a rejected key, and the same for a key without credits: an +# API failure does not fail the run. The CLI emits a synthetic assistant message whose ``model`` is +# the literal ```` and whose content is the error text, then a result that says +# ``success`` but sets ``is_error``. The CLI emits the same kind of synthetic message after a +# response stops at ``max_output_tokens``, and that run can still succeed. +ZERO_USAGE: dict[str, Any] = { + "input_tokens": 0, + "output_tokens": 0, + "cache_creation_input_tokens": 0, + "cache_read_input_tokens": 0, +} + + +def synthetic_message( + text: str = "Credit balance is too low", message_id: str = "msg_synthetic" +) -> AssistantMessage: + return AssistantMessage( + content=[TextBlock(text=text)], + model="", + error="billing_error", + usage=dict(ZERO_USAGE), + message_id=message_id, + stop_reason="stop_sequence", + session_id="sess-1", + ) + + +def api_error_result_message(text: str = "Credit balance is too low") -> ResultMessage: + return ResultMessage( + subtype="success", + duration_ms=1, + duration_api_ms=0, + is_error=True, + num_turns=1, + session_id="sess-1", + usage=dict(ZERO_USAGE), + result=text, + api_error_status=400, + ) + + def init_message( session_id: str = "sess-1", tools: list[str] | None = None ) -> SystemMessage: @@ -960,6 +1001,95 @@ async def test_error_result_fails_root_but_keeps_spend( assert attrs["gen_ai.usage.input_tokens"] == 40000 assert attrs["gen_ai.usage.output_tokens"] == 500 + async def test_api_error_under_success_subtype_fails_the_run( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr( + handler_mod, + "query", + _fake_query( + [init_message(), synthetic_message(), api_error_result_message()] + ), + ) + with pytest.raises(RuntimeError, match=r"400.*Credit balance is too low"): + await create_claude_agents_handler(capture_content=True)(BASE_CONFIG, "q") + # The synthetic message is not a model response, so it gets no chat span. + assert [s.name for s in spans()] == ["invoke_agent"] + assert root().status.status_code == StatusCode.ERROR + [exception] = [e for e in root().events if e.name == "exception"] + assert "Credit balance is too low" in exception.attributes["exception.message"] + # The error text is not model output. + assert "gen_ai.output.messages" not in root().attributes + + async def test_streamed_api_error_fails_the_run_without_a_done_event( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr( + handler_mod, + "query", + _fake_query( + [init_message(), synthetic_message(), api_error_result_message()] + ), + ) + events: list[Any] = [] + with pytest.raises(RuntimeError, match=r"400.*Credit balance is too low"): + gen = await create_claude_agents_handler(capture_content=True).stream( + BASE_CONFIG, "q", {}, {} + ) + async for event in gen: + events.append(event) + assert events == [] + assert [s.name for s in spans()] == ["invoke_agent"] + assert root().status.status_code == StatusCode.ERROR + assert "gen_ai.output.messages" not in root().attributes + + async def test_success_result_with_is_error_false_is_a_successful_run( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr( + handler_mod, + "query", + _fake_query([assistant_message(), result_message("fine")]), + ) + result = await create_claude_agents_handler()(BASE_CONFIG, "q") + assert result["output"] == "fine" + assert root().status.status_code == StatusCode.OK + + async def test_synthetic_message_is_left_out_of_every_chat_span( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + # A synthetic message can share the real response's id, so grouping on it would append the + # error text to that response's output. + monkeypatch.setattr( + handler_mod, + "query", + _fake_query( + [ + assistant_message(10, 2, "msg_1", [TextBlock(text="first half")]), + synthetic_message("API Error: output token maximum", "msg_1"), + tool_result_user_message(), + assistant_message(12, 3, "msg_2", [TextBlock(text="second half")]), + result_message("second half"), + ] + ), + ) + result = await create_claude_agents_handler(capture_content=True)( + BASE_CONFIG, "q" + ) + assert result["output"] == "second half" + chats = named("chat ") + assert [c.name for c in chats] == [ + "chat claude-opus-4-5", + "chat claude-opus-4-5", + ] + for chat in chats: + assert "output token maximum" not in str( + chat.attributes.get("gen_ai.output.messages") + ) + assert "output token maximum" not in str( + chat.attributes.get("gen_ai.input.messages") + ) + async def test_reports_responses_that_arrived_when_the_sdk_throws( self, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/packages/claude-agents/tests/test_native_graph.py b/packages/claude-agents/tests/test_native_graph.py index e01900f4..efbbb782 100644 --- a/packages/claude-agents/tests/test_native_graph.py +++ b/packages/claude-agents/tests/test_native_graph.py @@ -21,10 +21,21 @@ def _make_result_msg( - text: str = "result", input_tokens: int = 5, output_tokens: int = 3 + text: str | None = "result", + input_tokens: int = 5, + output_tokens: int = 3, + subtype: str = "success", + is_error: bool = False, + errors: list[str] | None = None, + api_error_status: int | None = None, ) -> Any: m = MagicMock() m.__class__.__name__ = "ResultMessage" + # Set explicitly: an unset MagicMock attribute is truthy, so `is_error` would read as an error. + m.subtype = subtype + m.is_error = is_error + m.errors = errors + m.api_error_status = api_error_status m.result = text m.usage = {"input_tokens": input_tokens, "output_tokens": output_tokens} return m @@ -625,6 +636,126 @@ async def _bad_query(**kwargs: Any) -> AsyncIterator[Any]: assert "$ld:ai:graph:invocation_failure" in track_calls + # Measured against Agent SDK 0.3.220: an Anthropic API failure arrives as a `success` result + # with `is_error` set and the error text in `result`. The adapter used to return that text as + # the answer, and it ignored error subtypes entirely. + @pytest.mark.asyncio + async def test_root_api_error_result_fails_the_graph(self) -> None: + track_calls: list[str] = [] + mock_ld_client = MagicMock() + mock_ld_client.track = MagicMock( + side_effect=lambda evt, ctx, data, val: track_calls.append(evt) + ) + mock_sdk = _make_sdk_mock() + error_msg = _make_result_msg( + "Credit balance is too low", 0, 0, is_error=True, api_error_status=400 + ) + + async def _query(**kwargs: Any) -> AsyncIterator[Any]: + yield error_msg + + mock_sdk.query = _query + mock_sdk.ResultMessage = type(error_msg) + ctx = {"kind": "user", "key": "test"} + + with patch( + "importlib.import_module", + side_effect=lambda n: ( + mock_sdk if n == "claude_agent_sdk" else __import__(n) + ), + ): + with patch.object(_claude_ng, "get_client", return_value=mock_ld_client): + with pytest.raises(RuntimeError, match="Credit balance is too low"): + await to_claude_agents( + _make_def_promise(_make_graph_def()), + opts={"context": ctx}, + ).invoke("hi") + + assert "$ld:ai:graph:invocation_failure" in track_calls + assert "$ld:ai:graph:invocation_success" not in track_calls + + @pytest.mark.asyncio + async def test_root_error_subtype_fails_the_graph(self) -> None: + mock_sdk = _make_sdk_mock() + error_msg = _make_result_msg( + None, + subtype="error_max_turns", + is_error=True, + errors=["turn limit reached"], + ) + + async def _query(**kwargs: Any) -> AsyncIterator[Any]: + yield error_msg + + mock_sdk.query = _query + mock_sdk.ResultMessage = type(error_msg) + + with patch( + "importlib.import_module", + side_effect=lambda n: ( + mock_sdk if n == "claude_agent_sdk" else __import__(n) + ), + ): + with pytest.raises( + RuntimeError, match=r"error_max_turns.*turn limit reached" + ): + await to_claude_agents(_make_def_promise(_make_graph_def())).invoke( + "hi" + ) + + @pytest.mark.asyncio + async def test_sub_agent_api_error_fails_its_tool_call(self) -> None: + child_node = { + "key": "child", + "config": {"model": {"name": "claude-3"}, "instructions": "child"}, + "meta": {"variationKey": "v1", "version": 1}, + "edges": [], + "is_terminal": True, + } + root_node = { + "key": "root", + "config": {"model": {"name": "claude-3"}, "instructions": "root"}, + "meta": {"variationKey": "v1", "version": 1}, + "edges": [{"source_key": "root", "target_key": "child"}], + "is_terminal": False, + } + graph_def = _make_graph_def( + nodes={"root": root_node, "child": child_node}, + edges=[{"source_key": "root", "target_key": "child"}], + ) + child_tool_fn: list[Any] = [] + + def _tool_factory(name: str, desc: str, schema: Any) -> Any: + def _dec(fn: Any) -> Any: + if "child" in name: + child_tool_fn.append(fn) + return fn + + return _dec + + mock_sdk = _make_sdk_mock("root-done") + mock_sdk.tool = MagicMock(side_effect=_tool_factory) + error_msg = _make_result_msg( + "Credit balance is too low", 0, 0, is_error=True, api_error_status=400 + ) + + with patch( + "importlib.import_module", + side_effect=lambda n: ( + mock_sdk if n == "claude_agent_sdk" else __import__(n) + ), + ): + await to_claude_agents(_make_def_promise(graph_def)).invoke("hi") + + async def _query(**kwargs: Any) -> AsyncIterator[Any]: + yield error_msg + + mock_sdk.query = _query + mock_sdk.ResultMessage = type(error_msg) + assert child_tool_fn + with pytest.raises(RuntimeError, match="Credit balance is too low"): + await child_tool_fn[0]({"input": "sub-query"}) + @pytest.mark.asyncio async def test_returns_root_query_result(self) -> None: mock_sdk = _make_sdk_mock("expected output")