Provide custom exception handling in EventBus

[gissuebot]

Original issue created by toellrich on 2011-10-20 at 12:05 PM

---

I would love to replace our implementation of a synchronous event bus with the one from Guava. They do pretty much the same thing except for one notable difference: Ours propagates exceptions that happen during event handling since event handling must occur in the same database transaction as the code that raised the event. Is there any chance to make this configurable (flag/subclassing)?

Cheers Thomas

[gissuebot]

Original comment posted by ray.j.greenwell on 2011-11-01 at 12:34 AM

---

What happens when an Exception is thrown? Are subsequent handlers not notified of the event object? That seems unacceptable because there's no way to configure the order of handling.

Or would you want a "public Set<Exception> postAll (Object event)" ?

[gissuebot]

Original comment posted by toellrich on 2011-11-01 at 04:12 AM

---

In our case they would all be transactional and therefore wouldn't need to be notified explicitly because of the rollback.

[gissuebot]

Original comment posted by wasserman.louis on 2011-11-15 at 04:49 PM

---

I concur that it's not at all clear how the semantics of this would work, due to the lack of ordering on handlers. Would all the other handlers still be called?

[gissuebot]

Original comment posted by toellrich on 2011-11-15 at 08:24 PM

---

I would have the dispatching stop similar to the way SimpleEventBus of the CQRS-Framework Axon works (http://www.axonframework.org/docs/1.2/event-processing.html)

[gissuebot]

Original comment posted by wasserman.louis on 2011-11-15 at 08:43 PM

---

I think that framework have stronger ordering constraints for its handlers, however, and I'm not sure that behavior satisfies the principle of least astonishment.

For example, I don't think the JDK defines what order methods are iterated over with reflection, in which case there's no way to impose any ordering on several handlers in the same class.

[gissuebot]

Original comment posted by toellrich on 2011-11-15 at 08:57 PM

---

If you guys feel my request doesn't "gel" with your idea of an EventBus please feel free to ignore it ;)

I can always continue to use our own implementation or start using Axon framework. Thanks for your interest!

[gissuebot]

Original comment posted by wasserman.louis on 2011-11-17 at 01:47 AM

---

I didn't say that -- I think it's possible (likely?) that someone smarter than I am could figure out a sensible way of doing this. I'm just trying to figure out if there's any approach I can think of that would meet Guava's typical standards for "clear and well-defined semantics."

[gissuebot]

Original comment posted by kevinb@google.com on 2011-11-21 at 10:51 PM

---

Re: "I don't think the JDK defines what order methods are iterated over with reflection", not only does it not define it, but the order even changes between JDK 6 and 7.

[gissuebot]

Original comment posted by wasserman.louis on 2011-11-21 at 10:56 PM

---

Indeed. I think that's a pretty conclusive argument that there could never be a consistent, well-defined ordering on the handler order, assuming we're using the reflection-based register(Object) approach.

[gissuebot]

Original comment posted by cpovirk@google.com on 2011-12-05 at 11:25 PM

---

(No comment entered for this change.)

---

Labels: Type-Enhancement

[gissuebot]

Original comment posted by cbiffle@google.com on 2011-12-09 at 06:28 PM

---

Firewalling event producers from any failure in subscribers was quite deliberate. It was based on the notion of events being exchanged by separate components or library code, where a failure in one errant consumer shouldn't destroy all the other parts. Propagating exceptions across EventBus also allows producers to (perhaps deliberately and maliciously) discover aspects of consumer code shape and operation by inspecting stack traces, which makes me uncomfortable.

For your application, it sounds like something like this might suffice:

1. Distribute the events in arbitrary order, as we do today.
2. Take note of whether any subscribers failed. Skip any failed subscribers, as we do today.
3. Notify the event producer somehow (return value?) that one or more subscribers failed.

I would be hesitant to furnish the producer with a list of failed consumers, or to return a count (simply because I can't imagine how one would reasonably respond to a count). A boolean, though, seems reasonable.

Would a feature like this help?

[gissuebot]

Original comment posted by fry@google.com on 2011-12-10 at 04:22 PM

---

(No comment entered for this change.)

---

Labels: Package-EventBus

[gissuebot]

Original comment posted by toellrich on 2011-12-11 at 01:32 PM

---

A boolean return value from EventBus.post(Object event) that indicates whether any handler threw an exception? That would work for me since on false the event producer can throw a RuntimeException which would lead to a rollback of the transaction.

[gissuebot]

Original comment posted by cgdecker on 2011-12-11 at 02:08 PM

---

How would returning a boolean from post work with an AsyncEventBus? It would have to wait for all handlers to finish, which doesn't seem acceptable. I suppose post could return a ListenableFuture<Boolean> instead of just boolean, though that would create some overhead that seems unnecessary for most uses of EventBus.

[gissuebot]

Original comment posted by toellrich on 2011-12-12 at 02:45 AM

---

As I said, it's probably for the best if I stick to another event bus implementation.

[gissuebot]

Original comment posted by toellrich on 2011-12-16 at 09:20 PM

---

After your objections I've rethought the design of the application and changed it to no longer use an event bus (it was just a prototype). You may close the feature request.

[gissuebot]

Original comment posted by wasserman.louis on 2012-01-25 at 09:43 PM

---

Issue #868 has been merged into this issue.

[gissuebot]

Original comment posted by wasserman.louis on 2012-01-25 at 09:46 PM

---

Issue 868 suggested a much nicer way of doing this: when a handler fails, post a new event wrapping the exception, instead of doing a return value or something. Then, you can optionally log such events.

...I can think of one major way this can go wrong: what happens if a subscriber to Object fails? Then it'd post a SubscriberFailedEvent or whatever, then it'd receive the SubscriberFailedEvent and fail again, and would do an infinite loop. We might have code that specifically breaks this loop by ignoring failures on failures -- whenever a subscriber fails when it processes a SubscriberFailedEvent. I dunno.

Thoughts?

---

CC: cbiffle@google.com

[gissuebot]

Original comment posted by ewjmulder on 2012-01-26 at 08:28 AM

---

I see these issues are merged, makes sense indeed. About your last scenario: I mentioned this already in the first post of Issue 686: "Only downside might be some extra semantics about the exception event. Namely that any exceptions for listeners to exception events are not re-posted. But this is not any different from the fact that there is no new event if no-one listens to the DeadEvent. :)"

[gissuebot]

Original comment posted by wasserman.louis on 2012-01-27 at 01:34 AM

---

Sorry, I didn't parse that correctly originally. It's totally accurate.

[gissuebot]

Original comment posted by kevinb@google.com on 2012-01-31 at 07:21 PM

---

Thanks for articulating this problem to us, everyone!

We are considering introducing an "ExceptionEvent" and having the event bus post these events to itself upon subscriber failures.

But, any Throwable resulting from publishing an ExceptionEvent itself will probably just be dropped. (Or some anti-recursion policy.)

If you see any problems with this idea, let us know soon.

---

Status: Accepted Owner: cbiffle@google.com Labels: Milestone-Release12

[gissuebot]

Original comment posted by wasserman.louis on 2012-01-31 at 07:28 PM

---

Query: why ExceptionEvent vs. ThrowableEvent? Or is it just more readable that way?

[gissuebot]

Original comment posted by kevinb@google.com on 2012-02-01 at 06:27 AM

---

Just jotted that quickly without thinking.

Though, "ThrowableEvent" does kind of sound like the event itself can be thrown.

[gissuebot]

Original comment posted by wasserman.louis on 2012-02-01 at 06:55 AM

---

HandlerThrewEvent?

[gissuebot]

Original comment posted by cbiffle@google.com on 2012-02-06 at 07:18 PM

---

Louis is right that the event class name should reflect the context, not the contents. For example, DeadEvent is not called EventEvent. :-) (In fact, this is a good general design tip for EventBus-based systems.)

I'd suggest HandlerMethodThrew. It's long, but not by Java standards. :-) The term "handler method" is the one used in the API reference for a method annotated with @Subscribe. "Threw" is more correct than the obvious alternatives, like "failed," because it doesn't imply whether an operation was successful — that's up to the handler method's contract. Finally, you'll note I left off "event;" it seems redundant. DeadEvent is not named that because it is an event — it's because an event (other than it) is dead.

My concern about this is still the one I described above: it lets components see into each other's state. Java throwables contain a lot of state, including a complete stack trace and (depending on the class) application-specific information. EventBus currently tries hard not to leak information between subscribers (beyond things that can be derived through a side-channel anyway, like timing). Allowing a subscriber to see other subscribers' throwables would violate this principle.

As a strawman of an alternative, what if callers could specify an exception handler for a specific listener at register-time? Applications that truly want all their listeners to see everyone's throwables can just pass the same one to every call. Applications that don't use this behavior would never provide a handler. It also makes the recursion behavior application-controlled: crazy applications can register the handler as the handler for the handler.

The extreme version of this would actually introduce an observer interface for the exception handler, but that seems inconsistent.

[gissuebot]

Original comment posted by wasserman.louis on 2012-02-06 at 07:23 PM

---

I note that in a pinch, the exception handler you propose could be repurposed to more or less act the same as HandlerMethodThrew: you just pass a handler that posts the exception to the event bus. I like it, and the increased API surface is minimal.

[gissuebot]

Original comment posted by cbiffle@google.com on 2012-02-06 at 07:23 PM

---

(Yes, I realize that DeadEvent is arguably a violation of the principle I describe above. I'm thinking about how to fix that.)

[gissuebot]

Original comment posted by wasserman.louis on 2012-02-08 at 09:15 PM

---

Hrrrrrrm. I'm not sure what interface for the exception handler is most appropriate.

When we're handling an exception, do we pass in the handler object that threw the exception on one of its methods? Do we tell it which handler method threw the exception? Do we pass the handler object as an Object, damaging some of the nice type safety guarantees we had?

I'm not sure this is a solved problem yet.

[gissuebot]

Original comment posted by fabian.zeindl on 2012-02-11 at 10:32 AM

---

Just another input: I often use EventBus for stuff like CSP / threaded-programming, where i have one queue to input commands into the Thread. The reason i use EventBus there, is because it saves me from endless if (nstanceof) then constructs. I could refactor that to use a proper Interface, of course, but more often than not make the the code more complicated that it needs to be.

The reason i'm adding this is that in all my EventBus scenarios I have exactly one handler that's fired for an Event, which eliminates the "should other handlers be fired" as well problem. Dunno, if you can make something out of that.

[gissuebot]

Original comment posted by cbiffle@google.com on 2012-02-11 at 08:31 PM

---

Fabian, it sounds like what you want is runtime overload resolution (aka pattern matching). I agree that this is a missing feature in Java. It's not really EventBus's target use case, though.

Louis: my opinion on all of your questions is "no." I imagine the exception handler being just another handler that accepts HandlerMethodThrew; if nothing else, this greatly simplifies faking exceptions during testing. We should start with as narrow an interface for HandlerMethodThrew as possible -- we can always add more properties later if users require them. Because the exception handler is passed in per-register-call, users can always allocate new ones for each object if required (essentially partial application of the full many-input exception handler function).

[gissuebot]

Original comment posted by wasserman.louis on 2012-02-12 at 05:32 PM

---

Just to be clear: you're saying that if we wanted to tell the exception handler which method threw the exception, we'd add that data to HandlerMethodThrew?

I think I like this, I'm just trying to think if there's anything better.

[gissuebot]

Original comment posted by cbiffle@google.com on 2012-02-13 at 04:28 AM

---

Yes, precisely. As for whether the exception handler is an EventBus handler (with @Subscribe), or a single-method interface that accepts the event, I haven't decided.

[gissuebot]

Original comment posted by fabian.zeindl on 2012-02-15 at 10:05 AM

---

How do you do the logging, is there some way to get these into my default-logger, at least?

[gissuebot]

Original comment posted by cbiffle@google.com on 2012-02-15 at 02:31 PM

---

EventBus wouldn't do the logging for you. To get the exceptions into the logger of your choice, you'd provide your own exception handler that would forward them.

[gissuebot]

Original comment posted by fabian.zeindl on 2012-02-15 at 02:34 PM

---

Sure, but how is it done now?

[gissuebot]

Original comment posted by cbiffle@google.com on 2012-02-15 at 02:38 PM

---

https://github.com/google/guava/blob/master/guava/src/com/google/common/eventbus/EventBus.java#L319

[gissuebot]

Original comment posted by wasserman.louis on 2012-02-15 at 03:38 PM

---

(So, answer: no, there's no way to change the default logging behavior at the moment, not unless you can do it using one of the j.u.logging wrappers.)

[gissuebot]

Original comment posted by wasserman.louis on 2012-03-02 at 07:08 PM

---

Are we sure we'll have worked out these details to our satisfaction by release 12?

[gissuebot]

Original comment posted by cpovirk@google.com on 2012-03-23 at 09:27 PM

---

Rolling EventBus issues over to r13.

---

Labels: -Milestone-Release12, Milestone-Release13

[gissuebot]

Original comment posted by wasserman.louis on 2012-05-02 at 04:57 PM

---

FYI, cbiffle, Emily Soldal's "return a Ticket when a listener is registered" suggestion from issue 784 also addresses your suggestion of "register exception handlers on a per-listener basis."

[gissuebot]

Original comment posted by kevinb@google.com on 2012-05-30 at 07:41 PM

---

(No comment entered for this change.)

---

Labels: -Type-Enhancement, Type-Enhancement-Temp

[gissuebot]

Original comment posted by kevinb@google.com on 2012-05-30 at 07:45 PM

---

(No comment entered for this change.)

---

Labels: -Type-Enhancement-Temp, Type-Enhancement

[gissuebot]

Original comment posted by wasserman.louis on 2012-06-28 at 09:49 AM

---

Deleting the milestone from this and marking as Research like the rest of EventBus, since I don't think this'll make it out in 13. (I'm going to start putting in some research on the EventBus issues shortly, though.)

---

Status: Research Labels: -Milestone-Release13

[gissuebot]

Original comment posted by ashwin.jayaprakash on 2012-07-01 at 06:50 PM

---

At the very least, please make EventHandler "public and final" so that we can subclass Eventbus and override the "dispatch(Object event, EventHandler wrapper)" method. We can write our own code try-catch block and call "wrapper.handleEvent(event)" instead of just logging exceptions.

Right now EventHandler is package-private and we cannot even override the protected method. Seems weird that the method is protected but the parameter is package-protected.

[gissuebot]

Original comment posted by ashwin.jayaprakash on 2012-07-02 at 05:34 PM

---

Oh I see that in R13 the "EventBus.dispatch" method is package-private (https://github.com/google/guava/blob/master/guava/src/com/google/common/eventbus/EventBus.java#L300)

[gissuebot]

Original comment posted by wasserman.louis on 2012-09-05 at 02:07 PM

---

(No comment entered for this change.)

[gissuebot]

Original comment posted by wasserman.louis on 2012-09-05 at 02:08 PM

---

Issue #1132 has been merged into this issue.

[gissuebot]

Original comment posted by sudarshan89 on 2012-09-07 at 08:19 PM

---

We plan to use the event bus in production, for which we need to have some clarity on the exception handling mechanism. Any timelines by which this could be decided ?

[gissuebot]

Original comment posted by adam.g...@evocatus.com on 2012-09-07 at 08:35 PM

---

@sudarshan If you can't wait just copy the code from the eventbus package and put it into your own package. I had to do this because of JULI.

Then you just have to change EventBus.java to throw your own custom RuntimeException (or use Throwables.propagate() if you don't care what the wrapped exception is).

  protected void dispatch(Object event, EventHandler wrapper) {     try {       wrapper.handleEvent(event);     } catch (InvocationTargetException e) {       // My logger       logger.error(fatal, "Could not dispatch event: " + event + " to handler " + wrapper, e);       // My own custom runtime exception       throw new EventBusException(e);     }   }

[gissuebot]

Original comment posted by dro...@solveitsoftware.com on 2013-03-07 at 02:08 PM

---

Do you guys have any update regarding this issue? Any ETAs?