fix(models): record model-call failures on the provider's own span

[PranavMishra28]

Summary

Span.__exit__ finishes a span without attaching an exception (tracing/spans.py), so a span carries error data only if the provider sets it explicitly. OpenAIResponsesModel does that at both of its span sites. The other three providers do not, at eight sites between them:

file	span	sites
models/openai_chatcompletions.py	generation_span	get_response, stream_response
extensions/models/litellm_model.py	generation_span	get_response, stream_response
extensions/models/any_llm_model.py	response_span	_get_response_via_responses, _stream_response_via_responses
extensions/models/any_llm_model.py	generation_span	_get_response_via_chat, _stream_response_via_chat

A model call that fails through Chat Completions, LiteLLM, or AnyLLM exports a span with no error marker, so in the traces UI a failed turn looks like a normal one.

Two details make this concrete rather than cosmetic:

• any_llm_model.py opens the same response_span as openai_responses.py, so byte-identical span types are error-annotated by one provider and blank by the other.
• openai_chatcompletions.get_response has no try at all, so even the ModelBehaviorError it raises itself for a choice-less payload ("possible provider error payload") escapes with the span unannotated.

Root cause

Error annotation is per-call-site convention rather than a property of opening a model span. Nothing makes it fail loudly when a new provider, or a new branch of an existing one, forgets it.

Approach

Add model_span_errors to util/_error_tracing.py, alongside the existing attach_error_to_span and get_trace_error, and apply it at the eight sites. It is a context manager entered in the same with statement as the span:

with (
    generation_span(...) as span_generation,
    model_span_errors(
        span_generation,
        message="Error getting response",
        trace_include_sensitive_data=tracing.include_data(),
    ),
):

Two consequences worth calling out. The span bodies are not re-indented, so the diff is the with headers plus the helper rather than several hundred lines of moved code. And because the annotation is attached to the span's own scope, it cannot be present in one branch of a provider and missing in another, which is exactly how the current divergence arose. Redaction reuses get_trace_error, so nothing new decides what is safe to record. Messages match the existing convention: "Error getting response" for non-streaming, "Error streaming response" for streaming.

Parenthesized multi-context with is already used in run_internal/model_retry.py, and requires-python is >=3.10.

What this deliberately does not change

• openai_responses.py. It already annotates both spans and is the behavior this matches. Rewriting working code onto the new helper would widen the diff without changing behavior.
• The per-site log_model_action_error calls. Logging is a separate concern from span data; this PR only fixes what the trace records.
• Span.__exit__. Attaching errors there would change every span type in the SDK, not the model providers, and fix(tracing): mark the agent span when a non-streaming run fails #4073 is already addressing agent-span error reporting.
• Every generation_span in openai_responses.py's streaming terminal-event path and other non-provider spans.

Test plan

tests/test_provider_span_errors.py, 9 cases covering all eight sites plus redaction. Each patches the provider's fetch to raise, runs the call inside a trace(), and asserts the span carries the expected error.

• Chat Completions: get_response, stream_response
• Chat Completions: exception text is redacted under ModelTracing.ENABLED_WITHOUT_DATA
• LiteLLM: get_response, stream_response
• AnyLLM, parametrized over all four internal paths (response_span × get/stream, generation_span × get/stream)

All 9 fail on main with generation span carried no error / response span carried no error, and pass with this change.

Verification, in AGENTS.md's mandated order:

make format     # All checks passed
make lint       # All checks passed
make typecheck  # mypy: 0 errors  |  pyright: 0 errors, 836 source files
make tests      # 6232 passed, 4 skipped  /  38 passed, 5 skipped (serial)

Issue number

N/A — searched open and closed issues and PRs for SpanError, set_error, generation_span, response_span, and attach_error_to_span. The nearest is #4073, which fixes under-reported failures on the agent span (run.py, run_internal/) and touches none of these files. No duplicate found.

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

Developed with Claude Code; reviewed and tested by Pranav before marking ready for review.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f40a512bd3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[PranavMishra28]

@seratch since you're already holding #4142, one thing worth saying up front so this doesn't create ordering work for you: these two are fully independent. Zero file overlap (#4142 is run_internal/ plus util/_asyncio_tasks.py, this one is the three provider modules plus util/_error_tracing.py), no shared symbols, and either can land first without touching the other.

The one review question I'd flag, since it's the only judgement call in here: I put the error recording in a context manager (model_span_errors) rather than adding an except block at each of the 8 sites. The reason is that the existing hand-written error handling had already drifted, the Responses path attaches errors and the Chat Completions path doesn't, so 8 independent except blocks would just re-create the same drift later. The tradeoff is that each site's with header grows by four lines. If you'd rather see explicit except blocks I can switch it, it's a mechanical change, but I think the shared invariant is the thing that actually keeps this fixed.

All 9 checks green including both mypy and pyright. The 9 new tests in tests/test_provider_span_errors.py all fail on main, I checked each one against origin/main rather than assuming.

[seratch]

I've verified this against real provider requests and the OpenAI Platform trace backend. The matrix covered a few provider integrations, the base and PR revisions, non-streaming and streaming calls, successful requests, and identical controlled error responses. All traces were exported successfully and appeared in Platform Logs. Successful calls remained unchanged, while the PR correctly added Error getting response or Error streaming response to failed generation spans. We did not observe trace ingestion failures, schema issues, UI rendering problems, or API key exposure.

One change is still required before merging. model_span_errors eagerly evaluates str(error) even when tracing is disabled or sensitive trace data is excluded. If an exception has a broken or side-effecting __str__, this can replace the original provider exception with a new exception from __str__. Please avoid stringifying the exception unless its text will actually be included in the span, and ensure that tracing never changes the exception propagated to the caller. Please also add regression coverage for disabled and redacted tracing that verifies the original exception object is preserved.

[PranavMishra28]

thanks for actually running it against the trace backend, that's more verification than i could do locally and it's useful to know the export side is clean.

you're right about the eager stringify, and it's worse than i realised. reproduced with an exception whose __str__ raises, trace_include_sensitive_data=False:

assert ValueError('__str__ exploded') is BoomStr()

so the caller loses the provider failure entirely and gets my tracing code's exception instead. that's a real regression i introduced, not a theoretical one.

fixed in two parts. the stringify only happens when the text will actually be exported:

def _model_error_text(error: Exception, *, trace_include_sensitive_data: bool) -> str:
    if not trace_include_sensitive_data:
        return REDACTED_TRACE_ERROR_MESSAGE
    try:
        return str(error)
    except Exception:
        logger.warning(...)
        return f"Unrenderable {type(error).__name__}"

and recording as a whole is best-effort now, so a raising __str__, a raising span.set_error, or anything else in the annotate path cannot change what propagates. that's the invariant you asked for stated in one place rather than at each call site.

four regression tests, all failing without the change: redacted tracing preserves the original exception object and never calls __str__ (asserted with a counter, not just the type), sensitive tracing calls it exactly once, a raising __str__ under both settings, and a monkeypatched attach_error_to_span that raises.

on the other finding in your review, the terminal-event one, that's also real and it's not limited to the AnyLLM path. a consumer that stops at response.failed and closes the iterator gets GeneratorExit at the yield, which skips raise terminal_failure_error after the loop, and GeneratorExit is a BaseException so nothing records it. before:

SPANS: [('response', None)]

both Responses streaming paths now record at the point the failure is known rather than at the point it would have been raised.

worth flagging clearly: the OpenAIResponsesModel half of that is pre-existing on main, not something this PR introduced. models/openai_responses.py:594 has the identical shape and the same gap, and i have it covered by test_openai_responses_records_terminal_failure_when_consumer_stops, which fails without the change. i included it because it's the same invariant and the same three-line block, but it does put a file outside the original diff in scope. happy to pull it into its own PR if you'd rather keep this one to the providers that were previously silent, the test comes with it either way.

gate green, both mypy and pyright, 6245 tests. 16 in test_provider_span_errors.py now, 6 of them new.