-
Notifications
You must be signed in to change notification settings - Fork 19
Description
The runWrapped method catches all Throwable, which includes CancellationException and that is one that shouldn't be wrapped.
public fun <R> runWrapped(block: () -> R): R =
try {
block()
} catch (t: Throwable) {
if (t is PowerSyncException) {
Logger.e("PowerSyncException: ${t.message}")
throw t
} else {
Logger.e("PowerSyncException: ${t.message}")
throw PowerSyncException(t.message ?: "Unknown internal exception", t)
}
}The else block here is the troublesome part.
In my particular case I am using the Arrow project which uses CancellationException to short-circuit error handling in some parts of the API so it's an easy way to reproduce this issue, but I'm guessing any usage of coroutines could potentially run into this. In short, If I have some code in a writeTransaction block that utilizes this short-circuit mechanism you will get the following error message.
Exception in thread "AWT-EventQueue-0" com.powersync.PowerSyncException: kotlin.coroutines.cancellation.CancellationException should never get swallowed. Always re-throw it if captured.This swallows the exception of Arrow's Raise, and leads to unexpected behavior.When working with Arrow prefer Either.catch or arrow.core.raise.catch to automatically rethrow CancellationException.
The message is from Arrow but does describe the issue well and I do not believe it is actually an Arrow issue, but a Powersync one.
In a project with Arrow and Powersync the following function will reproduce the issue every time:
suspend fun test(database: PowerSyncDatabase) : Either<String, Unit> = either {
database.writeTransaction { _ ->
Either.Left("").bind()
}
}I should add that Either.Left() here just replaces a function call that does some work and itself returns an Either instance, which .bind() checks. Replacing that call here with the error return case is easiest.