Skip to content

fix(DI): Ensure dependencies are cleaned up when exception occurrs during cleanup #4148

Merged
provinzkraut merged 3 commits intomainfrom
fix-di-cleanup-group
Apr 26, 2025
Merged

fix(DI): Ensure dependencies are cleaned up when exception occurrs during cleanup #4148
provinzkraut merged 3 commits intomainfrom
fix-di-cleanup-group

Conversation

@provinzkraut
Copy link
Copy Markdown
Member

@provinzkraut provinzkraut commented Apr 26, 2025

Fix a bug where a dependencies' cleanup was not guaranteed to be called would not be called if an exception was thrown:

  • Late during ASGI response rendering
  • During cleanup of another dependency when recovering from an exception

Refactor the code such that:

  • Exceptions are always caught and thrown into dependencies as long as the dependencies are alive
  • If the handler raises an exception and during dependency cleanup other exceptions are raised, the sub-exceptions are grouped into an ExceptionGroup. This behaviour is now equivalent to regular dependency cleanup

@provinzkraut provinzkraut requested review from a team as code owners April 26, 2025 12:57
@github-actions github-actions bot added area/dependencies This PR involves changes to the dependencies area/docs This PR involves changes to the documentation area/kwargs area/private-api This PR involves changes to the privatized API size: small type/bug pr/internal labels Apr 26, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 26, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.37%. Comparing base (0b867f4) to head (8e13188).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #4148   +/-   ##
=======================================
  Coverage   98.37%   98.37%           
=======================================
  Files         348      348           
  Lines       15845    15846    +1     
  Branches     1749     1749           
=======================================
+ Hits        15587    15588    +1     
  Misses        123      123           
  Partials      135      135

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 26, 2025

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/4148

@provinzkraut provinzkraut enabled auto-merge (squash) April 26, 2025 15:20
cofin
cofin approved these changes Apr 26, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@cofin cofin left a comment

Choose a reason for hiding this comment

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

LGTM

@provinzkraut provinzkraut merged commit dcfb674 into main Apr 26, 2025
29 checks passed
@provinzkraut provinzkraut deleted the fix-di-cleanup-group branch April 26, 2025 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cofin cofin cofin approved these changes

Assignees

No one assigned

Labels

area/dependencies This PR involves changes to the dependencies area/docs This PR involves changes to the documentation area/kwargs area/private-api This PR involves changes to the privatized API pr/internal size: small type/bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@provinzkraut @cofin