Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flow.combine hangs when the flow in the argument throws with TestCoroutineDispatcher #2405

Closed
kokeroulis opened this issue Nov 23, 2020 · 1 comment
Assignees

Comments

@kokeroulis
Copy link

@kokeroulis kokeroulis commented Nov 23, 2020

Hello,

During writing some unit tests for our project we found out the following behaviour which is a bit odd..
If you convert a Rx.Single to a Rx.Observable to a Flow which is used with Flow.combine then
the error is not being emitted if you are using a TestCoroutineDispatcher and the Flow which was produced
from RxJava is the second on the stream.

If the dispatcher is the TestCoroutineDispatcher then the stream just hangs. (onError, catch and collect doesn't do anything).

We found out that with some small changes the following code will work fine:

  • Move the Flow which got produced from RxJava, 1st in the chain
  • Replace the dispatcher with Dispatchers.Unconfined
  • Flow.zip works as expected.

Full example bellow

class RxJavaSingleIssue {

    // it works fine with this dispatcher
     private val dispatcher = Dispatchers.Unconfined

    // The stream get's stuck. It never emits the error value
    //private val dispatcher = TestCoroutineDispatcher()

    @Test
    fun `test something when repository throws`() {
        val usecase = SomeUseCase()

        var hasError = false
        var didItCatch = false
        var result: Pair<Int, String>? = null
        GlobalScope.launch(dispatcher) {
            usecase.getSomeData()
                .onError {
                    hasError = true
                }
                .catch {
                    didItCatch = true
                    emit(Pair(0, ""))
                }
                .collect {
                    result = it
                    println("a $it")
                }
        }

        Thread.sleep(3000)
        assert(hasError)
        assert(didItCatch)
        assert(result != null)
    }
}

class SomeUseCase(
    private val someRxRepository: SomeRxRepository = SomeRxRepository(),
    private val someFlowRepository: SomeFlowRepository = SomeFlowRepository()
) {

    fun getSomeData(): Flow<Pair<Int, String>> {
        // If we revert the order, then it always works fine
        return someFlowRepository.getFlowObject()
            .combine(
                someRxRepository.getSomething().toObservable().asFlow()
                           .onError { println("I will print that there is an error") }
            ) { int, string -> Pair(int, string) }
    }
}

class SomeRxRepository {

    fun getSomething(): Single<String> {
        return Single.error(IOException())
    }
}

class SomeFlowRepository {

    fun getFlowObject(): Flow<Int> {
        return flow {
            emit(1)
            emit(2)
        }
    }
}
@qwwdfsad qwwdfsad added the bug label Nov 24, 2020
@qwwdfsad qwwdfsad added the test label Jan 25, 2021
@dkhalanskyjb
Copy link
Member

@dkhalanskyjb dkhalanskyjb commented May 18, 2021

Simplifying the reproducer, I get the following:

    @Test
    fun reproducer2405() = runBlocking {
        val dispatcher = TestCoroutineDispatcher()
        var collectedError = false
        withContext(dispatcher) {
            flow { emit(1) }
                .combine(
                    flow<String> { throw IOException() }
                ) { int, string -> int.toString() + string }
                .catch { emit("error") }
                .collect {
                    assertEquals("error", it)
                    collectedError = true
                }
        }
        assertTrue(collectedError)
    }

Looks like this isn't related to RxSingle at all: any throwing flow in the argument to combine causes this to fail with TestCoroutineDispatcher. A similar issue was reported here: #1742

@dkhalanskyjb dkhalanskyjb changed the title Flow.Combine doesn't emit an error value when RxSingle is throwing Flow.combine hangs when the flow in the argument throws with TestCoroutineDispatcher May 18, 2021
@dkhalanskyjb dkhalanskyjb self-assigned this Oct 14, 2021
dkhalanskyjb added a commit that referenced this issue Nov 1, 2021
Defines two test dispatchers:
* StandardTestDispatcher, which, combined with runTest,
  gives an illusion of an event loop;
* UnconfinedTestDispatcher, which is like
  Dispatchers.Unconfined, but skips delays.

By default, StandardTestDispatcher is used due to the somewhat
chaotic execution order of Dispatchers.Unconfined.
TestCoroutineDispatcher is deprecated.

Fixes #1626
Fixes #1742
Fixes #2082
Fixes #2102
Fixes #2405
Fixes #2462
dkhalanskyjb added a commit that referenced this issue Nov 17, 2021
Defines two test dispatchers:
* StandardTestDispatcher, which, combined with runTest,
  gives an illusion of an event loop;
* UnconfinedTestDispatcher, which is like
  Dispatchers.Unconfined, but skips delays.

By default, StandardTestDispatcher is used due to the somewhat
chaotic execution order of Dispatchers.Unconfined.
TestCoroutineDispatcher is deprecated.

Fixes #1626
Fixes #1742
Fixes #2082
Fixes #2102
Fixes #2405
Fixes #2462
dkhalanskyjb added a commit that referenced this issue Nov 17, 2021
Defines two test dispatchers:
* StandardTestDispatcher, which, combined with runTest,
  gives an illusion of an event loop;
* UnconfinedTestDispatcher, which is like
  Dispatchers.Unconfined, but skips delays.

By default, StandardTestDispatcher is used due to the somewhat
chaotic execution order of Dispatchers.Unconfined.
TestCoroutineDispatcher is deprecated.

Fixes #1626
Fixes #1742
Fixes #2082
Fixes #2102
Fixes #2405
Fixes #2462
dkhalanskyjb added a commit that referenced this issue Nov 19, 2021
Defines two test dispatchers:
* StandardTestDispatcher, which, combined with runTest,
  gives an illusion of an event loop;
* UnconfinedTestDispatcher, which is like
  Dispatchers.Unconfined, but skips delays.

By default, StandardTestDispatcher is used due to the somewhat
chaotic execution order of Dispatchers.Unconfined.
TestCoroutineDispatcher is deprecated.

Fixes #1626
Fixes #1742
Fixes #2082
Fixes #2102
Fixes #2405
Fixes #2462
yorickhenning pushed a commit to yorickhenning/kotlinx.coroutines that referenced this issue Jan 28, 2022
This commit introduces the new version of the test module.
Please see README.md and MIGRATION.md for a thorough
discussion of the changes.

Fixes Kotlin#1203
Fixes Kotlin#1609
Fixes Kotlin#2379
Fixes Kotlin#1749
Fixes Kotlin#1204
Fixes Kotlin#1390
Fixes Kotlin#1222
Fixes Kotlin#1395
Fixes Kotlin#1881
Fixes Kotlin#1910
Fixes Kotlin#1772
Fixes Kotlin#1626
Fixes Kotlin#1742
Fixes Kotlin#2082
Fixes Kotlin#2102
Fixes Kotlin#2405
Fixes Kotlin#2462

Co-authored-by: Vsevolod Tolstopyatov <qwwdfsad@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants