fix(events): stop a failed dispatch from looking like a delivered one - #163
Merged
Merged
Conversation
Two defects, one incident. A listener bound with bindInternal() is unresolvable from outside its module scope, and the EventBus resolves listeners from outside every module scope. That cost a real tenant membership, and neither half of the bus would say so. 1. resolveListener() DESTROYED THE CAUSE. The container refused with a ScopeViolationException naming the scope, the class and the fix. That was caught, discarded, and replaced with `new`, which threw ArgumentCountError — so dispatch() logged "Too few arguments to function …::__construct()". Every reader went to the listener's constructor, which was correct, instead of to the binding, which was not. The same shape had already been misdiagnosed twice. `new` is now attempted only when it can actually succeed — a constructor with no required parameters — which is the case the fallback was always for. Otherwise the container's own exception is rethrown untouched. 2. dispatch() REPORTED SUCCESS FOR AN EVENT NOBODY HANDLED. Isolating a broken subscriber is right; leaving the caller unable to tell isolation from success is not. A transactional outbox marked its row dispatched because dispatch() returned normally, so the row was consumed and never retried and the seat was lost for good — while the table recorded status=1, attempts=1, last_error=NULL. dispatch() now returns the failures it isolated, keyed by listener class, empty on full success. Additive and backward compatible: every existing `$bus->dispatch($e);` ignores it and behaves exactly as before. Anything that RECORDS a delivery should check it and re-queue instead of consuming. Failures are still logged, so a caller that ignores the return is no worse off. The logger stays optional, which is the second reason the return value has to exist: with none bound there was no trace at all. Verified against the original failure — with the broken listener still in place, the outbox row survived as pending and the next relay run assigned the seat.
hakeemRash
requested review from
Alshatri and
craftdevscommunity
as code owners
September 3, 2026 21:13
Alshatri
approved these changes
Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Releases kernel 1.13.1. Two defects in
EventBus, one incident: a listener bound withbindInternal()is unresolvable from outside its module scope, and the bus resolves listeners from outside every module scope. That cost a real tenant membership, and neither half of the bus would say so.resolveListener()destroyed the causeThe container refused with a
ScopeViolationExceptionnaming the scope, the class and the fix. It was caught, discarded, and replaced withnew, which threwArgumentCountError— so the log read "Too few arguments to function …::__construct()". Readers went to the listener's constructor, which was correct, instead of to the binding, which was not. The same shape had been misdiagnosed twice before.newis now attempted only when it can actually succeed (no required constructor parameters) — the case the fallback was always for. Otherwise the container's own exception is rethrown untouched.dispatch()reported success for an event nobody handledIsolating a broken subscriber is right; leaving the caller unable to tell isolation from success is not. A transactional outbox marked its row dispatched because
dispatch()returned normally — the row was consumed, never retried, and the seat was lost for good, while the table recordedstatus=1, attempts=1, last_error=NULL.dispatch()now returns the failures it isolated, keyed by listener class, empty on full success. Additive and backward compatible: every existing$bus->dispatch($e);ignores it and behaves exactly as before. Anything that RECORDS a delivery should check it and re-queue instead of consuming.Verification
Reproduced the original failure with the broken listener still in place: the outbox row survived as
pendinginstead of being consumed, and the next relay run assigned the seat. A standalone harness covers all four paths — mis-scoped arg-taking listener (real cause surfaced, notArgumentCountError), no-arg listener still falling back tonew, container-resolved happy path, and isolation preserved across a broken + healthy subscriber pair.Consumed by
hkm-plugin-user^2.2, whose outbox no longer marks a failed dispatch as delivered.