21

Assuming that one event has multiple handlers, if any of event handlers throw an exception then the remaining handlers are not executed.

Does this mean that event handlers should never throw?

Lucas
  • 16,930
  • 31
  • 110
  • 182
Marko
  • 5,437
  • 4
  • 28
  • 47

6 Answers6

15

Since invoking an event means that the caller has no knowledge about the callee:

  1. Invoking an event handler should be robust in the face of arbitrary exceptions. Everything up the call stack needs to clean up its own mess correctly, in case something completely unexpected occurs.

  2. Event handlers should really avoid throwing exceptions.

Things like null reference exceptions are really inexcusable in any code, so obviously we aren't concerned about that.

Things like file IO exceptions can always happen when writing or reading a file, so I would avoid ever doing IO within an event handler. If it makes sense to do IO in an event handler, then it also makes senst to handle the IO exceptions within the handler too. Don't propogate that back to the caller. Find some way to deal with it.

Jeffrey L Whitledge
  • 58,241
  • 9
  • 71
  • 99
  • When you say in point 1. that "Invoking an event handler should be robust in the face of arbitrary exceptions", do you mean that we should put a try catch arount the event invocation (since we don't have a way to know what the subscribers will do)? Or do you mean that it is really the responsibility of each of the event handlers to guarantee that they do not throw? – Jorge Galvão Jun 17 '19 at 17:41
  • @JorgeGalvão Point 1 is that anything that invokes an event should expect any sort of exception from that event. In practice, it really depends on the situation. Possibly catching the exceptions or just making sure the process ends with the right sort of error message. Point 2 was the converse, that it really is the responsibility of event handlers to guarantee that they do not throw. Point 1 is really for public APIs where you have no control over what the event handlers are going to do. Point 2 is for all situations where it is possible to avoid generating exceptions. – Jeffrey L Whitledge Jun 17 '19 at 18:18
  • I understand what you are saying. But catching an exception otside the event handlers might be wothless: the sole fact that some event haldlers might not have been called because one of them threw an exception might be a fatal exception, in the sense that it is expected for the events to be in fact called and one should be able to rely on that. And the only way to rely on that is if indeed no handler throws an exception. And this is true for public API too (in my opinion). – Jorge Galvão Jun 18 '19 at 23:10
  • In short, I argue that try-catching outside the handlers is probably worthless, since the most problematic effect will not be avoided. – Jorge Galvão Jun 18 '19 at 23:17
  • @JorgeGalvão I agree 100%. Catching an unexpected exception is hardly ever the correct behavior. – Jeffrey L Whitledge Jun 19 '19 at 17:58
9

In an ideal world, yes. It's a good idea to try to design event handlers so that they:

  • Don't throw exceptions
  • Execute very quickly

Not doing so will result in unexpected side effects, since other subscribers to the event may never receive messages, or receive them very late.

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
5

You should NOT swallow exceptions in an event handler, but neither would I generally recommend throwing them. The problem is that there's generally no good place to catch exceptions thrown from within event handlers.

The design pattern of event handlers leads to some potential problems because they are both indirectly invoked and can be multicast. Since they are indirect, you have to be careful and plan where you can catch exceptions they may throw. Since they are multicast, an exception in a handler may result in other handler never receiving the event.

LBushkin
  • 129,300
  • 32
  • 216
  • 265
4

Why are event handlers are not like other patterns? What's the difference? Exception means that a subprogram tells to a caller that it can not work as it is designed to work. This means that the caller must do something with it. If a handler is performing some processing vital for the use case, then it is sensible that when it crushes, the rest of the program must stop.

Dmitry Tashkinov
  • 1,976
  • 19
  • 16
2

It's tempting to do this but ultimately, it depends on the situation.

  • If you want the program to crash rather than run the risk of running in a bad or weird state, then by all means throw an exception.
  • If you REALLY don't care (think hard about this one) and only need to note the exception, then wrapping in try/catch blocks make perfect sense.
  • If you want to give all of the event-handlers a chance to run before crashing (e.g, maybe some of the handlers release resources), then it requires a bit more effort...
hythlodayr
  • 2,377
  • 15
  • 23
1

In an ASP.NET application, I'd just let the event handlers throw exceptions. Those exceptions will then allow the custom error page functionality of ASP.NET to work.

In a desktop application, I always add a try/catch/finally block around the guts of any event handler. This way, any exceptions are always logged and displayed to the end-user, but never escape the event handler, so they don't crash the application.

John Saunders
  • 160,644
  • 26
  • 247
  • 397
  • Look at this scenario. One event handler failed and the exception was logged. The next handler was called. It inferred that program had worked correct so far, but in fact it has to deal with, for example, outdated data because it hadn't been properly updated by the previous handler. I think there's no simple rule. It depends on what event handlers do. – Dmitry Tashkinov Feb 24 '10 at 19:50
  • 5
    @Dmitry Tashkinov - Without commenting on the main question, your scenario seems to indicate that there is some implicit ordering to the event handlers. Depending on one handler to run before another smells like a bad design to me. One handler swallowing an exception shouldn't affect another handler (though it may affect the invoking code). Rather an invalid program state should cause the other handlers to either throw an exception (if they care about that part of the program state) or work fine (if they don't). – Jeffrey L Whitledge Feb 24 '10 at 20:01
  • @Jeffrey - The handlers aren't necessarily going to pick-up on the problem unless coded for (i.e., keeping track of state), because they may be performing vastly different tasks for the same event. – hythlodayr Feb 24 '10 at 20:08
  • @hythlodayr - But what I am saying is that the *sequence* in which event handlers are called should typically not be relied on. Each handler should be independent of the others. If they are performing vastly different tasks, then they shouldn't care about the success or failure of the other handlers. – Jeffrey L Whitledge Feb 24 '10 at 20:11
  • @Jeffrey - it doesn't necesserily means an order. It may mean other things. For example, one handler updates a master panel and another - a details panel. Selected item changed. One handler worked, another humbly failed, and the selected item on the master panel does not correspond to what is being showed on the details panel. – Dmitry Tashkinov Feb 24 '10 at 20:17
  • @Jeffrey - Absolutely on sequence. Dependency on sequence in one bit of code, makes it extremely annoying (or near-impossible, in production) to replace one component with another. But while the example wasn't so great, I think one should swallow exceptions with caution... – hythlodayr Feb 24 '10 at 20:21
  • @Dmitry Tashkinov - Are you suggesting that the portion which worked correctly should not have been allowed to do so? The user has already been told "Could not update the master panel. Please remove the obstruction." In most cases I can think of, master and details panels disagreeing with each other is a lesser problem than both panels agreeing on the wrong information. Perhaps alternative situations could be concocted, but, in general, events should not be counted on to stand or fall together. And if we are counting on it, what happens when the failing event happens after the successful one? – Jeffrey L Whitledge Feb 24 '10 at 20:27
  • continued... I certainly agree that swallowing arbitrary exceptions is a bad idea, even in an event handler. This is why I put the part about "Please remove the obstruction." If updating the master panel failed because of an out of bounds array reference, or something like that, then program should fail immediately in a blaze of glory. But, sadly, those aren't the only kinds of exceptions that are possible. – Jeffrey L Whitledge Feb 24 '10 at 20:33
  • @Jeffrey - The idea was that even if the sequence doesn't matter, an exception in only one event handler can result in an inconsistency in the whole program. And I think it's not always an exclusive responsibility of the handler to decide if it is fatal or not. Just read your last comment. Cannot argue with this. – Dmitry Tashkinov Feb 24 '10 at 20:39
  • @DmitryTashkinov: If a situation which causes an exception might leave an object in an unsafe state, perhaps the solution would be for the object in question to have an "DangerState" field. Before putting the object into an unsafe state, test and set the field (if already set, throw an exception). When the object is successfully restored to a safe state, clear the field. I would consider this an especially nice pattern for locks: pair locks with a "safe/unsafe" field; when acquiring a lock, check to ensure the locked resource is "safe" and throw if not. If an exception occurs... – supercat Jan 05 '12 at 16:34
  • ...while the resource is "unsafe", release the lock but leave the flag in the "unsafe" mode. This will cause other routines that try to enter the lock to throw an exception rather than deadlocking. – supercat Jan 05 '12 at 16:35
  • @supercat: when the object is in an unsafe state, it's difficult to reason about what operations are safe to perform on it, event test-and-set on a field. – John Saunders Jan 05 '12 at 17:58
  • @JohnSaunders: A field which exists purely for the purpose of indicating whether an object is in a known-clean or potentially-corrupted state is not hard to reason about. If it holds the expected value at the expected time, the object is clean. If it holds anything else, the object is potentially corrupted and should no longer be used. Keeping track of such a thing would be much nicer if vb.net/C# either had a construct which was similar to "using", but for an interface that included a method to indicate normal vs abnormal exit, or else provided a means for a Dispose method to do so. – supercat Jan 05 '12 at 18:35