sksamuel / aedile

Kotlin Wrapper for Caffeine
Apache License 2.0
164 stars 15 forks source link

Cache.get() cancels coroutine when exception thrown #30

Open cloudshiftchris opened 1 month ago

cloudshiftchris commented 1 month ago

Expecting cache.get(key) { ... load value here ... } to propagate exceptions to the caller, as documented:

If the suspendable computation throws, the exception will be propagated to the caller.

What happens instead is the coroutine is cancelled and a JobCancellationException is thrown.

try {
    val cache = cacheBuilder<String, String>().build()

    cache.get("key") { throw IllegalArgumentException("Whoops") }
} catch (t: Throwable) {
    // this is a JobCancellationException; expected to be the thrown IllegalArgumentException
    println(t)
}

Exception: kotlinx.coroutines.JobCancellationException: TimeoutCoroutine is cancelling; job=TimeoutCoroutine(timeMillis=45000){Cancelling}@609ac453 (this is inside a kotest test)

cloudshiftchris commented 1 month ago

Looking through the tests - they aren't up-to-date wrt cacheBuilder (most use caffeineBuilder) - the test for exception propagation works for caffeineBuilder but fails when changed to cacheBuilder

                // from unit test: https://github.com/sksamuel/aedile/blob/8d3ea128b474062dae9ad6b925f3f0c5d748a75d/aedile-core/src/test/kotlin/com/sksamuel/aedile/core/CacheTest.kt#L42
                // exception propagation works
                val cache = caffeineBuilder<String, String>().build()
                shouldThrow<IllegalStateException> {
                    cache.get("foo") {
                        error("kapow")
                    }
                }

                // using cacheBuilder instead
                // exception propagation fails - coroutine is cancelled
                val cache2 = cacheBuilder<String, String>().build()
                shouldThrow<IllegalStateException> {
                    cache2.get("foo") {
                        error("kapow")
                    }
                }