Add JBoss Logging and log exceptions when calling the thread context #197
Conversation
| if (t instanceof RuntimeException) { | ||
| throw (RuntimeException) t; | ||
| } | ||
| throw new RuntimeException(t); |
There was a problem hiding this comment.
I don't want to wrap Throwable just because of Java's crap handling, please use a rethrow method such as https://github.com/resteasy/Resteasy/blob/43f46228bbf8278a96648b48309f71d5ccad1e4c/resteasy-core/src/main/java/org/jboss/resteasy/core/SynchronousDispatcher.java#L186
| if (t instanceof RuntimeException) { | ||
| throw (RuntimeException) t; | ||
| } | ||
| throw new RuntimeException(t); |
| } | ||
| throw new RuntimeException(t); |
|
LGTM, except the wrapping of |
kabir
force-pushed
the
error-stack-traces
branch
from
3060d43
to
3e3ad48
Compare
|
Updated. The rethrow is a nice trick! |
|
| } | ||
| } | ||
|
|
||
| public static <T extends Throwable> void rethrow(Throwable t) throws T { |
There was a problem hiding this comment.
Slight nitpicking but wouldn't it be cleaner to have some util class containing this method instead of placing it here?
There was a problem hiding this comment.
Here's what I have in SmallRye Fault Tolerance:
public class SneakyThrow {
private SneakyThrow() {
}
/**
* This method can and should be used as part of a {@code throw} statement,
* such as: {@code throw sneakyThrow(exception);}. It is guaranteed to never return normally,
* and this style of usage makes sure that the Java compiler is aware of that.
*/
@SuppressWarnings("unchecked")
public static <E extends Throwable> RuntimeException sneakyThrow(Throwable e) throws E {
throw (E) e;
}
}Note the comment about usage. It's often nice (and I made it a rule for myself) to use the method like throw sneakyThrow(...), so that javac's control flow analysis knows that this is actually a throw.
There was a problem hiding this comment.
That's rather nice, thanks.
There was a problem hiding this comment.
Hah, SneakyThrow is a really good util class name :-D
There was a problem hiding this comment.
Not my invention :-) I think it's in Lombok, and was most likely used before that too.
|
@kabir also, the formatter plugin dislikes you ;-) |
kabir
force-pushed
the
error-stack-traces
branch
from
3e3ad48
to
a3d3bd4
Compare
|
Formatted and util made |
|
TBH I do wonder why we have to log an exception we rethrow, shouldn't it be logged by the caller? |
|
@FroMage AFAICT it gets swallowed by the CP plumbing |
If we have exception-swallowing in SR-CP I want to know where and fix it. Do you have stack traces? |
We don't really know who the caller is. In my case it was Reactive Messaging which makes a load of these calls, and it was not logged there which made tracking down why streams suddenly stop emitting data tricky. Although it sounds like we've decided that RM using CP is a bug, there could be other complex things calling this. To me it makes sense having it centralised in CP. |
providers