diff --git a/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt b/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt index 21fe496bd3..0f65390dd0 100644 --- a/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt +++ b/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt @@ -27,7 +27,7 @@ class ListAllCoroutineThrowableSubclassesTest { "kotlinx.coroutines.JobCancellationException", "kotlinx.coroutines.internal.UndeliveredElementException", "kotlinx.coroutines.CompletionHandlerException", - "kotlinx.coroutines.DiagnosticCoroutineContextException", + "kotlinx.coroutines.internal.DiagnosticCoroutineContextException", "kotlinx.coroutines.CoroutinesInternalError", "kotlinx.coroutines.channels.ClosedSendChannelException", "kotlinx.coroutines.channels.ClosedReceiveChannelException", diff --git a/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api b/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api index 3a2d08f428..0ff58ae22f 100644 --- a/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api +++ b/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api @@ -182,13 +182,16 @@ public final class kotlinx/coroutines/CoroutineDispatcher$Key : kotlin/coroutine public abstract interface class kotlinx/coroutines/CoroutineExceptionHandler : kotlin/coroutines/CoroutineContext$Element { public static final field Key Lkotlinx/coroutines/CoroutineExceptionHandler$Key; public abstract fun handleException (Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)V + public abstract fun tryHandleException (Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)Z } public final class kotlinx/coroutines/CoroutineExceptionHandler$DefaultImpls { public static fun fold (Lkotlinx/coroutines/CoroutineExceptionHandler;Ljava/lang/Object;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object; public static fun get (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext$Key;)Lkotlin/coroutines/CoroutineContext$Element; + public static fun handleException (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)V public static fun minusKey (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext$Key;)Lkotlin/coroutines/CoroutineContext; public static fun plus (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext;)Lkotlin/coroutines/CoroutineContext; + public static fun tryHandleException (Lkotlinx/coroutines/CoroutineExceptionHandler;Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)Z } public final class kotlinx/coroutines/CoroutineExceptionHandler$Key : kotlin/coroutines/CoroutineContext$Key { diff --git a/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt b/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt index 49923a92e7..ab8f54168a 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 @@ -22,15 +21,15 @@ public fun handleCoroutineException(context: CoroutineContext, exception: Throwa // Invoke an exception handler from the context if present try { context[CoroutineExceptionHandler]?.let { - it.handleException(context, exception) - return + if (it.tryHandleException(context, exception)) + 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 { @@ -47,8 +46,8 @@ internal fun handlerException(originalException: Throwable, thrownException: Thr @Suppress("FunctionName") public inline fun CoroutineExceptionHandler(crossinline handler: (CoroutineContext, Throwable) -> Unit): CoroutineExceptionHandler = object : AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler { - override fun handleException(context: CoroutineContext, exception: Throwable) = - handler.invoke(context, exception) + override fun tryHandleException(context: CoroutineContext, exception: Throwable): Boolean = + true.also { handler.invoke(context, exception) } } /** @@ -105,5 +104,23 @@ public interface CoroutineExceptionHandler : CoroutineContext.Element { * Handles uncaught [exception] in the given [context]. It is invoked * if coroutine has an uncaught exception. */ - public fun handleException(context: CoroutineContext, exception: Throwable) + @Deprecated( + "Use tryHandleException instead", + replaceWith = ReplaceWith("this.tryHandleException(context, exception)"), + level = DeprecationLevel.WARNING + ) + public fun handleException(context: CoroutineContext, exception: Throwable) { + if (!tryHandleException(context, exception)) + handleUncaughtCoroutineException(context, exception) + } + + /** + * Handles uncaught [exception] in the given [context]. + * Returns `true` if the processing was successful and no other processing is needed. + */ + public fun tryHandleException(context: CoroutineContext, exception: Throwable): Boolean { + @Suppress("DEPRECATION") + handleException(context, exception) + return true + } } diff --git a/kotlinx-coroutines-core/common/src/internal/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/common/src/internal/CoroutineExceptionHandlerImpl.kt new file mode 100644 index 0000000000..56f26cab1d --- /dev/null +++ b/kotlinx-coroutines-core/common/src/internal/CoroutineExceptionHandlerImpl.kt @@ -0,0 +1,71 @@ +/* + * 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 { + if (handler.tryHandleException(context, exception)) + 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/CoroutineExceptionHandlerImplJvm.kt b/kotlinx-coroutines-core/jvm/src/internal/CoroutineExceptionHandlerImplJvm.kt new file mode 100644 index 0000000000..7f11898a09 --- /dev/null +++ b/kotlinx-coroutines-core/jvm/src/internal/CoroutineExceptionHandlerImplJvm.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/jvm/test/exceptions/Exceptions.kt b/kotlinx-coroutines-core/jvm/test/exceptions/Exceptions.kt index 4849f52071..0c96d10b26 100644 --- a/kotlinx-coroutines-core/jvm/test/exceptions/Exceptions.kt +++ b/kotlinx-coroutines-core/jvm/test/exceptions/Exceptions.kt @@ -39,7 +39,7 @@ class CapturingHandler : AbstractCoroutineContextElement(CoroutineExceptionHandl { private var unhandled: ArrayList? = ArrayList() - override fun handleException(context: CoroutineContext, exception: Throwable) = synchronized(this) { + override fun tryHandleException(context: CoroutineContext, exception: Throwable) = synchronized(this) { unhandled!!.add(exception) } 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 15d48a2ae2..0a0fcc6718 100644 --- a/kotlinx-coroutines-test/common/src/TestScope.kt +++ b/kotlinx-coroutines-test/common/src/TestScope.kt @@ -220,6 +220,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()) { @@ -234,6 +242,8 @@ internal class TestScopeImpl(context: CoroutineContext) : fun leave(): 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..a25cf8400d --- /dev/null +++ b/kotlinx-coroutines-test/common/src/internal/ExceptionCollector.kt @@ -0,0 +1,91 @@ +/* + * 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. + */ + override fun tryHandleException(context: CoroutineContext, 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 + } + + 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/RunTestTest.kt b/kotlinx-coroutines-test/common/test/RunTestTest.kt index 0315543d54..4a804cad30 100644 --- a/kotlinx-coroutines-test/common/test/RunTestTest.kt +++ b/kotlinx-coroutines-test/common/test/RunTestTest.kt @@ -160,7 +160,12 @@ class RunTestTest { } }) { runTest(dispatchTimeoutMs = 1) { - coroutineContext[CoroutineExceptionHandler]!!.handleException(coroutineContext, TestException("A")) + assertTrue( + coroutineContext[CoroutineExceptionHandler]!!.tryHandleException( + coroutineContext, + TestException("A") + ) + ) withContext(Dispatchers.Default) { delay(10000) 3 diff --git a/kotlinx-coroutines-test/common/test/TestScopeTest.kt b/kotlinx-coroutines-test/common/test/TestScopeTest.kt index 45f7f3ef80..ced8c5d781 100644 --- a/kotlinx-coroutines-test/common/test/TestScopeTest.kt +++ b/kotlinx-coroutines-test/common/test/TestScopeTest.kt @@ -476,6 +476,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..ee6e63f4c5 100644 --- a/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineExceptionHandler.kt +++ b/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineExceptionHandler.kt @@ -56,14 +56,13 @@ public class TestCoroutineExceptionHandler : private var _coroutinesCleanedUp = false @Suppress("INVISIBLE_MEMBER") - override fun handleException(context: CoroutineContext, exception: Throwable) { + override fun tryHandleException(context: CoroutineContext, exception: Throwable): Boolean = synchronized(_lock) { - if (_coroutinesCleanedUp) { - handleCoroutineExceptionImpl(context, exception) + if (!_coroutinesCleanedUp) { + _exceptions += exception } - _exceptions += exception + !_coroutinesCleanedUp } - } public override val uncaughtExceptions: List get() = synchronized(_lock) { _exceptions.toList() } diff --git a/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineScope.kt b/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineScope.kt index 5af83f5197..3e5b061e25 100644 --- a/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineScope.kt +++ b/kotlinx-coroutines-test/jvm/src/migration/TestCoroutineScope.kt @@ -187,10 +187,8 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo var scope: TestCoroutineScopeImpl? = null val ownExceptionHandler = object : AbstractCoroutineContextElement(CoroutineExceptionHandler), TestCoroutineScopeExceptionHandler { - override fun handleException(context: CoroutineContext, exception: Throwable) { - if (!scope!!.reportException(exception)) - throw exception // let this exception crash everything - } + override fun tryHandleException(context: CoroutineContext, exception: Throwable) = + scope!!.reportException(exception) } val exceptionHandler = when (val exceptionHandler = ctxWithDispatcher[CoroutineExceptionHandler]) { is UncaughtExceptionCaptor -> exceptionHandler 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/jvm/test/migration/RunTestLegacyScopeTest.kt b/kotlinx-coroutines-test/jvm/test/migration/RunTestLegacyScopeTest.kt index ed5b1577f5..1c984155ea 100644 --- a/kotlinx-coroutines-test/jvm/test/migration/RunTestLegacyScopeTest.kt +++ b/kotlinx-coroutines-test/jvm/test/migration/RunTestLegacyScopeTest.kt @@ -101,7 +101,12 @@ class RunTestLegacyScopeTest { } }) { runTestWithLegacyScope(dispatchTimeoutMs = 1) { - coroutineContext[CoroutineExceptionHandler]!!.handleException(coroutineContext, TestException("A")) + assertTrue( + coroutineContext[CoroutineExceptionHandler]!!.tryHandleException( + coroutineContext, + TestException("A") + ) + ) withContext(Dispatchers.Default) { delay(10000) 3 diff --git a/kotlinx-coroutines-test/jvm/test/migration/TestCoroutineExceptionHandlerTest.kt b/kotlinx-coroutines-test/jvm/test/migration/TestCoroutineExceptionHandlerTest.kt index 20da130725..6b82386c29 100644 --- a/kotlinx-coroutines-test/jvm/test/migration/TestCoroutineExceptionHandlerTest.kt +++ b/kotlinx-coroutines-test/jvm/test/migration/TestCoroutineExceptionHandlerTest.kt @@ -12,7 +12,7 @@ class TestCoroutineExceptionHandlerTest { fun whenExceptionsCaught_availableViaProperty() { val subject = TestCoroutineExceptionHandler() val expected = IllegalArgumentException() - subject.handleException(subject, expected) + assertTrue(subject.tryHandleException(subject, expected)) assertEquals(listOf(expected), subject.uncaughtExceptions) } -} \ No newline at end of file +} 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)) } } diff --git a/ui/kotlinx-coroutines-android/src/AndroidExceptionPreHandler.kt b/ui/kotlinx-coroutines-android/src/AndroidExceptionPreHandler.kt index 0bc603ea1e..013daf0a9f 100644 --- a/ui/kotlinx-coroutines-android/src/AndroidExceptionPreHandler.kt +++ b/ui/kotlinx-coroutines-android/src/AndroidExceptionPreHandler.kt @@ -30,7 +30,7 @@ internal class AndroidExceptionPreHandler : return declared } - override fun handleException(context: CoroutineContext, exception: Throwable) { + override fun tryHandleException(context: CoroutineContext, exception: Throwable): Boolean { /* * Android Oreo introduced private API for a global pre-handler for uncaught exceptions, to ensure that the * exceptions are logged even if the default uncaught exception handler is replaced by the app. The pre-handler @@ -48,5 +48,6 @@ internal class AndroidExceptionPreHandler : (preHandler()?.invoke(null) as? Thread.UncaughtExceptionHandler) ?.uncaughtException(Thread.currentThread(), exception) } + return false // let the default handler, too, handle the exception } }