From 4fda39aaece09926fceca6e49271de1d0843858a Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 12 Sep 2022 16:26:11 +0200 Subject: [PATCH] Attempt to report uncaught exceptions in the test module When when a `Job` doesn't have a parent, failures in it can not be reported via structured concurrency. Instead, the mechanism of unhandled exceptions is used. If there is a `CoroutineExceptionHandler` in the coroutine context or registered as a `ServiceLoader` service, this gets notified, and otherwise, something platform-specific happens: on Native, the program crashes, and on the JVM, by default, the exception is just logged, though this is configurable via `Thread.setUncaughtExceptionHandler`. With tests on the JVM, this is an issue: we want exceptions not simply *logged*, we want them to fail the test, and this extends beyond just coroutines. However, JUnit does not override the uncaught exception handler, and uncaught exceptions do just get logged: This is a problem with a widely-used `viewModelScope` on Android. This is a scope without a parent, and so the exceptions in it are uncaught. On Android, such uncaught exceptions crash the app by default, but in tests, they just get logged. Clearly, without overriding the behavior of uncaught exceptions, the tests are lacking. This can be solved on the test writers' side via `setUncaughtExceptionHandler`, but one has to remember to do that. In this commit, we attempt to solve this issue for the overwhelming majority of users. To that end, when the test framework is used, we collect the uncaught exceptions and report them at the end of a test. This approach is marginally less robust than `setUncaughtExceptionHandler`: if an exception happened after the last test using `kotlinx-coroutines-test`, it won't get reported, for example. `CoroutineExceptionHandler` is designed in such a way that, when it is used in a coroutine context, its presence means that the exceptions are safe in its care and will not be propagated further, but when used as a service, it has no such property. We, however, know that our `CoroutineExceptionHandler` reports the exceptions properly and they don't need to be further logged, and so we had to extend the behavior of mechanism for uncaught exception handling so that it the handler throws a new kind of exception if the exception was processed successfully. Also, because there's no `ServiceLoader` mechanism on JS or Native, we had to refactor the whole uncaught exception handling mechanism a bit in the same vein as we had to adapt the `Main` dispatcher to `Dispatchers.setMain`: by introducing internal setter APIs that services have to call manually to register in. Fixes #1205 as thoroughly as we can given the circumstances. --- ...ListAllCoroutineThrowableSubclassesTest.kt | 3 +- .../common/src/CoroutineExceptionHandler.kt | 7 +- .../CoroutineExceptionHandlerImpl.common.kt | 72 ++++++++++++++ .../js/src/CoroutineExceptionHandlerImpl.kt | 12 --- .../internal/CoroutineExceptionHandlerImpl.kt | 26 +++++ .../jvm/src/CoroutineExceptionHandlerImpl.kt | 62 ------------ .../internal/CoroutineExceptionHandlerImpl.kt | 49 ++++++++++ .../src/CoroutineExceptionHandlerImpl.kt | 14 --- .../internal/CoroutineExceptionHandlerImpl.kt | 31 ++++++ .../common/src/TestScope.kt | 12 +++ .../common/src/internal/ExceptionCollector.kt | 98 +++++++++++++++++++ .../common/test/Helpers.kt | 27 ++++- .../common/test/TestScopeTest.kt | 56 +++++++++++ kotlinx-coroutines-test/js/test/Helpers.kt | 13 +-- ...tlinx.coroutines.CoroutineExceptionHandler | 1 + .../TestCoroutineExceptionHandler.kt | 2 +- .../jvm/src/module-info.java | 3 + .../jvm/test/HelpersJvm.kt | 9 +- .../native/test/Helpers.kt | 9 +- 19 files changed, 396 insertions(+), 110 deletions(-) create mode 100644 kotlinx-coroutines-core/common/src/internal/CoroutineExceptionHandlerImpl.common.kt delete mode 100644 kotlinx-coroutines-core/js/src/CoroutineExceptionHandlerImpl.kt create mode 100644 kotlinx-coroutines-core/js/src/internal/CoroutineExceptionHandlerImpl.kt delete mode 100644 kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt create mode 100644 kotlinx-coroutines-core/jvm/src/internal/CoroutineExceptionHandlerImpl.kt delete mode 100644 kotlinx-coroutines-core/native/src/CoroutineExceptionHandlerImpl.kt create mode 100644 kotlinx-coroutines-core/native/src/internal/CoroutineExceptionHandlerImpl.kt create mode 100644 kotlinx-coroutines-test/common/src/internal/ExceptionCollector.kt create mode 100644 kotlinx-coroutines-test/jvm/resources/META-INF/services/kotlinx.coroutines.CoroutineExceptionHandler diff --git a/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt b/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt index 21fe496bd3..7253658e9b 100644 --- a/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt +++ b/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt @@ -27,7 +27,8 @@ class ListAllCoroutineThrowableSubclassesTest { "kotlinx.coroutines.JobCancellationException", "kotlinx.coroutines.internal.UndeliveredElementException", "kotlinx.coroutines.CompletionHandlerException", - "kotlinx.coroutines.DiagnosticCoroutineContextException", + "kotlinx.coroutines.internal.DiagnosticCoroutineContextException", + "kotlinx.coroutines.internal.ExceptionSuccessfullyProcessed", "kotlinx.coroutines.CoroutinesInternalError", "kotlinx.coroutines.channels.ClosedSendChannelException", "kotlinx.coroutines.channels.ClosedReceiveChannelException", diff --git a/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt b/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt index d13ef67c66..e641447ba3 100644 --- a/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt +++ b/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt @@ -4,10 +4,9 @@ package kotlinx.coroutines +import kotlinx.coroutines.internal.* import kotlin.coroutines.* -internal expect fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable) - /** * Helper function for coroutine builder implementations to handle uncaught and unexpected exceptions in coroutines, * that could not be otherwise handled in a normal way through structured concurrency, saving them to a future, and @@ -26,11 +25,11 @@ public fun handleCoroutineException(context: CoroutineContext, exception: Throwa return } } catch (t: Throwable) { - handleCoroutineExceptionImpl(context, handlerException(exception, t)) + handleUncaughtCoroutineException(context, handlerException(exception, t)) return } // If a handler is not present in the context or an exception was thrown, fallback to the global handler - handleCoroutineExceptionImpl(context, exception) + handleUncaughtCoroutineException(context, exception) } internal fun handlerException(originalException: Throwable, thrownException: Throwable): Throwable { diff --git a/kotlinx-coroutines-core/common/src/internal/CoroutineExceptionHandlerImpl.common.kt b/kotlinx-coroutines-core/common/src/internal/CoroutineExceptionHandlerImpl.common.kt new file mode 100644 index 0000000000..3f5925a3ee --- /dev/null +++ b/kotlinx-coroutines-core/common/src/internal/CoroutineExceptionHandlerImpl.common.kt @@ -0,0 +1,72 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.internal + +import kotlinx.coroutines.* +import kotlin.coroutines.* + +/** + * The list of globally installed [CoroutineExceptionHandler] instances that will be notified of any exceptions that + * were not processed in any other manner. + */ +internal expect val platformExceptionHandlers: Collection + +/** + * Ensures that the given [callback] is present in the [platformExceptionHandlers] list. + */ +internal expect fun ensurePlatformExceptionHandlerLoaded(callback: CoroutineExceptionHandler) + +/** + * The platform-dependent global exception handler, used so that the exception is logged at least *somewhere*. + */ +internal expect fun propagateExceptionFinalResort(exception: Throwable) + +/** + * Deal with exceptions that happened in coroutines and weren't programmatically dealt with. + * + * First, it notifies every [CoroutineExceptionHandler] in the [platformExceptionHandlers] list. + * If one of them throws [ExceptionSuccessfullyProcessed], it means that that handler believes that the exception was + * dealt with sufficiently well and doesn't need any further processing. + * Otherwise, the platform-dependent global exception handler is also invoked. + */ +internal fun handleUncaughtCoroutineException(context: CoroutineContext, exception: Throwable) { + // use additional extension handlers + for (handler in platformExceptionHandlers) { + try { + handler.handleException(context, exception) + } catch (_: ExceptionSuccessfullyProcessed) { + return + } catch (t: Throwable) { + propagateExceptionFinalResort(handlerException(exception, t)) + } + } + + try { + exception.addSuppressed(DiagnosticCoroutineContextException(context)) + } catch (e: Throwable) { + // addSuppressed is never user-defined and cannot normally throw with the only exception being OOM + // we do ignore that just in case to definitely deliver the exception + } + propagateExceptionFinalResort(exception) +} + +/** + * Private exception that is added to suppressed exceptions of the original exception + * when it is reported to the last-ditch current thread 'uncaughtExceptionHandler'. + * + * The purpose of this exception is to add an otherwise inaccessible diagnostic information and to + * be able to poke the context of the failing coroutine in the debugger. + */ +internal expect class DiagnosticCoroutineContextException(context: CoroutineContext) : RuntimeException + +/** + * A dummy exception that signifies that the exception was successfully processed by the handler and no further + * action is required. + * + * Would be nicer if [CoroutineExceptionHandler] could return a boolean, but that would be a breaking change. + * For now, we will take solace in knowledge that such exceptions are exceedingly rare, even rarer than globally + * uncaught exceptions in general. + */ +internal object ExceptionSuccessfullyProcessed : Exception() diff --git a/kotlinx-coroutines-core/js/src/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/js/src/CoroutineExceptionHandlerImpl.kt deleted file mode 100644 index 54a65e10a6..0000000000 --- a/kotlinx-coroutines-core/js/src/CoroutineExceptionHandlerImpl.kt +++ /dev/null @@ -1,12 +0,0 @@ -/* - * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. - */ - -package kotlinx.coroutines - -import kotlin.coroutines.* - -internal actual fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable) { - // log exception - console.error(exception) -} diff --git a/kotlinx-coroutines-core/js/src/internal/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/js/src/internal/CoroutineExceptionHandlerImpl.kt new file mode 100644 index 0000000000..675cc4a67a --- /dev/null +++ b/kotlinx-coroutines-core/js/src/internal/CoroutineExceptionHandlerImpl.kt @@ -0,0 +1,26 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.internal + +import kotlinx.coroutines.* +import kotlin.coroutines.* + +private val platformExceptionHandlers_ = mutableSetOf() + +internal actual val platformExceptionHandlers: Collection + get() = platformExceptionHandlers_ + +internal actual fun ensurePlatformExceptionHandlerLoaded(callback: CoroutineExceptionHandler) { + platformExceptionHandlers_ += callback +} + +internal actual fun propagateExceptionFinalResort(exception: Throwable) { + // log exception + console.error(exception) +} + +internal actual class DiagnosticCoroutineContextException actual constructor(context: CoroutineContext) : + RuntimeException(context.toString()) + diff --git a/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt deleted file mode 100644 index 0d68b047f4..0000000000 --- a/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. - */ - -package kotlinx.coroutines - -import java.util.* -import kotlin.coroutines.* - -/** - * A list of globally installed [CoroutineExceptionHandler] instances. - * - * Note that Android may have dummy [Thread.contextClassLoader] which is used by one-argument [ServiceLoader.load] function, - * see (https://stackoverflow.com/questions/13407006/android-class-loader-may-fail-for-processes-that-host-multiple-applications). - * So here we explicitly use two-argument `load` with a class-loader of [CoroutineExceptionHandler] class. - * - * We are explicitly using the `ServiceLoader.load(MyClass::class.java, MyClass::class.java.classLoader).iterator()` - * form of the ServiceLoader call to enable R8 optimization when compiled on Android. - */ -private val handlers: List = ServiceLoader.load( - CoroutineExceptionHandler::class.java, - CoroutineExceptionHandler::class.java.classLoader -).iterator().asSequence().toList() - -/** - * Private exception without stacktrace that is added to suppressed exceptions of the original exception - * when it is reported to the last-ditch current thread 'uncaughtExceptionHandler'. - * - * The purpose of this exception is to add an otherwise inaccessible diagnostic information and to - * be able to poke the failing coroutine context in the debugger. - */ -private class DiagnosticCoroutineContextException(@Transient private val context: CoroutineContext) : RuntimeException() { - override fun getLocalizedMessage(): String { - return context.toString() - } - - override fun fillInStackTrace(): Throwable { - // Prevent Android <= 6.0 bug, #1866 - stackTrace = emptyArray() - return this - } -} - -internal actual fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable) { - // use additional extension handlers - for (handler in handlers) { - try { - handler.handleException(context, exception) - } catch (t: Throwable) { - // Use thread's handler if custom handler failed to handle exception - val currentThread = Thread.currentThread() - currentThread.uncaughtExceptionHandler.uncaughtException(currentThread, handlerException(exception, t)) - } - } - - // use thread's handler - val currentThread = Thread.currentThread() - // addSuppressed is never user-defined and cannot normally throw with the only exception being OOM - // we do ignore that just in case to definitely deliver the exception - runCatching { exception.addSuppressed(DiagnosticCoroutineContextException(context)) } - currentThread.uncaughtExceptionHandler.uncaughtException(currentThread, exception) -} diff --git a/kotlinx-coroutines-core/jvm/src/internal/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/jvm/src/internal/CoroutineExceptionHandlerImpl.kt new file mode 100644 index 0000000000..7f11898a09 --- /dev/null +++ b/kotlinx-coroutines-core/jvm/src/internal/CoroutineExceptionHandlerImpl.kt @@ -0,0 +1,49 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.internal + +import java.util.* +import kotlinx.coroutines.* +import kotlin.coroutines.* + +/** + * A list of globally installed [CoroutineExceptionHandler] instances. + * + * Note that Android may have dummy [Thread.contextClassLoader] which is used by one-argument [ServiceLoader.load] function, + * see (https://stackoverflow.com/questions/13407006/android-class-loader-may-fail-for-processes-that-host-multiple-applications). + * So here we explicitly use two-argument `load` with a class-loader of [CoroutineExceptionHandler] class. + * + * We are explicitly using the `ServiceLoader.load(MyClass::class.java, MyClass::class.java.classLoader).iterator()` + * form of the ServiceLoader call to enable R8 optimization when compiled on Android. + */ +internal actual val platformExceptionHandlers: Collection = ServiceLoader.load( + CoroutineExceptionHandler::class.java, + CoroutineExceptionHandler::class.java.classLoader +).iterator().asSequence().toList() + +internal actual fun ensurePlatformExceptionHandlerLoaded(callback: CoroutineExceptionHandler) { + // we use JVM's mechanism of ServiceLoader, so this should be a no-op on JVM. + // The only thing we do is make sure that the ServiceLoader did work correctly. + check(callback in platformExceptionHandlers) { "Exception handler was not found via a ServiceLoader" } +} + +internal actual fun propagateExceptionFinalResort(exception: Throwable) { + // use the thread's handler + val currentThread = Thread.currentThread() + currentThread.uncaughtExceptionHandler.uncaughtException(currentThread, exception) +} + +// This implementation doesn't store a stacktrace, which is good because a stacktrace doesn't make sense for this. +internal actual class DiagnosticCoroutineContextException actual constructor(@Transient private val context: CoroutineContext) : RuntimeException() { + override fun getLocalizedMessage(): String { + return context.toString() + } + + override fun fillInStackTrace(): Throwable { + // Prevent Android <= 6.0 bug, #1866 + stackTrace = emptyArray() + return this + } +} diff --git a/kotlinx-coroutines-core/native/src/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/native/src/CoroutineExceptionHandlerImpl.kt deleted file mode 100644 index 434813dc29..0000000000 --- a/kotlinx-coroutines-core/native/src/CoroutineExceptionHandlerImpl.kt +++ /dev/null @@ -1,14 +0,0 @@ -/* - * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. - */ - -package kotlinx.coroutines - -import kotlin.coroutines.* -import kotlin.native.* - -@OptIn(ExperimentalStdlibApi::class) -internal actual fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable) { - // log exception - processUnhandledException(exception) -} diff --git a/kotlinx-coroutines-core/native/src/internal/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/native/src/internal/CoroutineExceptionHandlerImpl.kt new file mode 100644 index 0000000000..43d776cb54 --- /dev/null +++ b/kotlinx-coroutines-core/native/src/internal/CoroutineExceptionHandlerImpl.kt @@ -0,0 +1,31 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.internal + +import kotlinx.coroutines.* +import kotlin.coroutines.* +import kotlin.native.* + +private val lock = SynchronizedObject() + +internal actual val platformExceptionHandlers: Collection + get() = synchronized(lock) { platformExceptionHandlers_ } + +private val platformExceptionHandlers_ = mutableSetOf() + +internal actual fun ensurePlatformExceptionHandlerLoaded(callback: CoroutineExceptionHandler) { + synchronized(lock) { + platformExceptionHandlers_ += callback + } +} + +@OptIn(ExperimentalStdlibApi::class) +internal actual fun propagateExceptionFinalResort(exception: Throwable) { + // log exception + processUnhandledException(exception) +} + +internal actual class DiagnosticCoroutineContextException actual constructor(context: CoroutineContext) : + RuntimeException(context.toString()) diff --git a/kotlinx-coroutines-test/common/src/TestScope.kt b/kotlinx-coroutines-test/common/src/TestScope.kt index a0a29a9553..713f511909 100644 --- a/kotlinx-coroutines-test/common/src/TestScope.kt +++ b/kotlinx-coroutines-test/common/src/TestScope.kt @@ -226,6 +226,14 @@ internal class TestScopeImpl(context: CoroutineContext) : throw IllegalStateException("Only a single call to `runTest` can be performed during one test.") entered = true check(!finished) + /** the order is important: [reportException] is only guaranteed not to throw if [entered] is `true` but + * [finished] is `false`. + * However, we also want [uncaughtExceptions] to be queried after the callback is registered, + * because the exception collector will be able to report the exceptions that arrived before this test but + * after the previous one, and learning about such exceptions as soon is possible is nice. */ + @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") + run { ensurePlatformExceptionHandlerLoaded(ExceptionCollector) } + ExceptionCollector.addOnExceptionCallback(lock, this::reportException) uncaughtExceptions } if (exceptions.isNotEmpty()) { @@ -239,6 +247,8 @@ internal class TestScopeImpl(context: CoroutineContext) : /** Called at the end of the test. May only be called once. Returns the list of caught unhandled exceptions. */ fun leave(): List = synchronized(lock) { check(entered && !finished) + /** After [finished] becomes `true`, it is no longer valid to have [reportException] as the callback. */ + ExceptionCollector.removeOnExceptionCallback(lock) finished = true uncaughtExceptions } @@ -247,6 +257,8 @@ internal class TestScopeImpl(context: CoroutineContext) : fun legacyLeave(): List { val exceptions = synchronized(lock) { check(entered && !finished) + /** After [finished] becomes `true`, it is no longer valid to have [reportException] as the callback. */ + ExceptionCollector.removeOnExceptionCallback(lock) finished = true uncaughtExceptions } diff --git a/kotlinx-coroutines-test/common/src/internal/ExceptionCollector.kt b/kotlinx-coroutines-test/common/src/internal/ExceptionCollector.kt new file mode 100644 index 0000000000..90fa763523 --- /dev/null +++ b/kotlinx-coroutines-test/common/src/internal/ExceptionCollector.kt @@ -0,0 +1,98 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.test.internal + +import kotlinx.coroutines.* +import kotlinx.coroutines.internal.* +import kotlin.coroutines.* + +/** + * If [addOnExceptionCallback] is called, the provided callback will be evaluated each time + * [handleCoroutineException] is executed and can't find a [CoroutineExceptionHandler] to + * process the exception. + * + * When a callback is registered once, even if it's later removed, the system starts to assume that + * other callbacks will eventually be registered, and so collects the exceptions. + * Once a new callback is registered, the collected exceptions are used with it. + * + * The callbacks in this object are the last resort before relying on platform-dependent + * ways to report uncaught exceptions from coroutines. + */ +internal object ExceptionCollector : AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler { + private val lock = SynchronizedObject() + private var enabled = false + private val unprocessedExceptions = mutableListOf() + private val callbacks = mutableMapOf Unit>() + + /** + * Registers [callback] to be executed when an uncaught exception happens. + * [owner] is a key by which to distinguish different callbacks. + */ + fun addOnExceptionCallback(owner: Any, callback: (Throwable) -> Unit) = synchronized(lock) { + enabled = true // never becomes `false` again + val previousValue = callbacks.put(owner, callback) + check(previousValue === null) + // try to process the exceptions using the newly-registered callback + unprocessedExceptions.forEach { reportException(it) } + unprocessedExceptions.clear() + } + + /** + * Unregisters the callback associated with [owner]. + */ + fun removeOnExceptionCallback(owner: Any) = synchronized(lock) { + val existingValue = callbacks.remove(owner) + check(existingValue !== null) + } + + /** + * Tries to handle the exception by propagating it to an interested consumer. + * Returns `true` if the exception does not need further processing. + * + * Doesn't throw. + */ + fun handleException(exception: Throwable): Boolean = synchronized(lock) { + if (!enabled) return false + if (reportException(exception)) return true + /** we don't return the result of the `add` function because we don't have a guarantee + * that a callback will eventually appear and collect the unprocessed exceptions, so + * we can't consider [exception] to be properly handled. */ + unprocessedExceptions.add(exception) + return false + } + + /** + * Try to report [exception] to the existing callbacks. + */ + private fun reportException(exception: Throwable): Boolean { + var executedACallback = false + for (callback in callbacks.values) { + callback(exception) + executedACallback = true + /** We don't leave the function here because we want to fan-out the exceptions to every interested consumer, + * it's not enough to have the exception processed by one of them. + * The reason is, it's less big of a deal to observe multiple concurrent reports of bad behavior than not + * to observe the report in the exact callback that is connected to that bad behavior. */ + } + return executedACallback + } + + @Suppress("INVISIBLE_MEMBER") + override fun handleException(context: CoroutineContext, exception: Throwable) { + if (handleException(exception)) { + throw ExceptionSuccessfullyProcessed + } + } + + override fun equals(other: Any?): Boolean = other is ExceptionCollector || other is ExceptionCollectorAsService +} + +/** + * A workaround for being unable to treat an object as a `ServiceLoader` service. + */ +internal class ExceptionCollectorAsService: CoroutineExceptionHandler by ExceptionCollector { + override fun equals(other: Any?): Boolean = other is ExceptionCollectorAsService || other is ExceptionCollector + override fun hashCode(): Int = ExceptionCollector.hashCode() +} diff --git a/kotlinx-coroutines-test/common/test/Helpers.kt b/kotlinx-coroutines-test/common/test/Helpers.kt index 98375b0905..345c66f91a 100644 --- a/kotlinx-coroutines-test/common/test/Helpers.kt +++ b/kotlinx-coroutines-test/common/test/Helpers.kt @@ -31,9 +31,32 @@ inline fun assertRunsFast(timeout: Duration, block: () -> T): T { inline fun assertRunsFast(block: () -> T): T = assertRunsFast(2.seconds, block) /** - * Passes [test] as an argument to [block], but as a function returning not a [TestResult] but [Unit]. + * Runs [test], and then invokes [block], passing to it the lambda that functionally behaves + * the same way [test] does. */ -expect fun testResultMap(block: (() -> Unit) -> Unit, test: () -> TestResult): TestResult +fun testResultMap(block: (() -> Unit) -> Unit, test: () -> TestResult): TestResult = testResultChain( + block = test, + after = { + block { it.getOrThrow() } + createTestResult { } + } +) + +/** + * Chains together [block] and [after], passing the result of [block] to [after]. + */ +expect fun testResultChain(block: () -> TestResult, after: (Result) -> TestResult): TestResult + +fun testResultChain(vararg chained: (Result) -> TestResult): TestResult = + if (chained.isEmpty()) { + createTestResult { } + } else { + testResultChain(block = { + chained[0](Result.success(Unit)) + }) { + testResultChain(*chained.drop(1).toTypedArray()) + } + } class TestException(message: String? = null): Exception(message) diff --git a/kotlinx-coroutines-test/common/test/TestScopeTest.kt b/kotlinx-coroutines-test/common/test/TestScopeTest.kt index a21e9168f1..433faef7ac 100644 --- a/kotlinx-coroutines-test/common/test/TestScopeTest.kt +++ b/kotlinx-coroutines-test/common/test/TestScopeTest.kt @@ -491,6 +491,62 @@ class TestScopeTest { } } + /* + * Tests that the [TestScope] exception reporting mechanism will report the exceptions that happen between + * different tests. + * + * This test must be ran manually, because such exceptions still go through the global exception handler + * (as there's no guarantee that another test will happen), and the global exception handler will + * log the exceptions or, on Native, crash the test suite. + */ + @Test + @Ignore + fun testReportingStrayUncaughtExceptionsBetweenTests() { + val thrown = TestException("x") + testResultChain({ + // register a handler for uncaught exceptions + runTest { } + }, { + GlobalScope.launch(start = CoroutineStart.UNDISPATCHED) { + throw thrown + } + runTest { + fail("unreached") + } + }, { + // this `runTest` will not report the exception + runTest { + when (val exception = it.exceptionOrNull()) { + is UncaughtExceptionsBeforeTest -> { + assertEquals(1, exception.suppressedExceptions.size) + assertSame(exception.suppressedExceptions[0], thrown) + } + else -> fail("unexpected exception: $exception") + } + } + }) + } + + /** + * Tests that the uncaught exceptions that happen during the test are reported. + */ + @Test + fun testReportingStrayUncaughtExceptionsDuringTest(): TestResult { + val thrown = TestException("x") + return testResultChain({ _ -> + runTest { + val job = launch(Dispatchers.Default + NonCancellable) { + throw thrown + } + job.join() + } + }, { + runTest { + assertEquals(thrown, it.exceptionOrNull()) + } + }) + } + companion object { internal val invalidContexts = listOf( Dispatchers.Default, // not a [TestDispatcher] diff --git a/kotlinx-coroutines-test/js/test/Helpers.kt b/kotlinx-coroutines-test/js/test/Helpers.kt index 5f19d1ac58..5fd0291c3b 100644 --- a/kotlinx-coroutines-test/js/test/Helpers.kt +++ b/kotlinx-coroutines-test/js/test/Helpers.kt @@ -1,20 +1,17 @@ /* - * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ package kotlinx.coroutines.test import kotlin.test.* -actual fun testResultMap(block: (() -> Unit) -> Unit, test: () -> TestResult): TestResult = - test().then( +actual fun testResultChain(block: () -> TestResult, after: (Result) -> TestResult): TestResult = + block().then( { - block { - } + after(Result.success(Unit)) }, { - block { - throw it - } + after(Result.failure(it)) }) actual typealias NoJs = Ignore diff --git a/kotlinx-coroutines-test/jvm/resources/META-INF/services/kotlinx.coroutines.CoroutineExceptionHandler b/kotlinx-coroutines-test/jvm/resources/META-INF/services/kotlinx.coroutines.CoroutineExceptionHandler new file mode 100644 index 0000000000..c9aaec2e60 --- /dev/null +++ b/kotlinx-coroutines-test/jvm/resources/META-INF/services/kotlinx.coroutines.CoroutineExceptionHandler @@ -0,0 +1 @@ +kotlinx.coroutines.test.internal.ExceptionCollectorAsService diff --git a/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineExceptionHandler.kt b/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineExceptionHandler.kt index aeb0f35882..5330cd3ddf 100644 --- a/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineExceptionHandler.kt +++ b/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineExceptionHandler.kt @@ -59,7 +59,7 @@ public class TestCoroutineExceptionHandler : override fun handleException(context: CoroutineContext, exception: Throwable) { synchronized(_lock) { if (_coroutinesCleanedUp) { - handleCoroutineExceptionImpl(context, exception) + handleUncaughtCoroutineException(context, exception) } _exceptions += exception } diff --git a/kotlinx-coroutines-test/jvm/src/module-info.java b/kotlinx-coroutines-test/jvm/src/module-info.java index f67ce6a161..40ee87d8af 100644 --- a/kotlinx-coroutines-test/jvm/src/module-info.java +++ b/kotlinx-coroutines-test/jvm/src/module-info.java @@ -1,4 +1,6 @@ +import kotlinx.coroutines.CoroutineExceptionHandler; import kotlinx.coroutines.internal.MainDispatcherFactory; +import kotlinx.coroutines.test.internal.ExceptionCollectorAsService; import kotlinx.coroutines.test.internal.TestMainDispatcherFactory; module kotlinx.coroutines.test { @@ -8,4 +10,5 @@ exports kotlinx.coroutines.test; provides MainDispatcherFactory with TestMainDispatcherFactory; + provides CoroutineExceptionHandler with ExceptionCollectorAsService; } diff --git a/kotlinx-coroutines-test/jvm/test/HelpersJvm.kt b/kotlinx-coroutines-test/jvm/test/HelpersJvm.kt index e9aa3ff747..8d40b078a3 100644 --- a/kotlinx-coroutines-test/jvm/test/HelpersJvm.kt +++ b/kotlinx-coroutines-test/jvm/test/HelpersJvm.kt @@ -3,8 +3,11 @@ */ package kotlinx.coroutines.test -actual fun testResultMap(block: (() -> Unit) -> Unit, test: () -> TestResult) { - block { - test() +actual fun testResultChain(block: () -> TestResult, after: (Result) -> TestResult): TestResult { + try { + block() + after(Result.success(Unit)) + } catch (e: Throwable) { + after(Result.failure(e)) } } diff --git a/kotlinx-coroutines-test/native/test/Helpers.kt b/kotlinx-coroutines-test/native/test/Helpers.kt index ef478b7eb1..be615fb022 100644 --- a/kotlinx-coroutines-test/native/test/Helpers.kt +++ b/kotlinx-coroutines-test/native/test/Helpers.kt @@ -5,9 +5,12 @@ package kotlinx.coroutines.test import kotlin.test.* -actual fun testResultMap(block: (() -> Unit) -> Unit, test: () -> TestResult) { - block { - test() +actual fun testResultChain(block: () -> TestResult, after: (Result) -> TestResult): TestResult { + try { + block() + after(Result.success(Unit)) + } catch (e: Throwable) { + after(Result.failure(e)) } }