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 73f55ff9..ac6694e4 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 @@ -36,6 +36,7 @@ from launchdarkly_ai_claude_agents.handler import ( _build_hooks, + _raise_if_run_failed, build_prompt, build_query_prompt, build_tool_mcp, @@ -184,6 +185,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 45c2098f..04117b91 100644 --- a/packages/claude-agents/tests/test_native_graph.py +++ b/packages/claude-agents/tests/test_native_graph.py @@ -22,10 +22,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 @@ -847,6 +858,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")