Wednesday, April 6, 2011

Should C# event handlers be exception safe?

Assume that one event has multiple handlers. If any of event handlers throw exception then remaining handlers are not executed.

Does this mean that event handlers should never throw?

From stackoverflow
  • 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.

  • 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.

  • 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.

    Dmitry Tashkinov : 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.
    Jeffrey L Whitledge : @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).
    hythlodayr : @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.
    Jeffrey L Whitledge : @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.
    Dmitry Tashkinov : @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.
    hythlodayr : @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...
    Jeffrey L Whitledge : @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 : 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.
    Dmitry Tashkinov : @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.
  • 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.

    hythlodayr : And here's one case of why checked exceptions are extremely helpful, me-thinks.
  • 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...
  • 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.

0 comments:

Post a Comment