Skip to content

fix(events): open the platform websocket only once per context - #1082

Merged
vdusek merged 1 commit into
masterfrom
fix/event-manager-nested-context
Aug 6, 2026
Merged

fix(events): open the platform websocket only once per context#1082
vdusek merged 1 commit into
masterfrom
fix/event-manager-nested-context

Conversation

@vdusek

@vdusek vdusek commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

ApifyEventManager ignored the re-entrancy contract of Crawlee's EventManager, which tracks active contexts with _active_ref_count. BasicCrawler._run_crawler always enters the global event manager, and Actor.init registers the ApifyEventManager as exactly that and has already entered it — so on the platform every crawler run re-entered it and opened a second websocket.

Measured on master, with a local events server and a BasicCrawler.run() inside an entered ApifyEventManager:

  • Two connections stay open for as long as the run lasts, so every platform event arrives twice. One Migrating became two Migrating listener calls and two PersistState(is_migrating=True) — state persisted twice per migration. The extra connection also counts against the platform limit of 10 per run.
  • The crawler's exit tears down that newer connection, which leaves __aexit__ bookkeeping pointing at an already-closed one. The Actor's own connection and its message-processing task therefore survive Actor.exit(), re-breaking the iterator shutdown from fix: close the platform events websocket iterator on shutdown #1077.

The websocket is now owned by the outermost context only, mirroring LocalEventManager. On top of that:

  • The shutdown moved into _teardown_platform_websocket() and the parent context is left in a finally, so a failed shutdown can no longer keep the manager active for good — which would mean never emitting PersistState again.
  • Task, connection and future are reset on exit, so the context can be entered again.
  • A cancelled entry cleans up as well. Without it, a later entry mistook itself for a nested context and silently returned a manager that received no platform events at all.
  • The error that prevented the first connection is raised as the __cause__ of the RuntimeError, not only logged.
  • Actor.reboot() reads _listeners_to_wrappers with get, so the lookup no longer inserts entries for events nobody listens to.

Adopts the patterns from apify/crawlee-python#2100 (Crawlee 1.9.1).

✍️ Drafted by Claude Code

@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 Aug 6, 2026
@vdusek vdusek self-assigned this Aug 6, 2026
@github-actions github-actions Bot added this to the 146th sprint - Tooling team milestone Aug 6, 2026
@github-actions github-actions Bot added the tested Temporary label used only programatically for some analytics. label Aug 6, 2026
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.23%. Comparing base (7d23ecd) to head (e3518a7).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1082      +/-   ##
==========================================
+ Coverage   92.20%   92.23%   +0.03%     
==========================================
  Files          51       51              
  Lines        3231     3247      +16     
==========================================
+ Hits         2979     2995      +16     
  Misses        252      252              
Flag Coverage Δ
e2e 35.20% <2.63%> (-0.15%) ⬇️
integration 56.63% <2.63%> (-0.25%) ⬇️
unit 84.50% <100.00%> (+0.07%) ⬆️

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 requested a review from Mantisus August 6, 2026 11:33
@vdusek
vdusek marked this pull request as ready for review August 6, 2026 11:33

@Mantisus Mantisus left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@vdusek
vdusek requested a review from B4nan August 6, 2026 12:45
@vdusek
vdusek merged commit c471dde into master Aug 6, 2026
31 checks passed
@vdusek
vdusek deleted the fix/event-manager-nested-context branch August 6, 2026 15:46
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. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants