diff --git a/CHANGELOG.md b/CHANGELOG.md index 37ee7cf..78fb170 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.13.1] - 2026-09-04 + +### Changed +- **`EventBus::dispatch()` now returns the listener failures it isolated** + (`array`, empty on full success). Isolation was + right — one broken subscriber must not stop the others — but callers had no + way to tell it apart from success, and one of them was a transactional + outbox. A mis-scoped listener threw, the bus swallowed it exactly as designed, + the outbox marked the row dispatched because `dispatch()` had returned + normally, and the row was consumed and never retried: a tenant membership was + lost permanently while the table recorded `status=1, attempts=1, + last_error=NULL`. Adding the value is backward compatible — every existing + `$bus->dispatch($e);` ignores it and behaves exactly as before — but anything + that RECORDS delivery should now check it and re-queue rather than consume. + +### Fixed +- **A listener that could not be resolved was reported as a broken constructor.** + `resolveListener()` caught every container failure and fell back to + `new $listenerClass()`. For a listener with constructor arguments that threw + `ArgumentCountError`, so a `bindInternal()` binding — which the container had + already refused with a `ScopeViolationException` naming the scope, the class + and the fix — was logged as "Too few arguments to function + …::__construct()". Every reader then 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 before. `new` is now attempted only when + it can actually succeed (a constructor with no required parameters); + otherwise the container's own exception is rethrown untouched. + ## [1.13.0] - 2026-09-03 ### Fixed diff --git a/src/Kernel/Events/EventBus.php b/src/Kernel/Events/EventBus.php index d2b1201..8562454 100644 --- a/src/Kernel/Events/EventBus.php +++ b/src/Kernel/Events/EventBus.php @@ -80,15 +80,44 @@ public function subscribe(string $eventName, string $listenerClass): void $this->subscribers[$eventName][] = $listenerClass; } - /** Dispatch an integration event to all subscribers, each in isolation. */ - public function dispatch(IntegrationEventContract $event): void + /** + * Dispatch an integration event to all subscribers, each in isolation. + * + * RETURNS THE FAILURES. Isolation is right — one broken listener must not + * stop the others — but isolation was being read as success, and a caller + * that cannot tell the difference will record one. That is precisely how a + * mis-scoped listener cost a real tenant membership: the listener threw, the + * bus swallowed it exactly as designed, and the transactional outbox then + * marked the row dispatched because dispatch() had returned normally. The + * row was consumed, never retried, and the seat was lost permanently — while + * `status=1, attempts=1, last_error=NULL` said the delivery went fine. + * + * A void return left the caller no way to know, so the fix belongs here. + * Adding the value is backward compatible: every existing + * `$bus->dispatch($e);` keeps working untouched and simply ignores it. An + * outbox, a relay, or anything else that records delivery SHOULD check it + * and re-queue rather than consume — see OutboxRelayService. + * + * Failures are still logged here, so a caller that ignores the return is no + * worse off than before. Note the logger is optional: with none bound there + * is no log line at all, which is the second reason the return value has to + * exist. + * + * @return array listener class => its failure. + * Empty when every subscriber handled the event. + */ + public function dispatch(IntegrationEventContract $event): array { + $failures = []; + foreach ($this->subscribers[$event->name()] ?? [] as $listenerClass) { try { $listener = $this->resolveListener($listenerClass); $listener->handle($event); } catch (\Throwable $e) { // Isolate subscriber failures — never mask the original dispatch. + $failures[$listenerClass] = $e; + $this->logger?->error('EventBus listener failed', [ 'listener' => $listenerClass, 'event' => $event->name(), @@ -97,6 +126,8 @@ public function dispatch(IntegrationEventContract $event): void ]); } } + + return $failures; } /** @@ -116,7 +147,23 @@ public function dispatch(IntegrationEventContract $event): void * a dependency-free listener and a container that cannot resolve it — * so nothing that worked before stops working. * + * AND THE FALLBACK IS NOW CONDITIONAL, because unconditionally it destroyed + * the evidence. A listener bound with bindInternal() is unresolvable from + * outside its module scope, so the container threw ScopeViolationException — + * naming the real fault exactly. That was caught here, discarded, and + * replaced with `new`, which threw ArgumentCountError; dispatch() logged + * "Too few arguments to function …::__construct()". Every reader then went + * looking at the listener's constructor, which was fine, instead of at the + * binding, which was not. The third time this shape cost a production seat + * was enough. + * + * So `new` is attempted ONLY when it can actually work — a constructor with + * no required parameters. Otherwise the container's own exception is + * rethrown untouched, and the log finally names the cause. + * * @param class-string $listenerClass + * + * @throws \Throwable the container's failure, when `new` cannot substitute */ private function resolveListener(string $listenerClass): object { @@ -126,10 +173,42 @@ private function resolveListener(string $listenerClass): object if (is_object($listener)) { return $listener; } - } catch (\Throwable) { - // Not bound and not autowirable here — fall through. + + // Bound to a non-object. `new` below is still worth a try, but there + // is no container exception to rethrow, so synthesise the cause. + $containerFailure = new \RuntimeException(sprintf( + 'Container resolved [%s] to a %s, not a listener object.', + $listenerClass, + get_debug_type($listener), + )); + } catch (\Throwable $e) { + $containerFailure = $e; + } + + if ($this->needsConstructorArguments($listenerClass)) { + throw $containerFailure; } return new $listenerClass(); } + + /** + * Would `new $listenerClass()` fail for want of arguments? + * + * A missing or unreadable class is reported as "no arguments needed" so the + * `new` below still runs and throws the honest Error about the class itself + * — reflection's complaint would only add a layer. + * + * @param class-string $listenerClass + */ + private function needsConstructorArguments(string $listenerClass): bool + { + try { + $constructor = (new \ReflectionClass($listenerClass))->getConstructor(); + } catch (\ReflectionException) { + return false; + } + + return $constructor !== null && $constructor->getNumberOfRequiredParameters() > 0; + } }