-
Notifications
You must be signed in to change notification settings - Fork 138
fix(rtc): emit events to handlers in registration order - #802
fix(rtc): emit events to handlers in registration order #802avionicharshit-byte wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
✅ Devin Review: No Issues Found
Devin Review analyzed this PR and found no bugs or issues to report.
1c40019 to
50a92d6
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
4dae24d to
c6ece0e
Compare
ff9b6b9 to
4c952be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Devin Review found 2 new potential issues.
⚠️ 2 issues in files not directly in the diff
⚠️ Fork pull requests cannot generate stubs
On fork pull requests, actions/checkout searches the base repository for the contributor's branch instead of checking out the fork. Both RTC and protocol generation fail when that branch does not exist in the base repository.
⚠️ Generated changes break fork checks
When generation changes stubs in a fork pull request, EndBug/add-and-commit pushes with a read-only workflow token. Both RTC and protocol generation fail instead of validating the contributor's changes.
fixes #778, and the approach is the one
404n0signalproposed there.EventEmitterkept handlers in aset, soemit()dispatched in hash order rather than registration order. A handler that mutates the emitted object and one that reads it therefore race, and since handler identities are stable the winner is decided once per process: whole sessions come out one way, whole sessions the other.AgentActivity._on_metrics_collectedin livekit-agents is the live case, it stampsspeech_idonto the metrics object before re-emitting, so a user handler on the same event sees the id orNonefor the entire run. Nothing raises and nothing logs, the metrics are just unkeyed._eventsnow maps each event to adictkeyed by handler. Insertion-ordered, still O(1) add, remove and dedup, soon/off/ duplicate registration behave exactly as before.the two ordering tests use handlers with fixed hashes, so a
setis guaranteed to yield them in the opposite order to the one they were registered in. without that they pass on main by luck: three consecutively defined functions happen to land in the set in order. on main the suite is 2 failed, 6 passed,At index 0 diff: None != 'speech_1'; with the change, 8 passed.