Skip to content

fix: exit already-entered contexts when Actor or event manager init fails#969

Open
vdusek wants to merge 1 commit into
masterfrom
fix/aenter-failure-cleanup
Open

fix: exit already-entered contexts when Actor or event manager init fails#969
vdusek wants to merge 1 commit into
masterfrom
fix/aenter-failure-cleanup

Conversation

@vdusek

@vdusek vdusek commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Description

A failed __aenter__ never triggers __aexit__, so resources entered earlier inside __aenter__ must be unwound manually before re-raising. Two paths leaked this way:

  • ApifyEventManager.__aenter__ raised on a websocket connection failure without exiting the already-entered parent EventManager, leaving its recurring persist-state task dangling.
  • _ActorType.__aenter__ left the event manager entered when the charging manager's __aenter__ raised.

Both paths now exit the already-entered context before propagating the exception. except BaseException is used deliberately so that a CancelledError during charging manager init also unwinds the event manager.

Testing

  • New regression test for the charging manager failure path (test_failed_charging_manager_init_does_not_leak_event_manager).
  • Extended test_lifecycle_on_platform_without_websocket with assertions that the event manager is inactive and the persist-state task is stopped after the failure.

…ails

A failed `__aenter__` never triggers `__aexit__`, so when the charging manager or the platform
events websocket failed to initialize, the already-entered event manager was left active with its
recurring persist-state task dangling.
@vdusek vdusek added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Jun 11, 2026
@vdusek vdusek self-assigned this Jun 11, 2026
@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.87%. Comparing base (2cc5602) to head (4a0c09f).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #969      +/-   ##
==========================================
- Coverage   89.91%   89.87%   -0.05%     
==========================================
  Files          49       49              
  Lines        3085     3090       +5     
==========================================
+ Hits         2774     2777       +3     
- Misses        311      313       +2     
Flag Coverage Δ
e2e 35.92% <0.00%> (-0.06%) ⬇️
integration 56.92% <33.33%> (-0.06%) ⬇️
unit 78.67% <100.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

@vdusek vdusek marked this pull request as ready for review June 11, 2026 13:53
@vdusek vdusek requested a review from Pijukatel June 11, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants