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

Update Nijato Extensions to cancel suspend Calls silently #23

Open
cwliu opened this issue Aug 3, 2021 · 1 comment
Open

Update Nijato Extensions to cancel suspend Calls silently #23

cwliu opened this issue Aug 3, 2021 · 1 comment
Labels
enhancement New feature or request

Comments

@cwliu
Copy link

cwliu commented Aug 3, 2021

Feature request

Type

Improvement - make what we have better

Is your feature request related to a problem?

After the suspending functionality PR, a Call.Failure will be returned even if it's from a Kotlin CancellationException:

suspend inline fun <reified T> Api.callAsync(crossinline receiver: suspend Api.() -> T): Call<T> = try {
    Call.Success(receiver(this))
} catch (throwable: Throwable) {
    Call.Failure(throwable)
}

that means callers need to handle this canceled result to prevent some error popup might be presented to users unexpectedly.

Describe the solution you'd like

Rethrow CancellationException to the caller, and this CancellationException will be ignored by the coroutines' exception handlers.

suspend inline fun <reified T> Api.callAsync(crossinline receiver: suspend Api.() -> T): Call<T> = try {
    Call.Success(receiver(this))
} catch (throwable: Throwable) {
+   if (throwable is CancellationException) {
+      throw throwable
+   }
    Call.Failure(throwable)
}

The benefits of this change

  1. Can reduce the boilerplate code
  2. Caller won't forget to handle the cancellation result from Call.Failure(throwable)

Additional context

Rethrow CancellationException might be a normal practice on Coroutines. I found these articles

... we catch CancellationException just to execute some actions when a coroutine has been cancelled, but we throw it again to avoid stopping the cancellation.
... in case you want to catch a generic Exception, you should remember to rethrow it in case its actual type is CancellationException otherwise you’ll also suppress the cancellation of coroutines.

from Kotlin Coroutines in Android — Part 5

... Do note that if something throws CancellationException you are generally expected to rethrow it so upstream objects are notified about the cancellation

from How to ignore JobCancellationException?

Also, this one is related to our topic. Kotlin: How to bypass CancellationException

@cwliu cwliu added the enhancement New feature or request label Aug 3, 2021
@Unlimity
Copy link
Member

Unlimity commented Aug 6, 2021

Hi @cwliu ! Thanks for your suggestions. It seems reasonable enough. Want to put up a pull request? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants