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

App exits after catching exception thrown by coroutine #753

Closed
kmansoft opened this issue Oct 23, 2018 · 19 comments

Comments

Projects
None yet
5 participants
@kmansoft
Copy link

commented Oct 23, 2018

I'm new to Kotlin and coroutines so maybe I'm doing something wrong, but...

The test code below throws an exception which is propagated to the caller in await().

The caller catches the exception, logs it, and ignores. So far so good.

But - immediately after this, the app exits like this, looks like the exception gets rethrown.

This is quite unexpected to me (but again I'm a novice). Do exceptions propagated by await() always get rethrown? What if I want to ignore it (show message to user, log, etc. but don't want the exception to keep bubbling up)?

        GlobalScope.launch(Dispatchers.Main) {

            val run = async(Dispatchers.IO) {
                if (System.currentTimeMillis() != 0L) {
                    throw IOException("Test")
                }
                "foobar"
            }

            try {
                val res = run.await()
                Log.i(TAG, "Result: " + res)
            } catch (x: Throwable) {
                Log.w(TAG, "Error:", x)
            }
        }
W MainActivity: Error:
W MainActivity: java.io.IOException: Test
W MainActivity: 	at org.kman.updatechecker.MainActivity$startCheckJob$2$run$1.invokeSuspend(MainActivity.kt:99)
W MainActivity: 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:32)
W MainActivity: 	at kotlinx.coroutines.DispatchedTask$DefaultImpls.run(Dispatched.kt:221)
W MainActivity: 	at kotlinx.coroutines.DispatchedContinuation.run(Dispatched.kt:67)
W MainActivity: 	at kotlinx.coroutines.scheduling.Task.run(Tasks.kt:94)
W MainActivity: 	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:586)
W MainActivity: 	at kotlinx.coroutines.scheduling.CoroutineScheduler.access$runSafely(CoroutineScheduler.kt:60)
W MainActivity: 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:732)

^^^^^ This is from test code's Log.w(TAG, "Error:", x)

E AndroidRuntime: FATAL EXCEPTION: main
E AndroidRuntime: Process: org.kman.updatechecker, PID: 8026
E AndroidRuntime: java.io.IOException: Test
E AndroidRuntime: 	at org.kman.updatechecker.MainActivity$startCheckJob$2$run$1.invokeSuspend(MainActivity.kt:99)
E AndroidRuntime: 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:32)
E AndroidRuntime: 	at kotlinx.coroutines.DispatchedTask$DefaultImpls.run(Dispatched.kt:221)
E AndroidRuntime: 	at kotlinx.coroutines.DispatchedContinuation.run(Dispatched.kt:67)
E AndroidRuntime: 	at kotlinx.coroutines.scheduling.Task.run(Tasks.kt:94)
E AndroidRuntime: 	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:586)
E AndroidRuntime: 	at kotlinx.coroutines.scheduling.CoroutineScheduler.access$runSafely(CoroutineScheduler.kt:60)
E AndroidRuntime: 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:732)

^^^^^ But the exception continues to bubble up

W ActivityManager:   Force finishing activity org.kman.updatechecker/.MainActivity

^^^^^ The app is terminated

I'm using Android Studio 3.2, Kotlin 1.3.0-rc-190-Studio3.2-1 and kotlinx-coroutines-android 1.0.0-RC1

@kmansoft kmansoft changed the title App exist after catching exception thrown by coroutine App exits after catching exception thrown by coroutine Oct 23, 2018

@kmansoft

This comment has been minimized.

Copy link
Author

commented Oct 23, 2018

PS:

I see in the exception guide

https://github.com/Kotlin/kotlinx.coroutines/blob/master/docs/exception-handling.md#exception-handling

that launch "propagates exceptions automatically" but async "exposes them to users".

Indeed, changing the code snippet to use async instead of launch makes it work as I expect:

GlobalScope.async(Dispatchers.Main) {

            val run = async(Dispatchers.IO) {
                if (System.currentTimeMillis() != 0L) {
                    throw IOException("Test")
                }
                "foobar"
            }

            try {
                val res = run.await()
                Log.i(TAG, "Result: " + res)
            } catch (x: Throwable) {
                Log.w(TAG, "Error:", x)
            }

            MyLog.i(TAG, "After the exception")
        }

But but but... this still seems pretty weird to me (remember I'm a newbie at Kotlin):

  • Async is for producing a value, launch seems more appropriate for an "async job" that doesn't return anything (but just "does something").

Yes I know it's possible to .await() on async and its type will be just Unit which is "void" but still strange when a particular coroutine "by its nature" doesn't return / produce anything at all (and it can be argued that "returns nothing" isn't quite the same as "returns void").

  • What if there is some library code (somewhere in the coroutine) that throws?

Since Kotlin doesn't have checked exceptions (yes I currently use Java), when writing a coroutine we won't be "told" by the compiler that "hey this can result in an exception, be careful"

  • Last but not least: the docs talk about "propagating exceptions" - but...

There is no exception here. Yes one did occur, but it was caught and not re-thrown.

That's there in the code, look, the exception does not escape, it's done for, dead-ended:

catch (x: Throwable) {
    Log.w(TAG, "Error:", x)
}

I'm sure there are reasons why things are as they are --

-- would it be possible to explain the rationale behind the "automatic rethrow even if caught and ignored" that's done by launch? I don't see anything in the official docs.

@fvasco

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2018

See #576

@kmansoft

This comment has been minimized.

Copy link
Author

commented Oct 23, 2018

Um the discussion in #576 is around what happens when a child coroutine crashes...

... such as from an unhandled exception.

But in this case (in my snippet) - there is no unhandled exception that the coroutine's code allows to escape. The exception is caught and is not rethrown. The coroutine doesn't crash.

There is also a snippet in #576 from @elizarov that looks like this:

launch { // child
    try {
        doSomething() // child's job
    } catch(e: Throwable) {
        // handle failure
    }
}

Which also uses "launch" (not "async") but supposedly will not crash from the exception in child.

I just tried updating my snippet to be in line with that sample snippet:

GlobalScope.launch(Dispatchers.Main) {

            try {

                val run = async(Dispatchers.IO) {
                    if (System.currentTimeMillis() != 0L) {
                        throw IOException("Test")
                    }
                    "foobar"
                }

                val res = try {
                    run.await()
                } catch (x: Throwable) {
                    Log.w(TAG, "Error from await", x)
                    null
                }
                Log.i(TAG, "Result: ${res}")

                MyLog.i(TAG, "After the exception")

            } catch (x2: Throwable) {
                Log.w(TAG, "Error in outside block", x2)
            }
        }

No difference, the app still crashes.

It prints "Error from await" and that exception bubbles up and kills the app and doesn't even get caught in the second catch block (as "x2").


That my coroutine creates another one (on Dispatchers.IO) is purely an implementation detail of this top level coroutine.

This (top level coroutine) already "knows" that this other coroutine can throw, and is prepared to deal with consequences by having a catch block around the await(), doing "something" with the exception and continuing on its merry way:

Retrying a network request after a few seconds. Loading some possibly stale but still "better than nothing" data from disk. Faking it. Whatever. That's purely up to the (top level) coroutine.

But with the way things are - the caller (which creates the top level coroutine) has to be aware of the coroutine's implementation details: "oh there is a try / catch in there, better use async() and avoid launch() which will crash".

The coroutine's implementation can change over time too - today it doesn't make any coroutines itself, in tomorrow it might and there can be exceptions. And then what if this is in a library code?

Am I missing something (clearly I am, being a notice here)?

@fvasco

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2018

Hi @kmansoft
currently launch, async, actor' and produceshare the same behaviour, there is no way to recover a failed child coroutine.asyncmisses of operator likeCompletionStage.exceptionally, if asynccrashes then the whole coroutine scope crashes. You can use thecatch` block to clean up resources, but you cannot recover the error in any way.

So you can use an isolated scope, but you apply its rule for all childs in the scope, or you can use the GlobalScope, but you loose all coroutine scope benefits, or you can handle the error inside the async block, in such case Kotlin coroutine scope enforce Go-like error handling.

@kmansoft

This comment has been minimized.

Copy link
Author

commented Oct 23, 2018

Re: currently launch, async, actor' and produceshare the same behaviour, there is no way to recover a failed child coroutine

Sorry but that doesn't seem to be the case - as I wrote above, changing

GlobalScope.launch

to

GlobalScope.async

at the top level coroutine creation - does stop the "automatic rethrow termination of coroutine".

Re: Go - yes, I know what you mean, I could make sure to catch all exceptions inside coroutines and return them some other way, such as Result<String, Exception> (String or whatever).

In fact I already tried that using this: https://github.com/kittinunf/Result and then wrote my own little helper class.

Both worked fine, but that's more code. I was just expecting that since Kotlin can propagate exceptions from inside a nested coroutine to the caller's "await", across threads, asynchronously - that I could use this built-in mechanism for fine grained exceptions handling.

Oh well. I guess. Thanks for your time.

@fvasco

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

@kmansoft please consider this code, I hope this helps you.

fun main(args: Array<String>) = runBlocking {
    val job = launch {
        val deferred: Deferred<Unit> = GlobalScope.async { error("Out of scope error") }
        try {
            deferred.await()
        } catch (e: Exception) {
            println("Handling error...")
            e.printStackTrace()
        }
    }

    job.join()
    println("That's all")
}
@kmansoft

This comment has been minimized.

Copy link
Author

commented Oct 24, 2018

@fvasco it does, but...

Seems that "the trick" is that you used GlobalScope.async on the child job (not just async).

But this (not using GlobalScope) still crashes from "bubbed up" exception same as before:

        val job = GlobalScope.launch(Dispatchers.Main) {
            val deferred = async(Dispatchers.IO) {
                MyLog.i(TAG, "In async")
                error("Out of scope error")
                "foobar"
            }
            try {
                deferred.await()
            } catch (e: Exception) {
                MyLog.i(TAG, "Handling error...", e)
            }
        }

In other words, one can avoid the child coroutine crashing the parent coroutine if the child is launched into a separate scope - not into the parent's scope?

But isn't doing that have an effect on cancellations? That is, when using global scope, the child will not be cancelled when the parent is canceled?

And speaking of cancellations.... I understand that they're implemented internally by throwing a special exception.

But if that's the case - then any code that uses await() in try / catch and does some sort of error recovery / feedback in the UI if it gets an exception -> will get CancellationException in its catch block, correct?

Think I've actually seen this while experimenting. So if I'm not mistaken, then handling errors from await() would need to look like this:

val child = async {
 // do something, maybe throw on errors
}
val value = try {
 child.await()
} catch (x1: CancellationException) {
// Got cancelled, just peacefully return without doing anything else
return
}
catch (x2: Exception /* or Throwable */) {
// There is a real error, show to user
textView.text = x2.toString()
}

I'm just trying to understand - what is/are good patterns for coroutines so I could: 1) run some child jobs 2) with error (exception) propagation and 3) cancellation

It seems that (1) is really easy and there are some slick features (like using delay() for animations - sweet!, or running parent on Dispatchers.Main and child jobs on Dispatchers.IO - cool).

But I'm having trouble pulling it all together with (2) and (3).

@jcornaz

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

In other words, one can avoid the child coroutine crashing the parent coroutine if the child is launched into a separate scope - not into the parent's scope?

But isn't doing that have an effect on cancellations? That is, when using global scope, the child will not be cancelled when the parent is canceled?

As @fvasco suggested in his first reply, you should use Supervisor scopes. Their purpose is exactly what you're looking for: to not crash the parent in case of child's failure. But cancelling the supervisor does cancel the children.

suspend fun main() {
  val job = GlobalScope.launch {
    
    supervisorScope {  // <-- This prevent the parent from crashing in case of error in a child
     
      val deferred = async { error("Ooops") }
      
      try {
        deferred.await()
      } catch (e: Exception) {
        println("Handling error ($e)")
      }
      
    }
  }

  job.join()
  println("That's all")
}

But if that's the case - then any code that uses await() in try / catch and does some sort of error recovery / feedback in the UI if it gets an exception -> will get CancellationException in its catch block, correct?

Yes. But you may ignore cancellation state of the coroutine with withContext(NonCancellable) {}

@kmansoft

This comment has been minimized.

Copy link
Author

commented Oct 25, 2018

Re: But cancelling the supervisor does cancel the children.

OK this is a useful property combination. Thank you.

Re: Yes. But you may ignore cancellation state of the coroutine with withContext(NonCancellable) {}

The point was not to have non-cancelable coroutines.

My point was - if the "parent" expects that a child can fail and is prepared to deal with that, by using a catch block around "await child" --

-- then this catch block will also trigger on CancellationException if the job (and the child) is canceled.

This means that the catch block has two have two separate stanzas, one for Cancellation and one for Exceptions (real failures from child, not cancellations).

I gave an example of this in my previous commend, with exceptions "x1" and "x2".

Is there a way to simplify that?

Again all I'm looking for is a simple (DRY, boilerplate free as much as possible) pattern where a coroutine can delegate to other coroutines and handle their exceptions, and where cancellations also work and don't require additional code (to differentiate from genuine failures).

?

@jcornaz

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2018

This means that the catch block has two have two separate stanzas, one for Cancellation and one for Exceptions (real failures from child, not cancellations)

It depends. In all cases (cancellation of child, cancellation of parent and failure of child), await cannot return. It does not really matter why. The important point is that we cannot get a result from await. Of course if you want to have different behavior depending on what cause await to not return you can. But if you want it you have to explicitly write it.

I gave an example of this in my previous commend, with exceptions "x1" and "x2".

Is there a way to simplify that?

Keep in mind that in both cases if coroutine is cancelled or failed the important point is : "it did not succeed". If you want to have different behavior depending on why it did not succeed, you have to explicitly handle it. (personally I would also show the user that the action cannot be completed because of a cancellation)

Again all I'm looking for is a simple (DRY, boilerplate free as much as possible) pattern where a coroutine can delegate to other coroutines and handle their exceptions, and where cancellations also work and don't require additional code (to differentiate from genuine failures).

Try as much as you can to use suspend function instead of using async everywhere. It simplifies a lot the code. You may also write few small inline functions to simplify common pattern in your application.

inline fun ignoreCancellationException(block: () -> Unit) {
  try {
    action()
  } catch (e: CancellationException) {
    // ignore
  }
}

suspend fun doSomethingMaybeThrowOnErrors() { ... }

suspend fun usage() {
  ignoreCancellationException { doSomethingMaybeThrowOnErrors() }
}
@kmansoft

This comment has been minimized.

Copy link
Author

commented Oct 26, 2018

It depends. In all cases (cancellation of child, cancellation of parent and failure of child), await cannot return. It does not really matter why.

This is true of course. But the "awkwardness" (to me) is that when a child has an exception, the runtime by default treats it as a catastrophic failure in the whole scope.

Consider "plain" (synchronous) functions:

fun loadFromNetwork(...) {
val httpRequest = httpClient.execute(...)
// Can throw IOException
// Cannot be handled here - this function doesn't know enough
// Just lets IOException escape, the caller will decide what to do
}

fun loadFromFile(...) {
val fileStream = FileInputStream(...)
// Can throw IOException
// Cannot be handled here - this function doesn't know enough
// Just lets IOException escape, the caller will decide what to do
}

// The caller knows more:

fun loadData() {
  try {
    loadFromNetwork()
  } catch (x1: IOException) {
      // Loading from network failed, load from file if it exists
      if (dataFile.exists()) {
        loadFromFile()
        // 
        // This is OK, the network is down, but we still have (older) data from file
        // 
      } catch (x2: IOException) {
         // Neither worked
         // ....
         return
      } else { /* file does not exist, etc. */ }
  }
}

Here the parent has catch blocks - so exceptions are handled. But it might not have, and exceptions would escape - and then it'd be up to the caller of loadData, or its caller, etc. and a catastrophic failure would happen only if there were no catch blocks anywhere up the call chain.

In other words, an exception in some child function is not necessarily a catastrophic failure.

If this exception is handled somewhere up the call chain, then it is handled and "forgotten", has no further effect. Either way the function that originated the exception (and maybe more functions) is (are) unwound (= what you wrote about how "we cannot get a value from a coroutine that did not suceed").

Now with coroutines - even when there is a catch in the caller, an exception in a child, by default, is a catastrophic failure.

Your earlier suggestion to use supervisorScope works, but it's more boilerplate code. I already have a catch() in the parent, so it's strange to have to do more stuff to indicate to the language / runtime that the parent is able to handle / ignore the exception.

OK, so maybe that's just a matter of defaults. Fine. And maybe this particular default was chosen to make other cases more elegant. Fine.

Try as much as you can to use suspend function instead of using async everywhere. It simplifies a lot the code.

Can you explain how to do this?

It's just that all the more or less complete examples I've seen (so far) use patterns like this:

GlobalScope.launch(Dispatchers.IO) {
..
}

launch(Dispatchers.Main .... ) {
}

async(Dispatchers ... ) {
}

launch {
}
async {
}

Let't say I already have a suspend fun (which runs on Main let's say) - and I want to have coroutines that run on IO.

How do I do this with "suspend function instead of using async everywhere"?

@jcornaz

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2018

Here the parent has catch blocks - so exceptions are handled. But it might not have, and exceptions would escape - and then it'd be up to the caller of loadData, or its caller, etc. and a catastrophic failure would happen only if there were no catch blocks anywhere up the call chain.

It works exactly the same with suspending functions. This is also the reason of my advice "favor suspending function". At the end the very purpose of Kotlin coroutines is suspending functions. Without suspending function, there would be no benefit of Kotlin coroutines over futures/promises.

Try as much as you can to use suspend function instead of using async everywhere. It simplifies a lot the code.

Can you explain how to do this?

It's just that all the more or less complete examples I've seen (so far) use patterns like this: [...]

This kind of pattern is legacy from future-oriented paradigm and libraries. Coroutines aim to provide a new approach by adding suspend modifier in the language itself, which simplifies a lot. launch and async are meant to be used only at the most top level for parallel decomposition.

Here's an example:

suspend fun workOnIO() = withContext(Dispatchers.IO) { 
	try {
		// work
	} catch(e: ExpectedException) {
		// ignore or handle this exception
	} finally {
		// release resources
	}
}

suspend fun parallelDecomposition() = coroutineScope {
	val a = async { ... }
	val b = async { ... }
	
	combine(a.await(), b.await())
}

class ViewController : CoroutineScope {
	private val job = SupervisorJob()
	override val coroutineContext = job + Dispatchers.Main
	
	fun handleAction() {
		launch {
			workOnIO()
			try {
				parallelDecomposition()
			} catch (e: ExpectedException) {
				return@launch
			}
			updateUI()
		}
	}

	fun dispose() {
		job.cancel()
	}       
}

You may also read one of my comment here: #342 (comment) Were I already gave quite thourough explanation about "why suspend is preferable"

Let't say I already have a suspend fun (which runs on Main let's say) - and I want to have coroutines that run on IO.

it depends on what you want exactly. Why do you need a "coroutine" to run on IO? Is it an actor? Is it supposed to have an independent life scope? Or is it just an async which do something and return a result? In this last case, you don't need a "coroutine". A suspend function with withContext if fairly enough, and will simplify your code.

@kmansoft

This comment has been minimized.

Copy link
Author

commented Oct 26, 2018

Why do you need a "coroutine" to run on IO? Is it an actor? Is it supposed to have an independent life scope? Or is it just an async which do something and return a result?

An async to do something and return a result (or exception or have the whole thing cancel).

And I am expecting to find an an easy, language- and runtime- supported method of doing this "sequentially" in terms of syntax, but "asynchronously" in terms of "what really happens" - which fits what you wrote in #342

So, the whole point of Kotlin coroutines is to not use callback and future anymore letting us to write code like sequential code while keeping the benefits of future and callbacks.


Here's an example:

Thanks, I'll try it. Like I said, it seems that most tutorials (that I've seen) used launch / async so I initially started exploring in that direction.

Thank you for the clarification and for your time.

@jcornaz

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2018

You probably also want to follow #763 which address the problem of async's astonishing behavior.

@kmansoft

This comment has been minimized.

Copy link
Author

commented Oct 26, 2018

@jcornaz I just tried it, yes nice that there is less "syntax noise" but (still!)...

The "recipe" you gave is very close to one found here:

https://proandroiddev.com/async-code-using-kotlin-coroutines-233d201099ff

Let me step back and restate what I'm trying to do.

Consider this all synchronous code, no coroutines, async / await's or anything:

fun showOrder(accountId: String, orderId: OrderId) {
  val account = try {
    getAccount(accountId)
  } catch (x: Execption) {
    // Network error, whatever, inform the user
    accountView = "Error getting account info: " + x.toString()
    return
   }

  if (isCancelled) {
    return
  }

  accountView = account.formatAsText()

  val order = try {
    account.getOrder(orderId)
  } catch (x: Exception) {
    // Error, inform the user
    orderView = "Error geting order info: " + x.toString()
    return
  }

  if (isCancelled) {
    return
  }

  orderView = order.formatAsText()
}

Things to note:

  1. I am able to handle errors close to where they can occur, to show the most relevant message to the user (not just "error" but "error getting account / order info");

  2. Cancellations (without trying to continue with updating the UI which perhaps has gone away);

  3. The operations are sequential (can't get order object without account object), but I don't want to make a new function that combines both and returns a single result - this is the function I'm writing here already.

The problem here is of course that getAccount / getOrder need to execute on an IO thread, and updating the UI needs to execute on the "main" (UI, GUI toolkit) thread.

Kotlin coroutines to the rescue! I can just run some parts of my code on an IO thread and other parts on the UI thread - but write the logic as if it were sequential and synchronous.

val job = SupervisorJob()
GlobalScope.launch(job + Dispatchers.Main) {
  try {
    // Get account
    val awaitAccount = withContext(Dispatchers.IO) {
      async {
          getAccount()
      }
    }

    val valAccount = try {
      awaitAccount.await()
    } catch (x: Exception) {
      // Error getting account
      accountView = "Error getting account info: " + x.toString()
      return@launch
    }

    // Got account, update the UI
    accountView = valAccount.formatAsText()

    // Get order
    val awaitOrder = withContext(Dispatchers.IO) {
      async {
        account.getOrder()
      }
    }

    val valOrder = try {
      awaitOrder.await()
    } catch (x: Exception) {
      // Error getting order
      orderView = "Error getting order info: " + x.toString()
      return@launch
    }

    // Got order, update the UI
    orderView = valOrder.formatAsText()
  } catch (x: Exception) {
    // Log / show the exception
  }
}

This magically executes every piece of code on the appropriate thread. Wonderful.

But for some reason, any exceptions thrown by getAccount / getOrder are now not caught in the catch blocks around the respective await()'s.

This is "astonitishing" again, there clearly are catch blocks around the awaits, and it's awaits which propagate exceptions, right?

Exceptions are caught only in the top-level catch ("Log / show the exception") now and are thrown out of async{} blocks. Before adding withContext, they were getting thrown out of await()'s.

And so, new questions if you don't mind:

  1. Why are my exceptions now getting thrown out of async{} (vs. before withConext: out of await)?

  2. I feel like I'm stumbling in the dark, chanting magical incantations (which sometimes work, but more often turn random objects into stones or worse)...

Are there any guides that explain in more depth what's going on?

I have read this:

https://kotlinlang.org/docs/reference/coroutines/exception-handling.html

... but it just says that "async exposes exceptions to uses" and doesn't explain why the introduction of withContext exposes them out of async{} and not from await(),

@jcornaz

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2018

You still use async which is unnessacary in your code, and introduce the astonishing behavior.

If I translate your first (blocking) version of showOrder in an (non-blocking) equivalent with Kotlin coroutines it will be:

suspend fun showOrder(accountId: String, orderId: OrderId) = withContext(Dispatchers.IO) { // <-- Let's use IO by default here
    val account = try {
        getAccount(accountId) // <-- It's fine, we are on IO.
    } catch (x: Execption) {
        // Network error, whatever, inform the user

        withContext(Dispatchers.Main) {
            // This has to run on UI I presume...
            accountView = "Error getting account info: " + x.toString()
        }

        return@withContext
    }

    // No need to check cancellation state here. Any suspending function like `withContext` would throw a cancellation exception

    withContext(Dispatchers.Main) {
        // This has to run on UI I presume...
        accountView = account.formatAsText()
    }

    val order = try {
        account.getOrder(orderId) // <-- It's fine, we are on IO.
    } catch (x: Exception) {
        // Error, inform the user

        withContext(Dispatchers.Main) {
            // This has to run on UI I presume...
            orderView = "Error geting order info: " + x.toString()
        }

        return@withContext
    }

    // No need to check cancellation state here. Any suspending function like `withContext` would throw a cancellation exception
    
    withContext(Dispatchers.Main) {
        // This has to run on UI I presume...
        orderView = order.formatAsText()
    }
}

You see that it is very much the same. The only difference are the suspend keyword for the function and the use of withContext wherever needed. This make the code as easy to read as if it was blocking.

Of course you need to adapt the usage, since you can only call a suspending function from a coroutine:

fun handleUserAction()  {
  job = launch { showOrder(accountId, orderId) }
}

fun cancelAction() {
  job?.cancel()
}
@jcornaz

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2018

... but it just says that "async exposes exceptions to uses" and doesn't explain why the introduction of withContext exposes them out of async{} and not from await(),

this is not about withContext. It is just the fact that async make the parent fail as soon as async is failed. That's it. I aggree it can be astonishing when comming from other technologies. This problem is discussed in #763.

My point is "you don't need it". One need async only for parallel decomposition. For other uses-cases just use suspending function (without using async at all) and you will get simple code with a simple mental model, since it is the same than the one you use to write blocking code.

@kmansoft

This comment has been minimized.

Copy link
Author

commented Oct 27, 2018

You see that it is very much the same. The only difference are the suspend keyword for the function and the use of withContext wherever needed.

Yes it is, just one thing, you switched the threads around.

As a side benefit you got better defined cancellation points (i.e. every time the code tries to reach into the UI state).

You still use async which is unnessacary in your code, and introduce the astonishing behavior.

I see it now - yesterday I misread your ViewController example, and saw something (async) which wasn't there.

I now changed my code to use only withContext without async:

GlobalScope.launch(job + Dispatchers.Main) {
	val account = try {
		withContext(Dispatchers.IO) {
			network.getAccount()
		}
	} catch (x: Throwable) {
		// This does get caught
	}
}

It now works much more to my taste.

Not "astounding" anymore. Exceptions get propagated and caught in the nearest catch blocks.

And then the whole withContext block now returns the "real" value type (not a Deferred which then needs to be await'ed on!) which also simplifies code structure.

Two remaining problems are that exceptions from inside withContext take priority over cancellations - and that it's necessary to catch CancellationException separately from other Throwable's (to differentiate cancellations from errors).

But for those I bet a utility function would work just fine.

@elizarov

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

I'm closing this issue. If there are still open problems, please report them separately.

@elizarov elizarov closed this Apr 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.