Skip to content

fix(events): correct the listener invocation, removal, and context release - #2100

Draft
vdusek wants to merge 13 commits into
fix/event-manager-listener-self-awaitfrom
refactor/event-manager-modernization
Draft

fix(events): correct the listener invocation, removal, and context release#2100
vdusek wants to merge 13 commits into
fix/event-manager-listener-self-awaitfrom
refactor/event-manager-modernization

Conversation

@vdusek

@vdusek vdusek commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Stacked on top of #2088 - review that one first.

A polish pass over EventManager and LocalEventManager. No public API changes.

Fixes

  • A listener whose __call__ is async (a class instance, not a function) was treated as sync and handed to asyncio.to_thread, which returned its coroutine without ever awaiting it - the listener silently never ran.
  • off() decided between "remove this listener" and "remove all listeners of the event" by the truthiness of listener, so a listener object that is falsy in a boolean context wiped all listeners of the event.
  • off() left an empty entry behind in _listeners_to_wrappers, keeping a reference to a listener that is no longer registered, and created entries for events and listeners that were never registered at all.
  • A listener that fits neither call shape (it takes two parameters, say) raised out of its own task instead of being logged like any other failing listener. Whether the listener takes the event data is now resolved once at registration, so the invocation itself is fully covered by the logging.
  • __aexit__ released the context only on its happy path. A cancellation or a failing emission left the manager active for good, and since re-entering an active manager is a no-op, PersistState was never emitted again.

pyee is gone

emit creates the listener task itself instead of going through pyee.asyncio.AsyncIOEventEmitter, which:

  • halves the tasks per invocation - the emitter used to schedule a wrapper task that spawned and awaited a second, inner listener task,
  • registers every listener task synchronously in emit, so wait_for_all_listeners_to_complete() can no longer miss the listeners of a just-emitted event - the one-tick defer that fix(events): prevent deadlock when closing or waiting for listeners from within a listener #2088 had to make explicit is not needed anymore,
  • drops a runtime dependency (pyee is still pulled in by playwright for the browser extras).

The _listeners_to_wrappers layout is deliberately kept as it is - apify-sdk-python reaches into it in Actor.reboot(). Its full unit test suite passes against this branch.

The rest

  • Everything that depends only on the listener (whether it takes the event data, sync/async, its name) is resolved once at registration instead of on every invocation, and the wrapping moved out of on() into _wrap_listener.
  • The two __aexit__ branches were collapsed into a single last_exit flag, __aenter__ mirrors it, and the manual "not active" check was replaced by the ensure_context decorator, so the message matches the one every other method raises.
  • The three EventManager.on.listener_wrapper(): ... DEBUG lines are gone - they reported that a task is awaited, that it completed, and that it was discarded.
  • LocalEventManager reads the CPU and the memory info concurrently - get_cpu_info alone blocks its thread for 100 ms while sampling the CPU.
  • Typing: __aenter__ -> Self, set[asyncio.Task[None]], and the emit implementation is annotated as widely as its last overload.
  • wait_for_all_tasks_for_finish renamed to wait_for_all_tasks_to_finish.
  • The listener helpers are private static methods of EventManager, and the attributes of both managers are documented with docstrings instead of comments.

New tests cover every fix above, the synchronous task registration in emit, the nested context teardown, and the concurrent system info readings.

✍️ Drafted by Claude Code

@vdusek vdusek added t-tooling Issues with this label are in the ownership of the tooling team. adhoc Ad-hoc unplanned task added during the sprint. labels Jul 29, 2026
@vdusek vdusek self-assigned this Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.62%. Comparing base (2155162) to head (d432297).

Additional details and impacted files
@@                            Coverage Diff                            @@
##           fix/event-manager-listener-self-await    #2100      +/-   ##
=========================================================================
- Coverage                                  93.64%   93.62%   -0.02%     
=========================================================================
  Files                                        181      181              
  Lines                                      12595    12605      +10     
=========================================================================
+ Hits                                       11794    11801       +7     
- Misses                                       801      804       +3     
Flag Coverage Δ
unit 93.62% <100.00%> (-0.02%) ⬇️

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.

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