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 1/7] 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") From 3abc2227d4a819fa4d3c03b702eee0d1800610a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Thu, 21 Jul 2022 17:54:02 +0200 Subject: [PATCH 2/7] fix license --- reactor-core/src/main/java/reactor/core/Exceptions.java | 2 +- reactor-core/src/test/java/reactor/core/ExceptionsTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/reactor-core/src/main/java/reactor/core/Exceptions.java b/reactor-core/src/main/java/reactor/core/Exceptions.java index 20cb6a1336..e5d6444642 100644 --- a/reactor-core/src/main/java/reactor/core/Exceptions.java +++ b/reactor-core/src/main/java/reactor/core/Exceptions.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2021 VMware Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2016-2022 VMware Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java index 3bf143449b..b591c328dc 100644 --- a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java +++ b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015-2021 VMware Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2015-2022 VMware Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 988b16e8ea0f92d114ac1b0c5775bc03e96f28bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Fri, 22 Jul 2022 12:28:23 +0200 Subject: [PATCH 3/7] Improve isFatal tests in throwIfFatal, add coverage, improve test names --- .../main/java/reactor/core/Exceptions.java | 31 ++++++----- .../java/reactor/core/ExceptionsTest.java | 54 ++++++++++++++++--- 2 files changed, 63 insertions(+), 22 deletions(-) diff --git a/reactor-core/src/main/java/reactor/core/Exceptions.java b/reactor-core/src/main/java/reactor/core/Exceptions.java index e5d6444642..a37e92f227 100644 --- a/reactor-core/src/main/java/reactor/core/Exceptions.java +++ b/reactor-core/src/main/java/reactor/core/Exceptions.java @@ -437,12 +437,9 @@ public static Throwable terminate(AtomicReferenceFieldUpdater * @return true if the throwable is considered Jvm Fatal */ public static boolean isJvmFatal(@Nullable Throwable t) { - if (t instanceof VirtualMachineError || + return t instanceof VirtualMachineError || t instanceof ThreadDeath || - t instanceof LinkageError) { - return true; - } - return false; + t instanceof LinkageError; } /** @@ -471,13 +468,14 @@ public static boolean isJvmFatal(@Nullable Throwable t) { * @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; + return isFatalButNotJvmFatal(t) || isJvmFatal(t); + } + + /** + * Internal intermediate test that only detect Fatal but not Jvm Fatal exceptions. + */ + static boolean isFatalButNotJvmFatal(@Nullable Throwable t) { + return t instanceof BubblingException || t instanceof ErrorCallbackNotImplemented; } /** @@ -493,13 +491,14 @@ public static void throwIfFatal(@Nullable Throwable t) { if (t == null) { return; } - //we give throwIfJvmFatal an opportunity to detect, log and throw first - throwIfJvmFatal(t); - if (isFatal(t)) { + if (isFatalButNotJvmFatal(t)) { LOGGER.warn("throwIfFatal detected a fatal exception, throwing", t); - assert t instanceof RuntimeException; throw (RuntimeException) t; } + if (isJvmFatal(t)) { + LOGGER.warn("throwIfFatal detected a jvm fatal exception, throwing", t); + 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 b591c328dc..8d69ad354f 100644 --- a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java +++ b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java @@ -23,6 +23,8 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; +import org.assertj.core.api.SoftAssertions; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import reactor.core.publisher.Mono; @@ -157,7 +159,7 @@ public void propagateDoesntWrapRuntimeException() { //TODO test terminate @Test - void isFatalNotJvmFatalBubbling() { + void bubblingExceptionIsFatalButNotJvmFatal() { Throwable exception = new BubblingException("expected"); assertThat(Exceptions.isFatal(exception)) .as("isFatal(bubbling)") @@ -168,7 +170,7 @@ void isFatalNotJvmFatalBubbling() { } @Test - void isFatalNotJvmFatalErrorCallback() { + void errorCallbackNotImplementedIsFatalButNotJvmFatal() { Throwable exception = new ErrorCallbackNotImplemented(new IllegalStateException("expected cause")); assertThat(Exceptions.isFatal(exception)) .as("isFatal(ErrorCallbackNotImplemented)") @@ -179,7 +181,7 @@ void isFatalNotJvmFatalErrorCallback() { } @Test - void isFatalAndJvmFatalVirtualMachineError() { + void virtualMachineErrorIsFatalAndJvmFatal() { Throwable exception = new VirtualMachineError() { }; assertThat(Exceptions.isFatal(exception)) .as("isFatal(VirtualMachineError)") @@ -190,7 +192,7 @@ void isFatalAndJvmFatalVirtualMachineError() { } @Test - void isFatalAndJvmFatalLinkageError() { + void linkageErrorIsFatalAndJvmFatal() { Throwable exception = new LinkageError(); assertThat(Exceptions.isFatal(exception)) .as("isFatal(LinkageError)") @@ -201,7 +203,7 @@ void isFatalAndJvmFatalLinkageError() { } @Test - void isFatalAndJvmFatalThreadDeath() { + void threadDeathIsFatalAndJvmFatal() { Throwable exception = new ThreadDeath(); assertThat(Exceptions.isFatal(exception)) .as("isFatal(ThreadDeath)") @@ -231,7 +233,36 @@ public void throwIfFatalThrowsErrorCallbackNotImplemented() { } @Test - public void throwIfJvmFatal() { + void throwIfFatalWithJvmFatalErrors() { + 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"); + + SoftAssertions.assertSoftly(softly -> { + softly.assertThatExceptionOfType(VirtualMachineError.class) + .as("VirtualMachineError") + .isThrownBy(() -> Exceptions.throwIfFatal(fatal1)) + .isSameAs(fatal1); + + softly.assertThatExceptionOfType(ThreadDeath.class) + .as("ThreadDeath") + .isThrownBy(() -> Exceptions.throwIfFatal(fatal2)) + .isSameAs(fatal2); + + softly.assertThatExceptionOfType(LinkageError.class) + .as("LinkageError") + .isThrownBy(() -> Exceptions.throwIfFatal(fatal3)) + .isSameAs(fatal3); + }); + } + + @Test + void throwIfJvmFatal() { VirtualMachineError fatal1 = new VirtualMachineError("expected to be logged") {}; ThreadDeath fatal2 = new ThreadDeath() { @Override @@ -257,6 +288,17 @@ public String getMessage() { .isSameAs(fatal3); } + @Test + void throwIfJvmFatalDoesntThrowOnSimplyFatalExceptions() { + SoftAssertions.assertSoftly(softly -> { + softly.assertThatCode(() -> Exceptions.throwIfJvmFatal(new BubblingException("not thrown"))) + .doesNotThrowAnyException(); + + softly.assertThatCode(() -> Exceptions.throwIfJvmFatal(new ErrorCallbackNotImplemented(new RuntimeException("not thrown")))) + .doesNotThrowAnyException(); + }); + } + @Test public void multipleWithNullVararg() { //noinspection ConstantConditions From b6632150335990c9c0737405bd8ea3cd451ee2f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Fri, 22 Jul 2022 13:22:35 +0200 Subject: [PATCH 4/7] Polish logging message and test log output of throwIfFatal --- .../main/java/reactor/core/Exceptions.java | 6 +- .../java/reactor/core/ExceptionsTest.java | 134 ++++++++++++------ 2 files changed, 93 insertions(+), 47 deletions(-) diff --git a/reactor-core/src/main/java/reactor/core/Exceptions.java b/reactor-core/src/main/java/reactor/core/Exceptions.java index a37e92f227..bfacf990ea 100644 --- a/reactor-core/src/main/java/reactor/core/Exceptions.java +++ b/reactor-core/src/main/java/reactor/core/Exceptions.java @@ -492,11 +492,11 @@ public static void throwIfFatal(@Nullable Throwable t) { return; } if (isFatalButNotJvmFatal(t)) { - LOGGER.warn("throwIfFatal detected a fatal exception, throwing", t); + LOGGER.warn("throwIfFatal detected a fatal exception, which is thrown and logged below:", t); throw (RuntimeException) t; } if (isJvmFatal(t)) { - LOGGER.warn("throwIfFatal detected a jvm fatal exception, throwing", t); + LOGGER.warn("throwIfFatal detected a jvm fatal exception, which is thrown and logged below:", t); throw (Error) t; } } @@ -514,7 +514,7 @@ public static void throwIfJvmFatal(@Nullable Throwable t) { return; } if (isJvmFatal(t)) { - LOGGER.warn("throwIfJvmFatal detected a fatal exception, throwing", t); + LOGGER.warn("throwIfJvmFatal detected a jvm fatal exception, which is thrown and logged below:", 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 8d69ad354f..ff94d0b6ef 100644 --- a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java +++ b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java @@ -24,11 +24,14 @@ import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import org.assertj.core.api.SoftAssertions; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import reactor.TestLoggerExtension; import reactor.core.publisher.Mono; +import reactor.test.util.LoggerUtils; import reactor.test.util.RaceTestUtils; +import reactor.test.util.TestLogger; import reactor.util.annotation.Nullable; import static org.assertj.core.api.Assertions.*; @@ -45,6 +48,32 @@ public class ExceptionsTest { static final AtomicReferenceFieldUpdater ADD_THROWABLE = AtomicReferenceFieldUpdater.newUpdater(ExceptionsTest.class, Throwable.class, "addThrowable"); + static VirtualMachineError JVM_FATAL_VIRTUAL_MACHINE_ERROR = new VirtualMachineError("expected to be logged") { + @Override + public String toString() { + return "custom VirtualMachineError: expected to be logged"; + } + }; + + static final ThreadDeath JVM_FATAL_THREAD_DEATH = new ThreadDeath() { + @Override + public String getMessage() { + return "expected to be logged"; + } + + @Override + public String toString() { + return "custom ThreadDeath: expected to be logged"; + } + }; + + static final LinkageError JVM_FATAL_LINKAGE_ERROR = new LinkageError("expected to be logged") { + @Override + public String toString() { + return "custom LinkageError: expected to be logged"; + } + }; + @Test public void bubble() throws Exception { Throwable t = new Exception("test"); @@ -182,114 +211,129 @@ void errorCallbackNotImplementedIsFatalButNotJvmFatal() { @Test void virtualMachineErrorIsFatalAndJvmFatal() { - Throwable exception = new VirtualMachineError() { }; - assertThat(Exceptions.isFatal(exception)) + assertThat(Exceptions.isFatal(JVM_FATAL_VIRTUAL_MACHINE_ERROR)) .as("isFatal(VirtualMachineError)") .isTrue(); - assertThat(Exceptions.isJvmFatal(exception)) + assertThat(Exceptions.isJvmFatal(JVM_FATAL_VIRTUAL_MACHINE_ERROR)) .as("isJvmFatal(VirtualMachineError)") .isTrue(); } @Test void linkageErrorIsFatalAndJvmFatal() { - Throwable exception = new LinkageError(); - assertThat(Exceptions.isFatal(exception)) + assertThat(Exceptions.isFatal(JVM_FATAL_LINKAGE_ERROR)) .as("isFatal(LinkageError)") .isTrue(); - assertThat(Exceptions.isJvmFatal(exception)) + assertThat(Exceptions.isJvmFatal(JVM_FATAL_LINKAGE_ERROR)) .as("isJvmFatal(LinkageError)") .isTrue(); } @Test void threadDeathIsFatalAndJvmFatal() { - Throwable exception = new ThreadDeath(); - assertThat(Exceptions.isFatal(exception)) + assertThat(Exceptions.isFatal(JVM_FATAL_THREAD_DEATH)) .as("isFatal(ThreadDeath)") .isTrue(); - assertThat(Exceptions.isJvmFatal(exception)) + assertThat(Exceptions.isJvmFatal(JVM_FATAL_THREAD_DEATH)) .as("isJvmFatal(ThreadDeath)") .isTrue(); } + @AfterEach + void disableCapture() { + LoggerUtils.disableCapture(); + } + @Test - public void throwIfFatalThrowsBubbling() { + void throwIfFatalThrowsAndLogsBubbling() { + TestLogger testLogger = new TestLogger(false); + LoggerUtils.enableCaptureWith(testLogger); + BubblingException expected = new BubblingException("expected to be logged"); assertThatExceptionOfType(BubblingException.class) .isThrownBy(() -> Exceptions.throwIfFatal(expected)) .isSameAs(expected); + + assertThat(testLogger.getErrContent()) + .startsWith("[ WARN] throwIfFatal detected a fatal exception, which is thrown and logged below: - reactor.core.Exceptions$BubblingException: expected to be logged"); } @Test - public void throwIfFatalThrowsErrorCallbackNotImplemented() { + void throwIfFatalThrowsAndLogsErrorCallbackNotImplemented() { + TestLogger testLogger = new TestLogger(false); + LoggerUtils.enableCaptureWith(testLogger); + ErrorCallbackNotImplemented expected = new ErrorCallbackNotImplemented(new IllegalStateException("expected to be logged")); assertThatExceptionOfType(ErrorCallbackNotImplemented.class) .isThrownBy(() -> Exceptions.throwIfFatal(expected)) .isSameAs(expected) .withCause(expected.getCause()); + + assertThat(testLogger.getErrContent()) + .startsWith("[ WARN] throwIfFatal detected a fatal exception, which is thrown and logged below: - reactor.core.Exceptions$ErrorCallbackNotImplemented: java.lang.IllegalStateException: expected to be logged"); } @Test - void throwIfFatalWithJvmFatalErrors() { - 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"); + void throwIfFatalWithJvmFatalErrorsDoesThrowAndLog() { + TestLogger testLogger = new TestLogger(false); + LoggerUtils.enableCaptureWith(testLogger); SoftAssertions.assertSoftly(softly -> { softly.assertThatExceptionOfType(VirtualMachineError.class) .as("VirtualMachineError") - .isThrownBy(() -> Exceptions.throwIfFatal(fatal1)) - .isSameAs(fatal1); + .isThrownBy(() -> Exceptions.throwIfFatal(JVM_FATAL_VIRTUAL_MACHINE_ERROR)) + .isSameAs(JVM_FATAL_VIRTUAL_MACHINE_ERROR); softly.assertThatExceptionOfType(ThreadDeath.class) .as("ThreadDeath") - .isThrownBy(() -> Exceptions.throwIfFatal(fatal2)) - .isSameAs(fatal2); + .isThrownBy(() -> Exceptions.throwIfFatal(JVM_FATAL_THREAD_DEATH)) + .isSameAs(JVM_FATAL_THREAD_DEATH); softly.assertThatExceptionOfType(LinkageError.class) .as("LinkageError") - .isThrownBy(() -> Exceptions.throwIfFatal(fatal3)) - .isSameAs(fatal3); + .isThrownBy(() -> Exceptions.throwIfFatal(JVM_FATAL_LINKAGE_ERROR)) + .isSameAs(JVM_FATAL_LINKAGE_ERROR); + + softly.assertThat(testLogger.getErrContent()) + .startsWith("[ WARN] throwIfFatal detected a jvm fatal exception, which is thrown and logged below: - custom VirtualMachineError: expected to be logged") + .contains("[ WARN] throwIfFatal detected a jvm fatal exception, which is thrown and logged below: - custom ThreadDeath: expected to be logged") + .contains("[ WARN] throwIfFatal detected a jvm fatal exception, which is thrown and logged below: - custom LinkageError: expected to be logged"); }); } @Test - void throwIfJvmFatal() { - 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"); + void throwIfJvmFatalDoesThrowAndLog() { + TestLogger testLogger = new TestLogger(false); + LoggerUtils.enableCaptureWith(testLogger); assertThatExceptionOfType(VirtualMachineError.class) .as("VirtualMachineError") - .isThrownBy(() -> Exceptions.throwIfJvmFatal(fatal1)) - .isSameAs(fatal1); + .isThrownBy(() -> Exceptions.throwIfJvmFatal(JVM_FATAL_VIRTUAL_MACHINE_ERROR)) + .isSameAs(JVM_FATAL_VIRTUAL_MACHINE_ERROR); assertThatExceptionOfType(ThreadDeath.class) .as("ThreadDeath") - .isThrownBy(() -> Exceptions.throwIfJvmFatal(fatal2)) - .isSameAs(fatal2); + .isThrownBy(() -> Exceptions.throwIfJvmFatal(JVM_FATAL_THREAD_DEATH)) + .isSameAs(JVM_FATAL_THREAD_DEATH); assertThatExceptionOfType(LinkageError.class) .as("LinkageError") - .isThrownBy(() -> Exceptions.throwIfJvmFatal(fatal3)) - .isSameAs(fatal3); - } + .isThrownBy(() -> Exceptions.throwIfJvmFatal(JVM_FATAL_LINKAGE_ERROR)) + .isSameAs(JVM_FATAL_LINKAGE_ERROR); + + assertThat(testLogger.getErrContent()) + .startsWith("[ WARN] throwIfJvmFatal detected a jvm fatal exception, which is thrown and logged below: - custom VirtualMachineError: expected to be logged") + .contains("[ WARN] throwIfJvmFatal detected a jvm fatal exception, which is thrown and logged below: - custom ThreadDeath: expected to be logged") + .contains("[ WARN] throwIfJvmFatal detected a jvm fatal exception, which is thrown and logged below: - custom LinkageError: expected to be logged"); + }; @Test - void throwIfJvmFatalDoesntThrowOnSimplyFatalExceptions() { + void throwIfJvmFatalDoesntThrowNorLogsOnSimplyFatalExceptions() { + TestLogger testLogger = new TestLogger(false); + LoggerUtils.enableCaptureWith(testLogger); + SoftAssertions.assertSoftly(softly -> { softly.assertThatCode(() -> Exceptions.throwIfJvmFatal(new BubblingException("not thrown"))) .doesNotThrowAnyException(); @@ -297,6 +341,8 @@ void throwIfJvmFatalDoesntThrowOnSimplyFatalExceptions() { softly.assertThatCode(() -> Exceptions.throwIfJvmFatal(new ErrorCallbackNotImplemented(new RuntimeException("not thrown")))) .doesNotThrowAnyException(); }); + + assertThat(testLogger.getErrContent()).isEmpty(); } @Test From 31912b12e6c75c757ab33625595919cb7a6f06f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Fri, 22 Jul 2022 17:19:21 +0200 Subject: [PATCH 5/7] fix leftover import --- reactor-core/src/test/java/reactor/core/ExceptionsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java index ff94d0b6ef..5d1e5591a5 100644 --- a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java +++ b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java @@ -27,7 +27,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; -import reactor.TestLoggerExtension; import reactor.core.publisher.Mono; import reactor.test.util.LoggerUtils; import reactor.test.util.RaceTestUtils; From 7c93a853bff6e545df26cd9612007c3663ff9cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Fri, 22 Jul 2022 17:44:26 +0200 Subject: [PATCH 6/7] fix a test asserting no log content now that throwIfFatal logs --- .../java/reactor/core/publisher/LambdaMonoSubscriberTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reactor-core/src/test/java/reactor/core/publisher/LambdaMonoSubscriberTest.java b/reactor-core/src/test/java/reactor/core/publisher/LambdaMonoSubscriberTest.java index 13dc1a34f1..3aa494bf6e 100644 --- a/reactor-core/src/test/java/reactor/core/publisher/LambdaMonoSubscriberTest.java +++ b/reactor-core/src/test/java/reactor/core/publisher/LambdaMonoSubscriberTest.java @@ -209,9 +209,9 @@ void onNextConsumerFatalDoesntTriggerCancellation(TestLogger testLogger) { TestSubscription testSubscription = new TestSubscription(); tested.onSubscribe(testSubscription); - //the error is expected to be thrown as it is fatal, so it doesn't go through onErrorDropped + //the error is expected to be thrown as it is fatal, so it doesn't go through onErrorDropped. However, throwIfJvmFatal now logs it. assertThatExceptionOfType(OutOfMemoryError.class).isThrownBy(() -> tested.onNext("foo")); - Assertions.assertThat(testLogger.getErrContent()).isEmpty(); + Assertions.assertThat(testLogger.getErrContent()).startsWith("[ WARN] throwIfFatal detected a jvm fatal exception, which is thrown and logged below: - java.lang.OutOfMemoryError"); assertThat(testSubscription.isCancelled).as("subscription isCancelled") .isFalse(); From 03ea8b26a628dcd052f66058faaa9266cc9a5d31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Mon, 1 Aug 2022 16:26:42 +0200 Subject: [PATCH 7/7] Use TestLoggerExtension rather than manual log capture setup --- .../java/reactor/core/ExceptionsTest.java | 37 +++++-------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java index 5d1e5591a5..fd05534d3b 100644 --- a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java +++ b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java @@ -24,11 +24,9 @@ import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import org.assertj.core.api.SoftAssertions; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import reactor.core.publisher.Mono; -import reactor.test.util.LoggerUtils; import reactor.test.util.RaceTestUtils; import reactor.test.util.TestLogger; import reactor.util.annotation.Nullable; @@ -238,16 +236,9 @@ void threadDeathIsFatalAndJvmFatal() { .isTrue(); } - @AfterEach - void disableCapture() { - LoggerUtils.disableCapture(); - } - @Test - void throwIfFatalThrowsAndLogsBubbling() { - TestLogger testLogger = new TestLogger(false); - LoggerUtils.enableCaptureWith(testLogger); - + @TestLoggerExtension.Redirect + void throwIfFatalThrowsAndLogsBubbling(TestLogger testLogger) { BubblingException expected = new BubblingException("expected to be logged"); assertThatExceptionOfType(BubblingException.class) @@ -259,10 +250,8 @@ void throwIfFatalThrowsAndLogsBubbling() { } @Test - void throwIfFatalThrowsAndLogsErrorCallbackNotImplemented() { - TestLogger testLogger = new TestLogger(false); - LoggerUtils.enableCaptureWith(testLogger); - + @TestLoggerExtension.Redirect + void throwIfFatalThrowsAndLogsErrorCallbackNotImplemented(TestLogger testLogger) { ErrorCallbackNotImplemented expected = new ErrorCallbackNotImplemented(new IllegalStateException("expected to be logged")); assertThatExceptionOfType(ErrorCallbackNotImplemented.class) @@ -275,10 +264,8 @@ void throwIfFatalThrowsAndLogsErrorCallbackNotImplemented() { } @Test - void throwIfFatalWithJvmFatalErrorsDoesThrowAndLog() { - TestLogger testLogger = new TestLogger(false); - LoggerUtils.enableCaptureWith(testLogger); - + @TestLoggerExtension.Redirect + void throwIfFatalWithJvmFatalErrorsDoesThrowAndLog(TestLogger testLogger) { SoftAssertions.assertSoftly(softly -> { softly.assertThatExceptionOfType(VirtualMachineError.class) .as("VirtualMachineError") @@ -303,10 +290,8 @@ void throwIfFatalWithJvmFatalErrorsDoesThrowAndLog() { } @Test - void throwIfJvmFatalDoesThrowAndLog() { - TestLogger testLogger = new TestLogger(false); - LoggerUtils.enableCaptureWith(testLogger); - + @TestLoggerExtension.Redirect + void throwIfJvmFatalDoesThrowAndLog(TestLogger testLogger) { assertThatExceptionOfType(VirtualMachineError.class) .as("VirtualMachineError") .isThrownBy(() -> Exceptions.throwIfJvmFatal(JVM_FATAL_VIRTUAL_MACHINE_ERROR)) @@ -329,10 +314,8 @@ void throwIfJvmFatalDoesThrowAndLog() { }; @Test - void throwIfJvmFatalDoesntThrowNorLogsOnSimplyFatalExceptions() { - TestLogger testLogger = new TestLogger(false); - LoggerUtils.enableCaptureWith(testLogger); - + @TestLoggerExtension.Redirect + void throwIfJvmFatalDoesntThrowNorLogsOnSimplyFatalExceptions(TestLogger testLogger) { SoftAssertions.assertSoftly(softly -> { softly.assertThatCode(() -> Exceptions.throwIfJvmFatal(new BubblingException("not thrown"))) .doesNotThrowAnyException();