From 5a2109ba2acdab77b9c5916c687c79efa06d820d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Thu, 21 Jul 2022 17:44:17 +0200 Subject: [PATCH] Add logging in Exceptions.throwIf[Jvm]Fatal, add isFatal methods This commit improves the discoverability of exceptions that are "fatal" to Reactor by: - adding `Exceptions.isFatal` and `Exceptions.isJvmFatal` methods - adding logging to `Exceptions.throwIfFatal` and `Exceptions.throwIfJvmFatal` just before a fatal exception is thrown This should at least help pinpointing such occurrences from the logs, in cases where the thrown exception bubble all the way up the stack of a thread with no uncaughtExceptionHandler for example. Fixes #3111. --- .../main/java/reactor/core/Exceptions.java | 94 ++++++++++++++++--- .../java/reactor/core/ExceptionsTest.java | 70 +++++++++++++- 2 files changed, 147 insertions(+), 17 deletions(-) diff --git a/reactor-core/src/main/java/reactor/core/Exceptions.java b/reactor-core/src/main/java/reactor/core/Exceptions.java index f82fe86b11..20cb6a1336 100644 --- a/reactor-core/src/main/java/reactor/core/Exceptions.java +++ b/reactor-core/src/main/java/reactor/core/Exceptions.java @@ -25,6 +25,8 @@ import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import reactor.core.publisher.Flux; +import reactor.util.Logger; +import reactor.util.Loggers; import reactor.util.annotation.Nullable; import reactor.util.retry.Retry; @@ -36,6 +38,8 @@ */ public abstract class Exceptions { + private static final Logger LOGGER = Loggers.getLogger(Exceptions.class); + /** * A common error message used when a reactive streams source doesn't seem to respect * backpressure signals, resulting in an operator's internal queue to be full. @@ -412,6 +416,70 @@ public static Throwable terminate(AtomicReferenceFieldUpdater return current; } + /** + * Check if a {@link Throwable} is considered by Reactor as Jvm Fatal and would be thrown + * by both {@link #throwIfFatal(Throwable)} and {@link #throwIfJvmFatal(Throwable)}. + * This is a subset of {@link #isFatal(Throwable)}, namely: + *
    + *
  • {@link VirtualMachineError}
    • + *
  • {@link ThreadDeath}
    • + *
  • {@link LinkageError}
    • + *
+ *

+ * Unless wrapped explicitly, such exceptions would always be thrown by operators instead of + * propagation through onError, potentially interrupting progress of Flux/Mono sequences. + * When they occur, the JVM itself is assumed to be in an unrecoverable state, and so is Reactor. + * + * @see #throwIfFatal(Throwable) + * @see #throwIfJvmFatal(Throwable) + * @see #isFatal(Throwable) + * @param t the {@link Throwable} to check + * @return true if the throwable is considered Jvm Fatal + */ + public static boolean isJvmFatal(@Nullable Throwable t) { + if (t instanceof VirtualMachineError || + t instanceof ThreadDeath || + t instanceof LinkageError) { + return true; + } + return false; + } + + /** + * Check if a {@link Throwable} is considered by Reactor as Fatal and would be thrown by + * {@link #throwIfFatal(Throwable)}. + *

    + *
  • {@code BubblingException} (as detectable by {@link #isBubbling(Throwable)})
    • + *
  • {@code ErrorCallbackNotImplemented} (as detectable by {@link #isErrorCallbackNotImplemented(Throwable)})
    • + *
  • {@link #isJvmFatal(Throwable) Jvm Fatal exceptions} + *
      + *
    • {@link VirtualMachineError}
      • + *
    • {@link ThreadDeath}
      • + *
    • {@link LinkageError}
      • + *
    + *
    • + *
+ *

+ * Unless wrapped explicitly, such exceptions would always be thrown by operators instead of + * propagation through onError, potentially interrupting progress of Flux/Mono sequences. + * When they occur, the assumption is that Reactor is in an unrecoverable state (notably + * because the JVM itself might be in an unrecoverable state). + * + * @see #throwIfFatal(Throwable) + * @see #isJvmFatal(Throwable) + * @param t the {@link Throwable} to check + * @return true if the throwable is considered fatal + */ + public static boolean isFatal(@Nullable Throwable t) { + if (t instanceof BubblingException || t instanceof ErrorCallbackNotImplemented) { + return true; + } + if (isJvmFatal(t)) { + return true; + } + return false; + } + /** * Throws a particular {@code Throwable} only if it belongs to a set of "fatal" error * varieties. These varieties are as follows:

    @@ -422,13 +490,16 @@ public static Throwable terminate(AtomicReferenceFieldUpdater * @param t the exception to evaluate */ public static void throwIfFatal(@Nullable Throwable t) { - if (t instanceof BubblingException) { - throw (BubblingException) t; - } - if (t instanceof ErrorCallbackNotImplemented) { - throw (ErrorCallbackNotImplemented) t; + if (t == null) { + return; } + //we give throwIfJvmFatal an opportunity to detect, log and throw first throwIfJvmFatal(t); + if (isFatal(t)) { + LOGGER.warn("throwIfFatal detected a fatal exception, throwing", t); + assert t instanceof RuntimeException; + throw (RuntimeException) t; + } } /** @@ -440,14 +511,13 @@ public static void throwIfFatal(@Nullable Throwable t) { * @param t the exception to evaluate */ public static void throwIfJvmFatal(@Nullable Throwable t) { - if (t instanceof VirtualMachineError) { - throw (VirtualMachineError) t; - } - if (t instanceof ThreadDeath) { - throw (ThreadDeath) t; + if (t == null) { + return; } - if (t instanceof LinkageError) { - throw (LinkageError) t; + if (isJvmFatal(t)) { + LOGGER.warn("throwIfJvmFatal detected a fatal exception, throwing", t); + assert t instanceof Error; + throw (Error) t; } } diff --git a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java index a05bb2369f..3bf143449b 100644 --- a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java +++ b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java @@ -156,9 +156,64 @@ public void propagateDoesntWrapRuntimeException() { //TODO test terminate + @Test + void isFatalNotJvmFatalBubbling() { + Throwable exception = new BubblingException("expected"); + assertThat(Exceptions.isFatal(exception)) + .as("isFatal(bubbling)") + .isTrue(); + assertThat(Exceptions.isJvmFatal(exception)) + .as("isJvmFatal(bubbling)") + .isFalse(); + } + + @Test + void isFatalNotJvmFatalErrorCallback() { + Throwable exception = new ErrorCallbackNotImplemented(new IllegalStateException("expected cause")); + assertThat(Exceptions.isFatal(exception)) + .as("isFatal(ErrorCallbackNotImplemented)") + .isTrue(); + assertThat(Exceptions.isJvmFatal(exception)) + .as("isJvmFatal(ErrorCallbackNotImplemented)") + .isFalse(); + } + + @Test + void isFatalAndJvmFatalVirtualMachineError() { + Throwable exception = new VirtualMachineError() { }; + assertThat(Exceptions.isFatal(exception)) + .as("isFatal(VirtualMachineError)") + .isTrue(); + assertThat(Exceptions.isJvmFatal(exception)) + .as("isJvmFatal(VirtualMachineError)") + .isTrue(); + } + + @Test + void isFatalAndJvmFatalLinkageError() { + Throwable exception = new LinkageError(); + assertThat(Exceptions.isFatal(exception)) + .as("isFatal(LinkageError)") + .isTrue(); + assertThat(Exceptions.isJvmFatal(exception)) + .as("isJvmFatal(LinkageError)") + .isTrue(); + } + + @Test + void isFatalAndJvmFatalThreadDeath() { + Throwable exception = new ThreadDeath(); + assertThat(Exceptions.isFatal(exception)) + .as("isFatal(ThreadDeath)") + .isTrue(); + assertThat(Exceptions.isJvmFatal(exception)) + .as("isJvmFatal(ThreadDeath)") + .isTrue(); + } + @Test public void throwIfFatalThrowsBubbling() { - BubblingException expected = new BubblingException("expected"); + BubblingException expected = new BubblingException("expected to be logged"); assertThatExceptionOfType(BubblingException.class) .isThrownBy(() -> Exceptions.throwIfFatal(expected)) @@ -167,7 +222,7 @@ public void throwIfFatalThrowsBubbling() { @Test public void throwIfFatalThrowsErrorCallbackNotImplemented() { - ErrorCallbackNotImplemented expected = new ErrorCallbackNotImplemented(new IllegalStateException("expected cause")); + ErrorCallbackNotImplemented expected = new ErrorCallbackNotImplemented(new IllegalStateException("expected to be logged")); assertThatExceptionOfType(ErrorCallbackNotImplemented.class) .isThrownBy(() -> Exceptions.throwIfFatal(expected)) @@ -177,9 +232,14 @@ public void throwIfFatalThrowsErrorCallbackNotImplemented() { @Test public void throwIfJvmFatal() { - VirtualMachineError fatal1 = new VirtualMachineError() {}; - ThreadDeath fatal2 = new ThreadDeath(); - LinkageError fatal3 = new LinkageError(); + VirtualMachineError fatal1 = new VirtualMachineError("expected to be logged") {}; + ThreadDeath fatal2 = new ThreadDeath() { + @Override + public String getMessage() { + return "expected to be logged"; + } + }; + LinkageError fatal3 = new LinkageError("expected to be logged"); assertThatExceptionOfType(VirtualMachineError.class) .as("VirtualMachineError")