1

I have a Repository defined as the following.

class StoryRepository {
    private val firestore = Firebase.firestore

    suspend fun fetchStories(): QuerySnapshot? {
        return try {
            firestore
                .collection("stories")
                .get()
                .await()
        } catch(e: Exception) {
            Log.e("StoryRepository", "Error in fetching Firestore stories: $e")
            null
        }
    }
}

I also have a ViewModel like this.

class HomeViewModel(
    application: Application
) : AndroidViewModel(application) {
    private var viewModelJob = Job()
    private val uiScope = CoroutineScope(Dispatchers.Main + viewModelJob)
    private val storyRepository = StoryRepository()

    private var _stories = MutableLiveData<List<Story>>()
    val stories: LiveData<List<Story>>
        get() = _stories

    init {
        uiScope.launch {
            getStories()
        }
        uiScope.launch {
            getMetadata()
        }            
    }

    private suspend fun getStories() {
        withContext(Dispatchers.IO) {
            val snapshots = storyRepository.fetchStories()
            // Is this correct?
            if (snapshots == null) {
                cancel(CancellationException("Task is null; local DB not refreshed"))
                return@withContext
            }
            val networkStories = snapshots.toObjects(NetworkStory::class.java)
            val stories = NetworkStoryContainer(networkStories).asDomainModel()
            _stories.postValue(stories)
        }
    }

    suspend fun getMetadata() {
        // Does some other fetching
    }

    override fun onCleared() {
        super.onCleared()
        viewModelJob.cancel()
    }
}

As you can see, sometimes, StoryRepository().fetchStories() may fail and return null. If the return value is null, I would like to not continue what follows after the checking for snapshots being null block. Therefore, I would like to cancel that particular coroutine (the one that runs getStories() without cancelling the other coroutine (the one that runs getMetadata()). How do I achieve this and is return-ing from withContext a bad-practice?

Richard
  • 7,037
  • 2
  • 23
  • 76
  • Seems correct as of https://discuss.kotlinlang.org/t/idiomatic-way-to-cancel-coroutine-from-within-a-suspend-function/5268 – Animesh Sahu May 29 '20 at 11:44
  • @AnimeshSahu Is it okay to `return` from `withContext` and should I provide a mechanism to catch the `CancellationException`? – Richard May 29 '20 at 13:24

1 Answers1

2

Although your approach is right, you can always make some improvements to make it simpler or more idiomatic (especially when you're not pleased with your own code).

These are just some suggestions that you may want to take into account:

You can make use of Kotlin Scope Functions, or more specifically the let function like this:

private suspend fun getStories() = withContext(Dispatchers.IO) {
    storyRepository.fetchStories()?.let { snapshots ->
        val networkStories = snapshots.toObjects(NetworkStory::class.java)
        NetworkStoryContainer(networkStories).asDomainModel()
    } ?: throw CancellationException("Task is null; local DB not refreshed")
}

This way you'll be returning your data or throwing a CancellationException if null.

When you're working with coroutines inside a ViewModel you have a CoroutineScope ready to be used if you add this dependendy to your gradle file:

androidx.lifecycle:lifecycle-viewmodel-ktx:{version}

So you can use viewModelScope to build your coroutines, which will run on the main thread:

init {
    viewModelScope.launch {
        _stories.value = getStories()
    }

    viewModelScope.launch {
        getMetadata()
    }
}

You can forget about cancelling its Job during onCleared since viewModelScope is lifecycle-aware.

Now all you have left to do is handling the exception with a try-catch block or with the invokeOnCompletion function applied on the Job returned by the launch builder.

Glenn Sandoval
  • 3,455
  • 1
  • 14
  • 22
  • If I wanted to catch the `CancellationException`, how do I achieve that with your code? Shall I wrap the entirety of `withContext(Dispatchers.IO)` block with a `try catch` block? – Richard May 29 '20 at 13:31
  • @Richard It would be easier if you wrap its call with a `try-catch` block inside the `launch` builder. Or you can use the `invokeOnCompletion` function that the `Job` has. Its throwable would not be `null`, that's how you know something went wrong. – Glenn Sandoval May 29 '20 at 13:37
  • I see, would it be something like the following: `viewModelScope.launch { _stories.value = getStories) }.invokeOnCompletion { cause -> catch(cause: CancellationException) { ... } }`? – Richard May 29 '20 at 13:41
  • @Richard More like: `.invokeOnCompletion { cause -> cause?.apply {...}}`. The thing is `cause` won't be `null` if there was a problem. You can do anything you want inside the `apply` block or whatever you use. – Glenn Sandoval May 29 '20 at 13:47
  • I think you meant *cause will be `null` if there isn't any problem*? Can you catch an exception inside an `apply` block? – Richard May 29 '20 at 13:49
  • @Richard You're not catching it inside, you're working with it inside. `cause` is the exception. – Glenn Sandoval May 29 '20 at 13:53