Skip to content

Add HandlerFailedException to CommandBus::dispatch @throws#874

Open
tuxes3 wants to merge 1 commit into
patchlevel:3.21.xfrom
tuxes3:3.21.x
Open

Add HandlerFailedException to CommandBus::dispatch @throws#874
tuxes3 wants to merge 1 commit into
patchlevel:3.21.xfrom
tuxes3:3.21.x

Conversation

@tuxes3

@tuxes3 tuxes3 commented Jul 14, 2026

Copy link
Copy Markdown

Without declaring it, catching the exception forces @phpstan-ignore catch.neverThrown everywhere, which is a bad pattern.

Without declaring it, catching the exception forces `@phpstan-ignore catch.neverThrown` everywhere, which is a bad pattern.

Signed-off-by: Nicolo Singer <nicolo@whatwedo.ch>
@DanielBadura

Copy link
Copy Markdown
Member

This is the wrong place, since this interface has no knowledge of the bundle's Symfony implementation.

In the library's minimal implementation, we're already throwing different exceptions:
https://github.com/patchlevel/event-sourcing/blob/3.21.x/src/CommandBus/SyncCommandBus.php#L32-L47

So this should most likely be handled in the bundle instead:
https://github.com/patchlevel/event-sourcing-bundle/blob/3.18.x/src/CommandBus/SymfonyCommandBus.php#L18-L26

One thing to keep in mind is that we're already unwrapping the HandlerFailedException there when it contains a previous exception. Since its symfonys bus interface declares that it throws ExceptionInterface, that is the exception that should we should use.

Not sure though, if it would resolve the phpstan issue. You could try it out in your local checkout.

@tuxes3

tuxes3 commented Jul 14, 2026

Copy link
Copy Markdown
Author

@DanielBadura

I understand. So what would be the correct way to either fix it in the bundle or in our setup? We use the symfony bundle with following setup:

framework:
    messenger:
        buses:
            command.bus: ~

patchlevel_event_sourcing:
    command_bus:
        service: command.bus

I just debugged it live:

Screenshot 2026-07-14 at 13 12 05

It seems that the unwrap logic does not work as excepted... Could we just use array_first ? or what would be your approach to fix that? I can open another PR after

@tuxes3

tuxes3 commented Jul 14, 2026

Copy link
Copy Markdown
Author

@DanielBadura anyway, all of this would still not fix the phpstan Error. Could we add a generic @throws \Throwable to the CommandBus?

@DanielBadura

Copy link
Copy Markdown
Member

Most probably we could but first I would like to check why it does not occur before 🤔 . Are you injecting our CommandBus interface, or are you using the MessageBusInterface from Symfony to inject the bus?

I just tried in a project of mine, catching an HandlerFailedException when dispatching a command. Both variants, using our interface and using the symfony one. I did not receive any issue from phpstan on level max. Do you have some other configuration for phpstan?

    public function __construct(
        private readonly CommandBus $commandBus,
    ) {
    }

    public function __invoke(Request $request, string $id): Response
    {
            try {
                $this->commandBus->dispatch(new Command(...));
            } catch (HandlerFailedException $e) {
				// do something
            }

		// ...
    }

Regarding the not unpacking: Yeah, it seems it was changed from index based to an hashmap? Need to check that and fix it. Using array_first is only possible in 8.5, so most probably using array_values()[0] it is.

@tuxes3

tuxes3 commented Jul 14, 2026

Copy link
Copy Markdown
Author

@DanielBadura I am using Patchlevel\EventSourcing\CommandBus\CommandBus and phpstan as is with some unrelated extensions at Level 6 🤔 I cannot explain why it does not show the error on your side ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants