Skip to content

fix: Add cycle detection to exceptions_from_error#5880

Merged
ericapisani merged 5 commits intomasterfrom
ep/py-1949-recursive-fastapi-exception-m4q
Mar 30, 2026
Merged

fix: Add cycle detection to exceptions_from_error#5880
ericapisani merged 5 commits intomasterfrom
ep/py-1949-recursive-fastapi-exception-m4q

Conversation

@ericapisani
Copy link
Copy Markdown
Member

@ericapisani ericapisani commented Mar 26, 2026

Summary

  • Add cycle detection to exceptions_from_error() to prevent infinite recursion when exception chains contain cycles
  • This fixes a production issue where FastAPI + multiple BaseHTTPMiddleware instances cause Starlette's collapse_excgroups() to create cyclic __context__ references (ExceptionGroup → inner exception → __context__ → ExceptionGroup)
  • Without this fix, the SDK silently drops the event due to a RecursionError

Test plan

  • test_exceptiongroup_starlette_collapse — realistic reproduction of the Starlette collapse_excgroups() pattern with exact expected exception values
  • test_cyclic_exception_group_cause — verifies cycle detection works for __cause__ chains
  • test_deeply_nested_cyclic_exception_group — verifies cycle detection across multiple nested ExceptionGroups

Fixes #5025 and PY-1949

🤖 Generated with Claude Code

ericapisani and others added 2 commits March 25, 2026 17:10
When using FastAPI with multiple BaseHTTPMiddleware instances, anyio
wraps exceptions in ExceptionGroups. When Starlette unpacks and reraises
these, it creates cyclic references (ExceptionGroup -> ValueError ->
__cause__ -> ExceptionGroup) that cause infinite recursion in
exceptions_from_error(), silently dropping the event.

Track seen exception object ids during recursive walks through
__cause__, __context__, and ExceptionGroup.exceptions to break cycles.

Fixes GH-5025
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace test_cyclic_exception_group_context with test_exceptiongroup_starlette_collapse
that simulates the actual collapse_excgroups() pattern from Starlette. This provides
a more realistic regression test for the cycle detection in exceptions_from_error()
by reproducing the exact __context__ cycle that occurs in production with FastAPI
and multiple BaseHTTPMiddleware instances.

Remove the now-redundant synthetic __context__ cycle test since the new test
covers the same code path with a more representative scenario.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@linear-code
Copy link
Copy Markdown

linear-code bot commented Mar 26, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

Langchain

  • Set gen_ai.operation.name and gen_ai.pipeline.name on LLM spans by ericapisani in #5849
  • Broaden AI provider detection beyond OpenAI and Anthropic by ericapisani in #5707
  • Update LLM span operation to gen_ai.generate_text by ericapisani in #5796

Other

Bug Fixes 🐛

Ci

  • Update validate-pr action to remove draft enforcement by stephanie-anderson in #5918
  • Use gh CLI to convert PR to draft by stephanie-anderson in #5874
  • Use GitHub App token for draft PR enforcement by stephanie-anderson in #5871

Openai

  • Always set gen_ai.response.streaming for Responses by alexander-alderman-webb in #5697
  • Simplify Responses input handling by alexander-alderman-webb in #5695
  • Use max_output_tokens for Responses API by alexander-alderman-webb in #5693
  • Always set gen_ai.response.streaming for Completions by alexander-alderman-webb in #5692
  • Simplify Completions input handling by alexander-alderman-webb in #5690
  • Simplify embeddings input handling by alexander-alderman-webb in #5688

Other

  • (google-genai) Guard response extraction by alexander-alderman-webb in #5869
  • (workflow) Fix permission issue with github app and PR draft graphql endpoint by Jeffreyhung in #5887
  • Add cycle detection to exceptions_from_error by ericapisani in #5880

Documentation 📚

  • Update CONTRIBUTING.md with contribution requirements and TOC by stephanie-anderson in #5896

Internal Changes 🔧

Ai

  • Remove unused GEN_AI_PIPELINE operation constant by ericapisani in #5886
  • Rename generate_text to text_completion by ericapisani in #5885

Langchain

  • Add text completion test by alexander-alderman-webb in #5740
  • Add tool execution test by alexander-alderman-webb in #5739
  • Add basic agent test with Responses call by alexander-alderman-webb in #5726
  • Replace mocks with httpx types by alexander-alderman-webb in #5724
  • Consolidate span origin assertion by alexander-alderman-webb in #5723
  • Consolidate available tools assertion by alexander-alderman-webb in #5721

Openai

  • Replace mocks with httpx types for streaming Responses by alexander-alderman-webb in #5882
  • Replace mocks with httpx types for streaming Completions by alexander-alderman-webb in #5879
  • Move input handling code into API-specific functions by alexander-alderman-webb in #5687

Other

  • (asyncpg) Normalize query whitespace in integration by ericapisani in #5855
  • 🤖 Update test matrix with new releases (03/30) by github-actions in #5912
  • Merge PR validation workflows and add reason-specific labels by stephanie-anderson in #5898
  • Add workflow to close unvetted non-maintainer PRs by stephanie-anderson in #5895
  • Exclude compromised litellm versions by alexander-alderman-webb in #5876
  • Reactivate litellm tests by alexander-alderman-webb in #5853
  • Add note to coordinate with assignee before PR submission by sentrivana in #5868
  • Temporarily stop running litellm tests by alexander-alderman-webb in #5851

Other

  • ci+docs: Add draft PR enforcement by stephanie-anderson in #5867

🤖 This preview updates automatically when you update the PR.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

Codecov Results 📊

13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 12.48s

📊 Comparison with Base Branch

Metric Change
Total Tests
Passed Tests
Failed Tests
Skipped Tests

✨ No test changes detected

All tests are passing successfully.

❌ Patch coverage is 50.00%. Project has 14688 uncovered lines.
✅ Project coverage is 30.08%. Comparing base (base) to head (head).

Files with missing lines (1)
File Patch % Lines
utils.py 52.15% ⚠️ 446 Missing and 79 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    25.27%    30.08%    +4.81%
==========================================
  Files          189       189         —
  Lines        20998     21007        +9
  Branches      6854      6862        +8
==========================================
+ Hits          5306      6319     +1013
- Misses       15692     14688     -1004
- Partials       432       477       +45

Generated by Codecov Action

@ericapisani
Copy link
Copy Markdown
Member Author

bugbot run

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

ericapisani and others added 2 commits March 26, 2026 13:51
Type seen_exceptions as Optional[list[BaseException]] instead of Optional[list]
and split the None checks for seen_exceptions and seen_exception_ids so each
parameter is independently initialized.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ericapisani ericapisani marked this pull request as ready for review March 26, 2026 13:03
@ericapisani ericapisani requested a review from a team as a code owner March 26, 2026 13:03
   exceptions_from_error

Add docstring details distinguishing mechanism.exception_id (sequential
   counter) from Python id() values used for cycle detection, and
   explaining parent_id's role in the event payload.
Copy link
Copy Markdown
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ty

@ericapisani ericapisani merged commit 7516309 into master Mar 30, 2026
158 checks passed
@ericapisani ericapisani deleted the ep/py-1949-recursive-fastapi-exception-m4q branch March 30, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix recursive FastAPI exceptions

2 participants