Don't record pydantic-ai CallDeferred exception or set level to error#1743
Don't record pydantic-ai CallDeferred exception or set level to error#1743Br1an67 wants to merge 4 commits intopydantic:mainfrom
Conversation
Skip recording pydantic-ai's CallDeferred and ApprovalRequired exceptions as errors in logfire spans. These are control flow exceptions used for deferred tool calls and human-in-the-loop approval, not actual errors. Uses lazy import to avoid circular imports between logfire and pydantic-ai. Closes pydantic#1515
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| # pydantic-ai uses CallDeferred and ApprovalRequired as control flow exceptions | ||
| # for deferred tool calls and human-in-the-loop approval. | ||
| # They should not be recorded as errors. | ||
| if _is_pydantic_ai_control_flow_exception(exception): | ||
| return |
There was a problem hiding this comment.
🚩 Incomplete coverage when exception escapes through OTel SDK's use_span (via start_as_current_span)
The fix correctly suppresses error recording for logfire.span(...) context managers (which use LogfireSpan.__exit__ → _LogfireWrappedSpan.record_exception → module-level record_exception). However, when external OTel instrumentation creates spans via _ProxyTracer.start_as_current_span (logfire/_internal/tracer.py:310), the OTel SDK's use_span context manager catches exceptions and calls both span.record_exception(error) and span.set_status(ERROR, ...) independently. The record_exception call is correctly intercepted and skipped, but set_status(ERROR) is called directly by use_span and is NOT gated by the new control flow exception check.
In practice, this likely doesn't matter for the stated use case: pydantic-ai catches CallDeferred/ApprovalRequired internally within its own instrumented spans, so these exceptions shouldn't escape through start_as_current_span-created spans. But if that assumption changes, spans could still get ERROR status without any recorded exception event.
Was this helpful? React with 👍 or 👎 to provide feedback.
…tatus - Add fallback CallDeferred/ApprovalRequired definitions in test_pydantic_ai.py so @pytest.mark.parametrize resolves at module level on Python < 3.10 where pydantic-ai may not be available - Add control flow exception check to set_exception_status() for defense in depth against ERROR status being set on control flow exceptions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ntic versions On pydantic 2.4, importing pydantic-ai exceptions may raise errors other than ImportError (e.g. AttributeError from version incompatibility). Use except Exception to gracefully handle all failure modes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The check in record_exception() already returns early for pydantic-ai control flow exceptions before set_exception_status() is called. The duplicate check was dead code causing coverage to drop below 100%. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 participant
Summary
Skip recording pydantic-ai's
CallDeferredandApprovalRequiredexceptions as errors in logfire spans. These are control flow exceptions used for deferred tool calls and human-in-the-loop approval, not actual errors.Changes
logfire/_internal/tracer.py: Added_is_pydantic_ai_control_flow_exception()helper with lazy import (to avoid circular imports) that checks if an exception isCallDeferredorApprovalRequired. Added an early return inrecord_exception()to skip recording these exceptions entirely — no error status, no error level, no exception event.tests/otel_integrations/test_pydantic_ai.py: Added parametrized test for bothCallDeferredandApprovalRequiredverifying the span has no exception events and no error level.Closes #1515