Flow's onCompletion not being called

[coroutineDispatcher]

Hi,

As the guideline in https://codelabs.developers.google.com/codelabs/advanced-kotlin-coroutines/index.html?index=..%2F..index#13 suggests, the codelab is finished:

growZoneChannel.asFlow()
            .mapLatest { growZone ->
                _spinner.value = true
                if (growZone == NoGrowZone) {
                    plantRepository.tryUpdateRecentPlantsCache()
                } else {
                    plantRepository.tryUpdateRecentPlantsForGrowZoneCache(growZone)
                }
            }
            .onCompletion {
                _spinner.value = false
            }
            .catch { throwable ->
                _snackbar.value = throwable.message
            }
            .launchIn(viewModelScope) 

Even if you do the latest challenge to refactor all this, we would end up in this case:

init {
        // When creating a new ViewModel, clear the grow zone and perform any related udpates
        clearGrowZoneNumber()

        loadDataFor(growZoneChannel) { growZone ->
            _spinner.value = true
            if (growZone == NoGrowZone) {
                plantRepository.tryUpdateRecentPlantsCache()
            } else {
                plantRepository.tryUpdateRecentPlantsForGrowZoneCache(growZone)
            }
        }
    }

//where the loadDataFor is just:

private fun <T> loadDataFor(source: ConflatedBroadcastChannel<T>, block: suspend (T) -> Unit) {
        source.asFlow()
            .mapLatest {
                block(it)
            }
            .onCompletion {
                _spinner.value = false
            }
            .catch { throwable ->
                _snackbar.value = throwable.message
            }
            .launchIn(viewModelScope)
    }

However, the onCompletion method is not being called, and causing the _spinner value not to be updated at the end. I don't know what's wrong but I did try my solution and did try finished_code directory also.

[coroutineDispatcher]

Ps: I did try for error checking but still a throwable isn't occurring at all.

[fegan104]

You're correct. I asked about this issue on Stackoverflow . Looks like the codelab is not correct when it states:

"onCompletion will be called every time the flow above it completes. It's the same thing as a finally block – it's a good place to put any code you need to execute during cleanup. Here we're resetting the spinner."

[coroutineDispatcher]

Have you resolved the issue? If so, please share.

[fegan104]

You can "solve" it by just putting the spinner calls before and after the block so they run every time.

source.asFlow()
   .mapLatest {
     _spinner.value = true
     block(it)
     _spinner.value = false
   }
   .catch { throwable ->   _snackbar.value = throwable.message}
   .launchIn(viewModelScope)

[khpproud]

I tried like this:

fun <T> loadDataFor(source: ConflatedBroadcastChannel<T>, block: suspend (T) -> Unit) {
        source.asFlow()
            .mapLatest {
                _spinner.value = true
                block(it)
            }
            .onEach { _spinner.value = false }
            .onCompletion { _spinner.value = false }
            .catch { _snackbar.value = it.message }
            .launchIn(viewModelScope)
}

The channel is not closed calling explicitly "close()" on viewModel's lifecycle, so it seems more appropriate to add "onEach".

[shiehnpin]

I think you can do it like this: fun loadDataFor(source: ConflatedBroadcastChannel, block: suspend (T) -> Unit) { source.asFlow() .onEach { _spinner.value = true } .mapLatest { block(it) } .onEach { _spinner.value = false } .onCompletion { _spinner.value = false } .catch { throwable -> _snackbar.value = throwable.message } .launchIn(viewModelScope) }

[funct7]

You're correct. I asked about this issue on Stackoverflow . Looks like the codelab is not correct when it states:

"onCompletion will be called every time the flow above it completes. It's the same thing as a finally block – it's a good place to put any code you need to execute during cleanup. Here we're resetting the spinner."

It IS the same thing as a finally block. The only problem is that the channel is not closing, so the flow is not completing. Kind of having an infinite loop running so the finally block is never reached.

There's no real way to keep the tutorial as it is, setting _spinner.value = false in onCompletion, since onCompletion will only be executed only once in the lifecycle of the channel when it completes, either by an exception or by closing the channel.

[funct7]

You can use onCompletion to set _spinner = false like so:

growZoneChannel.asFlow()
    .onEach {
        flow<Nothing> {
            _spinner.value = true
            if (it == NoGrowZone) plantRepository.tryUpdateRecentPlantsCache()
            else plantRepository.tryUpdateRecentPlantsForGrowZoneCache(it)
        }
            .onCompletion { _spinner.value = false }
            .catch { _snackbar.value = it.localizedMessage }
            .collect()
    }
    .launchIn(viewModelScope)

Hiding activity indicators using onCompletion is really not a good fit for a flow since a flow expects an asynchronous sequence to be emitted over time, and onCompletion is called only when the flow is completed.

[KoTius]

Flow never calls .onCompletion() because we get flow from ConflatedBroadcastChannel and observing it infinitely as intended. In "finished_code" the same error, but there are not commented code which loads data for livedatas. clearGrowZoneNumber() and setGrowZoneNumber() which are actually hiding spinner so we can't see the bug in the app.

So the answer would be something like:

    fun <T> loadDataFor(source: ConflatedBroadcastChannel<T>, block: suspend (T) -> Unit) {
        source.asFlow()
            .mapLatest {
                _spinner.value = true
                block(it)
                _spinner.value = false
            }
            .onCompletion {
                // In the codelab the flow is infinite since it comes from ConflatedBroadcastChannel.
                // So onCompletion will be called in case Flow finished with error or manual cancellation
                // If the flow is not infinite then _spinner.value = false will be called twice: 
                    -> First in time in onEach, second time in onCompletion
                _spinner.value = false
            }
            .catch { throwable ->  _snackbar.value = throwable.message  }
            .launchIn(viewModelScope)
    }

or

    fun <T> loadDataFor(source: ConflatedBroadcastChannel<T>, block: suspend (T) -> Unit) {
        source.asFlow()
            .onStart {
                _spinner.value = true
            }
            .mapLatest {
                block(it)
            }
            .onEach {
                _spinner.value = false
            }
            .onCompletion {
                // In the codelab the flow is infinite since it comes from ConflatedBroadcastChannel.
                // So onCompletion will be called in case Flow finished with error or manual cancellation
                // If the flow is not infinite then _spinner.value = false will be called twice: 
                    -> First in time in onEach, second time in onCompletion
                _spinner.value = false
            }
            .catch { throwable ->  _snackbar.value = throwable.message  }
            .launchIn(viewModelScope)
    }