ekuefler / gwt-supereventbus

High-octane event bus for GWT
https://ekuefler.github.io/gwt-supereventbus
Apache License 2.0
39 stars 2 forks source link

Exception handling #10

Open jnehlmeier opened 10 years ago

jnehlmeier commented 10 years ago

Hi,

currently evaluating your EventBus implementation to see if I would prefer it to gwteventbinder and currently I am wondering if there is any reason why you provide a separate exception handling mechanism for the event bus?

You could have also used GWT's UmbrellaException to record exceptions occurred during event processing and then post that single UmbrellaException to GWT.getUncaughtExceptionHandler() if that handler has been set. During DevMode that handler is automatically set and logs to console.

Personally I believe that an event should never cause any exception and if it does it is totally unexpected. Well and for all unexpected exceptions I have an UncaughtExceptionHandler in place anyways that, when in production mode, sends them to the server along with additional information that help reproduce/debug the issue afterwards.

I can only hardly see any benefit in adding exception handlers to the event bus and in turn being forced to write more code just to be able to redirect these exceptions to the app's UncaughtExceptionHandler.

So any reason why the EventBus should have its own exception handling mechanism?

ekuefler commented 10 years ago

Thanks for the question. I agree that event handlers shouldn't generally through their own exceptions, and when it happens there isn't usually much to do other than die and maybe log an error.

SuperEventBus is designed to have a similar API to Guava's EventBus, which uses a similar method of registering exception handlers on the event bus (https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/eventbus/EventBus.java#325). One reason I'm avoiding UmbrellaException is that I want to associate additional information with each exception, including the even being fired and the object to which it was being dispatched, and it seems awkward to have an additional layer of wrapping. I imagine some people will find this additional information useful for logging purposes.

The goal for this library is mostly to provide as many features and as much flexibility as possible and see what people figure out to do with it. So I think it's reasonable to provide a flexible exception reporting mechanism here, as it only takes a couple lines in setup to hook it back into GWT's uncaught exception handler. Does the current strategy actually prevent you from doing something you want to do, or are you just concerned about the extra code required to set it up?

jnehlmeier commented 10 years ago

I am just concerned about the extra code.

Currently I have to conditionally add my ExceptionHandler that routes to GWT's UncaughtException handler because you conditionally add a default ExceptionHandler in the EventBus constructor and I don't want to have exceptions logged twice during DevMode. Then I have to write a scheduleFinally() command to accumulate all exceptions occurred during event handling because I don't know how many have occurred and I only want to do a single server request for logging/storing. Because of this I actually like UmbrellaException because GWT makes very good use of it and until now I wasn't in the need of writing such a finally command.

I think it would align better with GWT if event bus would group all exceptions itself and then only do a single notification if anything went wrong. UmbrellaException would be the best choice then because people probably already have code that can inspect UmbrellaException if they are interested in it.

BTW: Guava only allows a single exception handler which seems to be sufficient. It would also be nice to have an additional constructor in your event bus that takes a default exception handler (if you want to stick with your own implementation) and unconditionally adds it to the event bus.