Deprioritize cancellation in mergeSuppressed

arrow-libs/core/arrow-autoclose/src/commonTest/kotlin/arrow/AutoCloseTest.kt

@@ -1,9 +1,12 @@
 package arrow
 
 import arrow.atomic.AtomicBoolean
+import arrow.core.ControlCancellationException
+import arrow.core.InternalArrowApi
 import io.kotest.assertions.AssertionErrorBuilder
+import io.kotest.assertions.assertionCounter
 import io.kotest.common.reflection.bestName
-import io.kotest.matchers.assertionCounter
+import io.kotest.matchers.collections.shouldHaveSingleElement
 import io.kotest.matchers.shouldBe
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.channels.Channel
@@ -12,6 +15,7 @@ import kotlinx.coroutines.test.runTest
 import kotlin.coroutines.cancellation.CancellationException
 import kotlin.test.Test
 
+@OptIn(InternalArrowApi::class)
 class AutoCloseTest {
 
   @Test
@@ -72,7 +76,7 @@ class AutoCloseTest {
           r.shutdown()
           throw error2
         }
-        autoClose({ Resource() }) { _, _ -> throw error3 }
+        val _ = autoClose({ Resource() }) { _, _ -> throw error3 }
         require(wasActive.complete(r.isActive()))
         throw error
       }
@@ -93,7 +97,7 @@ class AutoCloseTest {
 
     val e = shouldThrow<RuntimeException> {
       autoCloseScope {
-        autoClose({ Resource() }) { r, e ->
+        val _ = autoClose({ Resource() }) { r, e ->
           require(promise.complete(e))
           r.shutdown()
           throw error2
@@ -163,7 +167,7 @@ class AutoCloseTest {
     val wasActive = Channel<Boolean>(Channel.UNLIMITED)
     val closed = Channel<Resource>(Channel.UNLIMITED)
 
-    autoCloseScope {
+    val _ = autoCloseScope {
       val r1 = autoClose({ res1 }) { r, _ ->
         closed.trySend(r).getOrThrow()
         r.shutdown()
@@ -190,36 +194,58 @@ class AutoCloseTest {
     closed.cancel()
   }
 
+  @Test
+  fun normalRaise() = shouldAutoCloseWithSecond({}) { throw ControlCancellationException("second") }
 
-  @OptIn(ExperimentalStdlibApi::class) // 'AutoCloseable' in stdlib < 2.0
-  private class Resource : AutoCloseable {
-    private val isActive = AtomicBoolean(true)
+  @Test
+  fun returnRaise() = shouldAutoCloseWithSecond({ return }) { throw ControlCancellationException("second") }
 
-    fun isActive(): Boolean = isActive.get()
+  @Test
+  fun raiseRaise() = shouldAutoCloseWithSecond({ throw ControlCancellationException("first") }) { throw ControlCancellationException("second") }
 
-    fun shutdown() {
-      require(isActive.compareAndSet(expected = true, new = false)) {
-        "Already shut down"
-      }
-    }
+  @Test
+  fun cancelRaise() = shouldAutoCloseWithFirst({ throw CancellationException("first") }) { throw ControlCancellationException("second") }
 
-    override fun close() {
-      shutdown()
-    }
-  }
+  @Test
+  fun throwRaise() = shouldAutoCloseWithFirst({ throw RuntimeException("first") }) { throw ControlCancellationException("second") }
 
-  private suspend fun <T> CompletableDeferred<T>.shouldHaveCompleted(): T {
-    isCompleted shouldBe true
-    return await()
-  }
+  @Test
+  fun normalCancel() = shouldAutoCloseWithSecond({}) { throw CancellationException("second") }
+
+  @Test
+  fun returnCancel() = shouldAutoCloseWithSecond({ return }) { throw CancellationException("second") }
+
+  @Test
+  fun raiseCancel() = shouldAutoCloseWithSecond({ throw ControlCancellationException("first") }) { throw CancellationException("second") }
+
+  @Test
+  fun cancelCancel() = shouldAutoCloseWithFirst({ throw CancellationException("first") }) { throw CancellationException("second") }
+
+  @Test
+  fun throwCancel() = shouldAutoCloseWithFirst({ throw RuntimeException("first") }) { throw CancellationException("second") }
+
+  @Test
+  fun normalThrow() = shouldAutoCloseWithSecond({}) { throw RuntimeException("second") }
+
+  @Test
+  fun returnThrow() = shouldAutoCloseWithSecond({ return }) { throw RuntimeException("second") }
+
+  @Test
+  fun raiseThrow() = shouldAutoCloseWithSecond({ throw ControlCancellationException("first") }) { throw RuntimeException("second") }
+
+  @Test
+  fun cancelThrow() = shouldAutoCloseWithFirst({ throw CancellationException("first") }) { throw RuntimeException("second") }
+
+  @Test
+  fun throwThrow() = shouldAutoCloseWithFirst({ throw RuntimeException("first") }) { throw RuntimeException("second") }
 }
 
 // copied from Kotest so we can inline it
 inline fun <reified T : Throwable> shouldThrow(block: () -> Any?): T {
   assertionCounter.inc()
   val expectedExceptionClass = T::class
   val thrownThrowable = try {
-    block()
+    val _ = block()
     null  // Can't throw failure here directly, as it would be caught by the catch clause, and it's an AssertionError, which is a special case
   } catch (thrown: Throwable) {
     thrown
@@ -237,3 +263,71 @@ inline fun <reified T : Throwable> shouldThrow(block: () -> Any?): T {
       .build()
   }
 }
+
+private class Resource : AutoCloseable {
+  private val isActive = AtomicBoolean(true)
+
+  fun isActive(): Boolean = isActive.get()
+
+  fun shutdown() {
+    require(isActive.compareAndSet(expected = true, new = false)) {
+      "Already shut down"
+    }
+  }
+
+  override fun close() {
+    shutdown()
+  }
+}
+
+private suspend fun <T> CompletableDeferred<T>.shouldHaveCompleted(): T {
+  isCompleted shouldBe true
+  return await()
+}
+
+private inline fun shouldAutoCloseWithFirst(first: () -> Unit, crossinline second: () -> Nothing) {
+  var firstThrowable: Throwable? = null
+  lateinit var secondThrowable: Throwable
+  try {
+    autoCloseScope {
+      onClose {
+        it shouldBe firstThrowable
+        peekThrowable(second) { secondThrowable = it }
+      }
+      peekThrowable(first) { firstThrowable = it }
+    }
+  } catch (e: Throwable) {
+    e shouldBe firstThrowable
+    e.suppressedExceptions shouldHaveSingleElement secondThrowable
+  } finally {
+    val _ = secondThrowable // ensure that onClose ran
+  }
+}
+
+private inline fun shouldAutoCloseWithSecond(first: () -> Unit, crossinline second: () -> Nothing) {
+  var firstThrowable: Throwable? = null
+  lateinit var secondThrowable: Throwable
+  var finishedWithThrowable = false
+  try {
+    autoCloseScope {
+      onClose {
+        it shouldBe firstThrowable
+        peekThrowable(second) { secondThrowable = it }
+      }
+      peekThrowable(first) { firstThrowable = it }
+    }
+  } catch (e: Throwable) {
+    e shouldBe secondThrowable
+    if (firstThrowable != null) e.suppressedExceptions shouldHaveSingleElement firstThrowable
+    finishedWithThrowable = true
+  } finally {
+    finishedWithThrowable shouldBe true // otherwise, we finished with first somehow, either non-locally, or with Unit
+  }
+}
+
+private inline fun <R> peekThrowable(block: () -> R, peek: (Throwable) -> Unit): R = try {
+  block()
+} catch (e: Throwable) {
+  peek(e)
+  throw e
+}

arrow-libs/core/arrow-core/api/arrow-core.klib.api

@@ -699,7 +699,7 @@ sealed class <#A: out kotlin/Any?> arrow.core/Option { // arrow.core/Option|null
     }
 }
 
-sealed class arrow.core.raise/RaiseCancellationException : kotlin.coroutines.cancellation/CancellationException // arrow.core.raise/RaiseCancellationException|null[0]
+sealed class arrow.core.raise/RaiseCancellationException : arrow.core/ControlCancellationException // arrow.core.raise/RaiseCancellationException|null[0]
 
 final object arrow.core/ArrowCoreInternalException : kotlin/RuntimeException // arrow.core/ArrowCoreInternalException|null[0]
 

arrow-libs/core/arrow-core/api/jvm/arrow-core.api

@@ -1024,7 +1024,7 @@ public abstract class arrow/core/raise/RaiseAccumulate$Value {
 	public final fun getValue (Ljava/lang/Void;Lkotlin/reflect/KProperty;)Ljava/lang/Object;
 }
 
-public abstract class arrow/core/raise/RaiseCancellationException : java/util/concurrent/CancellationException {
+public abstract class arrow/core/raise/RaiseCancellationException : arrow/core/ControlCancellationException {
 	public synthetic fun <init> (Ljava/lang/Object;Larrow/core/raise/Raise;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
 }
 

arrow-libs/core/arrow-core/src/androidAndJvmMain/kotlin/arrow/core/raise/RaiseCancellationException.kt

@@ -1,9 +1,11 @@
 package arrow.core.raise
 
-import kotlin.coroutines.cancellation.CancellationException
+import arrow.core.ControlCancellationException
+import arrow.core.InternalArrowApi
 
+@OptIn(InternalArrowApi::class)
 @DelicateRaiseApi
 public actual sealed class RaiseCancellationException actual constructor(
   internal actual val raised: Any?,
   internal actual val raise: Raise<Any?>
-) : CancellationException(RaiseCancellationExceptionCaptured)
+) : ControlCancellationException(RaiseCancellationExceptionCaptured)

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/Fold.kt

@@ -6,7 +6,9 @@
 package arrow.core.raise
 
 import arrow.atomic.AtomicBoolean
+import arrow.core.ControlCancellationException
 import arrow.core.Either
+import arrow.core.InternalArrowApi
 import arrow.core.nonFatalOrThrow
 import kotlin.contracts.ExperimentalContracts
 import kotlin.contracts.InvocationKind.AT_MOST_ONCE
@@ -271,11 +273,12 @@ public annotation class DelicateRaiseApi
  * [RaiseCancellationException] is a _delicate_ api, and should be used with care.
  * It drives the short-circuiting behavior of [Raise].
  */
+@OptIn(InternalArrowApi::class)
 @DelicateRaiseApi
 public expect sealed class RaiseCancellationException(
   raised: Any?,
   raise: Raise<Any?>
-) : CancellationException {
+) : ControlCancellationException {
   internal val raised: Any?
   internal val raise: Raise<Any?>
 }

arrow-libs/core/arrow-core/src/jsMain/kotlin/arrow/core/raise/RaiseCancellationException.kt

@@ -1,7 +1,8 @@
 package arrow.core.raise
 
+import arrow.core.ControlCancellationException
+import arrow.core.InternalArrowApi
 import kotlinx.js.JsPlainObject
-import kotlin.coroutines.cancellation.CancellationException
 
 /**
  * There is no direct way to create an instance of [Error](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error) without a stack in JS.
@@ -15,11 +16,12 @@ internal external interface RaiseCancellationExceptionLike {
   val raise: Raise<Any?>
 }
 
+@OptIn(InternalArrowApi::class)
 @DelicateRaiseApi
 public actual sealed class RaiseCancellationException actual constructor(
   raised: Any?,
   raise: Raise<Any?>
-) : CancellationException(RaiseCancellationExceptionCaptured) {
+) : ControlCancellationException(RaiseCancellationExceptionCaptured) {
   private val _raised = raised
   private val _raise = raise
 

arrow-libs/core/arrow-core/src/nonJvmAndJsMain/kotlin/arrow/core/raise/RaiseCancellationException.kt

@@ -1,9 +1,11 @@
 package arrow.core.raise
 
-import kotlin.coroutines.cancellation.CancellationException
+import arrow.core.ControlCancellationException
+import arrow.core.InternalArrowApi
 
+@OptIn(InternalArrowApi::class)
 @DelicateRaiseApi
 public actual sealed class RaiseCancellationException actual constructor(
   internal actual val raised: Any?,
   internal actual val raise: Raise<Any?>
-) : CancellationException(RaiseCancellationExceptionCaptured)
+) : ControlCancellationException(RaiseCancellationExceptionCaptured)

arrow-libs/core/arrow-exception-utils/api/arrow-exception-utils.klib.api

@@ -6,6 +6,15 @@
 // - Show declarations: true
 
 // Library unique name: <io.arrow-kt:arrow-exception-utils>
+open annotation class arrow.core/InternalArrowApi : kotlin/Annotation { // arrow.core/InternalArrowApi|null[0]
+    constructor <init>() // arrow.core/InternalArrowApi.<init>|<init>(){}[0]
+}
+
+open class arrow.core/ControlCancellationException : kotlin.coroutines.cancellation/CancellationException { // arrow.core/ControlCancellationException|null[0]
+    constructor <init>() // arrow.core/ControlCancellationException.<init>|<init>(){}[0]
+    constructor <init>(kotlin/String?) // arrow.core/ControlCancellationException.<init>|<init>(kotlin.String?){}[0]
+}
+
 final fun (kotlin/Throwable).arrow.core/nonFatalOrThrow(): kotlin/Throwable // arrow.core/nonFatalOrThrow|nonFatalOrThrow@kotlin.Throwable(){}[0]
 final fun (kotlin/Throwable?).arrow.core/mergeSuppressed(kotlin/Throwable?): kotlin/Throwable? // arrow.core/mergeSuppressed|mergeSuppressed@kotlin.Throwable?(kotlin.Throwable?){}[0]
 final fun (kotlin/Throwable?).arrow.core/throwIfNotNull() // arrow.core/throwIfNotNull|throwIfNotNull@kotlin.Throwable?(){}[0]

arrow-libs/core/arrow-exception-utils/api/jvm/arrow-exception-utils.api

@@ -1,3 +1,11 @@
+public class arrow/core/ControlCancellationException : java/util/concurrent/CancellationException {
+	public fun <init> ()V
+	public fun <init> (Ljava/lang/String;)V
+}
+
+public abstract interface annotation class arrow/core/InternalArrowApi : java/lang/annotation/Annotation {
+}
+
 public final class arrow/core/NonFatalKt {
 	public static final fun NonFatal (Ljava/lang/Throwable;)Z
 }

arrow-libs/core/arrow-exception-utils/src/commonMain/kotlin/arrow/core/ControlCancellationException.kt

@@ -0,0 +1,21 @@
+package arrow.core
+
+import kotlin.coroutines.cancellation.CancellationException
+
+@MustBeDocumented
+@Retention(AnnotationRetention.BINARY)
+@RequiresOptIn("This declaration is public only to allow other arrow libraries to use it", RequiresOptIn.Level.ERROR)
+public annotation class InternalArrowApi
+
+/**
+ * [ControlCancellationException] is a _delicate_ api, and should be used with care.
+ * It denotes a short-circuiting exception.
+ * Exceptions of this type are deprioritized w.r.t. exception suppression.
+ *
+ * @see mergeSuppressed
+ */
+@InternalArrowApi
+public open class ControlCancellationException : CancellationException {
+  public constructor() : super()
+  public constructor(message: String?) : super(message)
+}

arrow-libs/core/arrow-exception-utils/src/commonMain/kotlin/arrow/core/ThrowableUtils.kt

@@ -6,15 +6,30 @@ import kotlin.coroutines.cancellation.CancellationException
 
 public fun Throwable?.throwIfNotNull() { if (this != null) throw this }
 
-@OptIn(ExperimentalContracts::class)
+/**
+ * Merges two nullable [Throwable] values by adding one as suppressed to the other.
+ *
+ * Returns the non-null throwable when only one is present, or `null` when both are `null`.
+ * [ControlCancellationException]s are deprioritized in the presence of other exceptions.
+ *
+ * @param other Another throwable to merge with this one.
+ * @return The merged throwable, or `null` if both are `null`.
+ */
+@OptIn(ExperimentalContracts::class, InternalArrowApi::class)
 public infix fun Throwable?.mergeSuppressed(other: Throwable?): Throwable? {
   contract {
     returns(null) implies (this@mergeSuppressed == null && other == null)
   }
   return when {
+    // other completed normally
     other == null -> this
+    // this completed normally or with a non-local return
     this == null -> other
+    // this completed with raise
+    this is ControlCancellationException -> other.also { other.addSuppressed(this) }
+    // other completed with cancellation or raise
     other is CancellationException -> this.also { addSuppressed(other) }
+    // both completed exceptionally or this completed with cancellation
     else -> this.also { addSuppressed(other.nonFatalOrThrow()) }
   }
 }