Skip to content

Commit 9227f94

Browse files
committed
Change the exception suppression invariant to work backwards:
All the benefits of the approach stay the same, but additionally, for an arbitrary flow pipeline, adding a catch operator that is not triggered will no longer change the type of resulting exception
1 parent 08a3ed3 commit 9227f94

File tree

5 files changed

+61
-10
lines changed

5 files changed

+61
-10
lines changed

kotlinx-coroutines-core/common/src/flow/Flow.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ import kotlin.coroutines.*
133133
*
134134
* When `emit` or `emitAll` throws, the Flow implementations must immediately stop emitting new values and finish with an exception.
135135
* For diagnostics or application-specific purposes, the exception may be different from the one thrown by the emit operation,
136-
* but then it will be suppressed, as discussed below.
136+
* suppressing the original exception as discussed below.
137137
* If there is a need to emit values after the downstream failed, please use the [catch][Flow.catch] operator.
138138
*
139139
* The [catch][Flow.catch] operator only catches upstream exceptions, but passes
@@ -152,7 +152,9 @@ import kotlin.coroutines.*
152152
* All exception-handling Flow operators follow the principle of exception suppression:
153153
*
154154
* If the upstream flow throws an exception during its completion when the downstream exception has been thrown,
155-
* the upstream exception becomes superseded and suppressed by the downstream exception.
155+
* the downstream exception becomes superseded and suppressed by the upstream exception, being a semantic
156+
* equivalent of throwing from `finally` block. Exception-handling operators then ignore this exception,
157+
* still following the downstream failure.
156158
*
157159
* Failure to adhere to the exception transparency requirement can lead to strange behaviors which make
158160
* it hard to reason about the code because an exception in the `collect { ... }` could be somehow "caught"

kotlinx-coroutines-core/common/src/flow/operators/Errors.kt

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,7 @@ internal suspend fun <T> Flow<T>.catchImpl(
214214
* The exception came from the upstream [semi-] independently.
215215
* For pure failures, when the downstream functions normally, we handle the exception as intended.
216216
* But if the downstream has failed prior to or concurrently
217-
* with the upstream, we forcefully rethrow it, preserving the contextual information and ensuring
218-
* that it's not lost.
217+
* with the upstream, we forcefully rethrow it, preserving the contextual information and ensuring that it's not lost.
219218
*/
220219
if (fromDownstream == null) {
221220
return e
@@ -238,12 +237,12 @@ internal suspend fun <T> Flow<T>.catchImpl(
238237
* ```
239238
* when *the downstream* throws.
240239
*/
241-
if (fromDownstream is CancellationException) {
242-
e.addSuppressed(fromDownstream)
243-
throw e
244-
} else {
240+
if (e is CancellationException) {
245241
fromDownstream.addSuppressed(e)
246242
throw fromDownstream
243+
} else {
244+
e.addSuppressed(fromDownstream)
245+
throw e
247246
}
248247
}
249248
}

kotlinx-coroutines-core/common/test/flow/operators/CatchTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ class CatchTest : TestBase() {
160160
throw TestException2()
161161
}
162162

163-
assertFailsWith<TestException2>(flow)
163+
assertFailsWith<TestException>(flow)
164164
finish(4)
165165
}
166166

kotlinx-coroutines-core/common/test/flow/operators/RetryTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class RetryTest : TestBase() {
120120
throw TestException2()
121121
}
122122

123-
assertFailsWith<TestException2>(flow)
123+
assertFailsWith<TestException>(flow)
124124
finish(4)
125125
}
126126

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package kotlinx.coroutines.exceptions
6+
7+
import kotlinx.coroutines.*
8+
import kotlinx.coroutines.flow.*
9+
import org.junit.*
10+
import org.junit.Test
11+
import kotlin.test.*
12+
13+
class FlowSuppressionTest : TestBase() {
14+
@Test
15+
fun testSuppressionForPrimaryException() = runTest {
16+
val flow = flow {
17+
try {
18+
emit(1)
19+
} finally {
20+
throw TestException()
21+
}
22+
}.catch { expectUnreached() }.onEach { throw TestException2() }
23+
24+
try {
25+
flow.collect()
26+
} catch (e: Throwable) {
27+
assertIs<TestException>(e)
28+
assertIs<TestException2>(e.suppressed[0])
29+
}
30+
}
31+
32+
@Test
33+
fun testSuppressionForPrimaryExceptionRetry() = runTest {
34+
val flow = flow {
35+
try {
36+
emit(1)
37+
} finally {
38+
throw TestException()
39+
}
40+
}.retry { expectUnreached(); true }.onEach { throw TestException2() }
41+
42+
try {
43+
flow.collect()
44+
} catch (e: Throwable) {
45+
assertIs<TestException>(e)
46+
assertIs<TestException2>(e.suppressed[0])
47+
48+
}
49+
}
50+
}

0 commit comments

Comments
 (0)