1

I have a problem in a Symfony 4.4 application where there are a dozen listeners listening to a single event that extends Symfony\Contracts\EventDispatcher\Event. The problem is that if one of the listeners throws an exception, then none of the listeners that come after it are executed.

I was looking for a way to catch exceptions from listeners so that this doesn't occur. I tried extending Symfony\Component\EventDispatcher\EventDispatcher and overriding the callListeners() method to include a try/catch so that I could log the exception but continue the execution. But I don't know how to tell Symfony to use my EventDispatcher instead of its own.

I don't know if that's even a recommendable way of solving this problem. Any idea of how I could get this to work, or if there are any other alternatives?

yivi
  • 42,438
  • 18
  • 116
  • 138
Chris
  • 4,277
  • 7
  • 40
  • 55
  • @yivi if we had asynchronous event handling we wouldn't have a problem, but we're not there yet. But each listener does a different job that should be completely independent from the others. It's not that we don't want exceptions to occur when there's a problem. It's just that an exception in one listener shouldn't stop all the other listeners that need to do their tasks. For example if we have an order event and a payment processing listener and an email notification listener, the payment processing shouldn't stop just because there was an error sending the email notification. – Chris Aug 12 '21 at 10:00
  • 1
    Again, looks like you are using the event dispatcher for something it's not very well suited. Considering what you describe, you should either be looking at something like Symfony Messenger or Tactician and use real async handling, or simply not use exceptions to control flow within event dispatcher listeners. – yivi Aug 12 '21 at 10:02
  • You can often override core classes by adding a compiler pass, grabbing the service definition and then setting a new class. It would be interesting to see what sort of unexpected side effects occur. https://stackoverflow.com/questions/62722129/changing-default-user-object-for-in-memory-provider/62729515#62729515 – Cerad Aug 12 '21 at 13:31
  • Thanks @Cerad, I'll look into it! I understand as @yivi mentioned that what we're trying to achieve is perhaps unusual and perhaps not recommendable, but that's the situation we're currently in. For the time being we've decided to try handling events with `SimpleBus` and its `EventBus` instead of Symfony's `EventDispatcher`. The reason being that this allows us to easily use a middleware to catch and log exceptions without stopping the rest of the listeners. Later we'll have to rethink our approach but for now this will work. – Chris Aug 12 '21 at 14:49

1 Answers1

0

If an exception is thrown, it should either be handled or the execution stopped. That's what exceptions are for.

And the Event Dispatcher component is not meant to be used asynchronously, with fully decoupled listeners. It's executed synchronously by design, had no transport support, and all listeners run on the same request/execution thread.

If one throws an Exception and it's not handled, execution should stop and never reach the next listener.

Even if you were using a fully async solution, like Symfony Messenger with a transport, throwing an exception and not handling it would prevent the execution of any other listeners to the same event.

I think you are using the wrong tools for the job.

Your listeners should not be throwing exceptions if you do not want to stop execution. Handle the exceptions in the listener, and use the opportunity to log, etc.

yivi
  • 42,438
  • 18
  • 116
  • 138
  • I understand your point, and perhaps I am using the wrong tool for the job. But I don't know if I entirely agree with you that all exceptions should be handled in the listener. That would imply wrapping all listener methods in a `try {} catch(\Exception $e) {}` to ensure that no unexpected error prevents other important listeners from doing their job. I also don't know if I agree about async solutions. If I for example queue each listener so they're handled on separate requests, an exception stopping one listener request would not affect any other. – Chris Aug 12 '21 at 14:44
  • 1
    In the end, the solution you say you are using in your [comment here](https://stackoverflow.com/questions/68754840/capturing-exceptions-from-event-listeners-in-symfony-so-that-they-dont-stop-oth/68755946?noredirect=1#comment121517925_68754840), is in line with my advice. Better switch to a tool better suited to the task at hand, instead of trying to force the wrong. And I didn't say that all exceptions should be handled, simply that all exceptions should be handled as long as you want execution to remain uninterrrupted. That's simply the way the language works. – yivi Aug 12 '21 at 15:43
  • Well to put things into perspective our problem arose from an event called during a cron task, and various listeners listened to it. But one of the first listeners failed with an unexpected exception, and then none of the others were called. We saw this error in our logs but deemed it low priority and took our time fixing it, not realizing that it was stopping some very important listeners from doing their job. The exception was unexpected and the only way to have avoided the problem was wrapping the entire listener method in a very generic try/catch. This could happen with any listener. – Chris Aug 12 '21 at 19:56
  • 1
    Indeed, I have several jobs like that. And the solution is to handle the possible exceptions if one need to execution to be resilient. It's very, very rare a method or function can throw an **undocumented** exception. If an exception is meant to stop execution, you can let it be, unhandled. Otherwise, handle it and log whatever you need there. – yivi Aug 15 '21 at 09:11