Exception Wrapper #32
27
README.md
27
README.md
@ -183,14 +183,37 @@ private void onDeadEvent(DeadEvent deadEvent) { ... }
|
|||||||
### Detecting Exceptions Thrown by Event Handlers
|
### Detecting Exceptions Thrown by Event Handlers
|
||||||
|
|
||||||
When an event handler throws an exception, an exception event is dispatched that wraps the original event.
|
When an event handler throws an exception, an exception event is dispatched that wraps the original event.
|
||||||
A exception handler is declared as follows:
|
An exception handler is declared as follows:
|
||||||
|
|
||||||
```java
|
```java
|
||||||
private void onExceptionEvent(ExceptionEvent ExceptionEvent) { ... }
|
private void onExceptionEvent(ExceptionEvent ExceptionEvent) { ... }
|
||||||
```
|
```
|
||||||
|
|
||||||
Both system events reference the event bus that caused them and a warning is logged if they are unhandled.
|
Both system events reference the event bus that caused them and a warning is logged if they are unhandled.
|
||||||
|
|
||||||
|
#### Yeeting Exceptions Out of an Event Handler
|
||||||
|
|
||||||
|
In some cases, a warning about an `Exception` that was thrown in an event handler is not enough, stays unnoticed, or an exception should be catched explicitly.
|
||||||
|
Event Bus explicitly dispatches no `ExceptionEvent` when an `ExceptionWrapper` exception is thrown and instead simply rethrows it.
|
||||||
|
`ExceptionWrapper` is an unchecked exception that (as the name says) simply wraps an exception that caused it.
|
||||||
|
This means the following is possible and results in a normal program exit:
|
||||||
|
```java
|
||||||
|
@Event(String.class)
|
||||||
|
void onString() {
|
||||||
|
throw new ExceptionWrapper(new RuntimeException("I failed!"));
|
||||||
|
}
|
||||||
|
|
||||||
|
void helloStackTrace() {
|
||||||
|
EventBus.getInstance().registerListener(this);
|
||||||
|
try {
|
||||||
|
EventBus.getInstance().dispatch("A string!");
|
||||||
|
System.exit(-1);
|
||||||
|
} catch(ExceptionWrapper e) {
|
||||||
|
e.getCause().printStackTrace();
|
||||||
|
System.exit(0);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
### What About Endless Recursion Caused By Dead Events and Exception Events?
|
### What About Endless Recursion Caused By Dead Events and Exception Events?
|
||||||
|
|
||||||
As one might imagine, an unhandled dead event would theoretically lead to an endless recursion.
|
As one might imagine, an unhandled dead event would theoretically lead to an endless recursion.
|
||||||
|
@ -112,10 +112,11 @@ public final class EventBus {
|
|||||||
*
|
*
|
||||||
* @param event the event to dispatch
|
* @param event the event to dispatch
|
||||||
* @throws EventBusException if an event handler isn't accessible or has an invalid signature
|
* @throws EventBusException if an event handler isn't accessible or has an invalid signature
|
||||||
|
* @throws ExceptionWrapper if it is thrown by an event handler
|
||||||
* @throws NullPointerException if the specified event is {@code null}
|
* @throws NullPointerException if the specified event is {@code null}
|
||||||
* @since 0.0.1
|
* @since 0.0.1
|
||||||
*/
|
*/
|
||||||
public void dispatch(Object event) throws EventBusException {
|
public void dispatch(Object event) {
|
||||||
Objects.requireNonNull(event);
|
Objects.requireNonNull(event);
|
||||||
logger.log(Level.INFO, "Dispatching event {0}", event);
|
logger.log(Level.INFO, "Dispatching event {0}", event);
|
||||||
|
|
||||||
@ -140,6 +141,10 @@ public final class EventBus {
|
|||||||
|
|
||||||
// Transparently pass error to the caller
|
// Transparently pass error to the caller
|
||||||
throw (Error) e.getCause();
|
throw (Error) e.getCause();
|
||||||
|
else if (e.getCause() instanceof ExceptionWrapper)
|
||||||
|
|
||||||
|
// Transparently pass exception wrapper to the caller
|
||||||
|
throw (ExceptionWrapper) e.getCause();
|
||||||
else if (event instanceof DeadEvent || event instanceof ExceptionEvent)
|
else if (event instanceof DeadEvent || event instanceof ExceptionEvent)
|
||||||
|
|
||||||
// Warn about system event not being handled
|
// Warn about system event not being handled
|
||||||
@ -214,7 +219,7 @@ public final class EventBus {
|
|||||||
* @since 0.0.1
|
* @since 0.0.1
|
||||||
* @see Event
|
* @see Event
|
||||||
*/
|
*/
|
||||||
public void registerListener(Object listener) throws EventBusException {
|
public void registerListener(Object listener) {
|
||||||
delvh marked this conversation as resolved
|
|||||||
Objects.requireNonNull(listener);
|
Objects.requireNonNull(listener);
|
||||||
if (registeredListeners.contains(listener))
|
if (registeredListeners.contains(listener))
|
||||||
throw new EventBusException(listener + " already registered!");
|
throw new EventBusException(listener + " already registered!");
|
||||||
|
@ -14,13 +14,14 @@ package dev.kske.eventbus.core;
|
|||||||
*/
|
*/
|
||||||
public final class EventBusException extends RuntimeException {
|
public final class EventBusException extends RuntimeException {
|
||||||
|
|
||||||
private static final long serialVersionUID = 1L;
|
private static final long serialVersionUID = 7254445250300604449L;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Creates a new event bus exception.
|
* Creates a new event bus exception.
|
||||||
*
|
*
|
||||||
* @param message the message to display
|
* @param message the message to display
|
||||||
* @param cause the cause of this exception
|
* @param cause the cause of this exception
|
||||||
|
* @since 0.0.1
|
||||||
*/
|
*/
|
||||||
public EventBusException(String message, Throwable cause) {
|
public EventBusException(String message, Throwable cause) {
|
||||||
super(message, cause);
|
super(message, cause);
|
||||||
@ -30,6 +31,7 @@ public final class EventBusException extends RuntimeException {
|
|||||||
* Creates a new event bus exception.
|
* Creates a new event bus exception.
|
||||||
*
|
*
|
||||||
* @param message the message to display
|
* @param message the message to display
|
||||||
|
* @since 0.0.1
|
||||||
*/
|
*/
|
||||||
public EventBusException(String message) {
|
public EventBusException(String message) {
|
||||||
super(message);
|
super(message);
|
||||||
|
@ -0,0 +1,24 @@
|
|||||||
|
package dev.kske.eventbus.core;
|
||||||
|
|
||||||
|
/**
|
||||||
mpk marked this conversation as resolved
delvh
commented
One blank line too much One blank line too much
|
|||||||
|
* This unchecked exception acts as a wrapper for an arbitrary exception to prevent an
|
||||||
|
* {@link ExceptionEvent} from being dispatched. Instead, the wrapped exception is rethrown by
|
||||||
|
* {@link EventBus#dispatch(Object)}.
|
||||||
|
*
|
||||||
|
* @author Kai S. K. Engelbart
|
||||||
|
* @since 1.2.1
|
||||||
|
*/
|
||||||
|
public final class ExceptionWrapper extends RuntimeException {
|
||||||
|
|
||||||
|
private static final long serialVersionUID = -2016681140617308788L;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Creates a new exception wrapper.
|
||||||
|
*
|
||||||
|
* @param cause the exception to wrap
|
||||||
|
* @since 1.2.1
|
||||||
|
*/
|
||||||
|
public ExceptionWrapper(Exception cause) {
|
||||||
|
super(cause);
|
||||||
delvh marked this conversation as resolved
delvh
commented
What I just noticed: Do we want to allow What I just noticed: Do we want to allow `null` causes?
mpk
commented
Well, Event Bus doesn't have a problem with it, even though it makes little sense. I would leave it. Well, Event Bus doesn't have a problem with it, even though it makes little sense. I would leave it.
|
|||||||
|
}
|
||||||
|
}
|
@ -0,0 +1,33 @@
|
|||||||
|
package dev.kske.eventbus.core;
|
||||||
|
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||||
|
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tests the behavior of the event bus when an {@link ExceptionWrapper} is thrown.
|
||||||
|
*
|
||||||
|
* @author Kai S. K. Engelbart
|
||||||
|
* @since 1.2.1
|
||||||
|
*/
|
||||||
|
public class ExceptionWrapperTest {
|
||||||
|
|
||||||
|
EventBus bus = new EventBus();
|
||||||
|
String event = "This event will cause an exception";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tests transparent rethrowing of an exception wrapper by {@link EventBus#dispatch(Object)}.
|
||||||
|
*
|
||||||
|
* @since 1.2.1
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
public void testExceptionWrapper() {
|
||||||
|
bus.registerListener(this);
|
||||||
|
assertThrows(ExceptionWrapper.class, () -> bus.dispatch(event));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Event(String.class)
|
||||||
|
void onString() {
|
||||||
|
throw new ExceptionWrapper(new RuntimeException("I failed!"));
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue
Block a user
Why did you remove that declaration?
Because we decided against that for our other projects and it also seems to be like this in other big Java projects like Spring. The
@throws
in the Javadoc will stay, though.I think that every exception you explicitly declare via
throw
should also be mentioned inthrows
.I know it is unnecessary for unchecked exceptions.
However, I noticed that whenever I'm not directly in a Giga-IDE such as Eclipse, my first course of action is to look for
throws
in the source code, and not@throws
in the Javadoc, especially as it is often faster to open the source code and look there instead of the javadoc as Javadoc always needs a timeout to plop up, and knowing Eclipse, it sometimes even doesn't plop up. And even if it plops up, you still have to read through the Javadoc, which is also longer than simply looking at the source code.That's why I prefer it that way.
And regarding Spring: Spring is not a project where you should get your styleguides from. Otherwise we also should name our interface implementations
<interface>Impl
or<interface>Spec
. Or have classes such as theMultiPartResolverTestFactoryTest
.In that case, however, the code base would already violate compliance, as we don't declare
throws NullPointerException
for example. Also, we would have to consider all unchecked exceptions potentially thrown by the methods that are called by the method we want to declarethrows
on, which don't follow those conventions themselved. That would be a large and unnecessary effort in my opinion.Why? This is completely arbitrary. So the
NullPointerException
should be removed frmo the Javadoc in that case?I'd say it depends: When you explicitly call
Objects.requireNonNull()
or something similar, then no.I've oversimplified my view point in the previous messages.
I am in favor of explicitly declaring unchecked exceptions, when:
throw
yourselfObjects.requireNonNull
)(i.e. you have two string parameters, of which one is is the complete name of an enum class and the other is the name of one of its constants. Then I would declare the exceptions thrown both from
Class.forName()
and from<enum class>.valueOf()
insidethrows
)I am only in favor of declaring something JVM-internal such as a
NullPointerException
when you're actually doing a null check, and never else.If I were to implement this, multiple other methods in the code base have to be adjusted, as well as their Javadoc. Are you in favor of this?
But first I'm interested in whether @kske overrules me.