Decided to write about several things which in my opinion yo

Question

Decided to write about several things which in my opinion you should and shouldn t do or at least try to avoid when using Kotlin coroutines Would appreciate if someone can check it and give a feedback wink <https medium com dmytrodanylyk kotlin coroutines patterns anti patterns f9d12984c68e>

oshai · Answer

I have 2 suggestions say in the title it is related to androind and make the example more clear what is wrong and right

bdawg.io · Answer

In code examples and the medium post preview image `withContext` instead of `witchContext` unless that s a new method I haven t come across yet

bdawg.io · Answer

Any particular reason you re utilizing your `networkClient` on `Dispatchers Default` instead of `<http Dispatchers IO|Dispatchers IO>`

Tolriq · Answer

I would have the classWorkManager extend `CoroutineScope` directly I would use `SupervisorJob` to avoid parent Job cancellation And to avoid recreating a job use `coroutineScope cancelChildren`

bdawg.io · Answer

I was just coming back on to suggest `cancelChildren` < Tolriq> haha

Dmytro Danylyk · Answer

Doh the typo I will fix that

Dmytro Danylyk · Answer

< bdawg io> regarding networkClient no reason just to switch context to any dispatcher which operates on background thread but I will be fixing type anyway so IO it is

dave08 · Answer

At first you recommend using Main as a scope and then you put withContext Main in the login fun I think it should be the opposite the network call should have a withContext IO and be called from the scoped to Main presenter and there win t be any crashes what do you think < Dmytro Danylyk>

Dmytro Danylyk · Answer

< dave08> yes you are right ideally it should be done that way But in the example of Avoid writing suspend function with an implicit dispatcher I wanted to show that if you do have the case where you need to write suspend function which touch UI and does some background work you need to do that correctly

Dmytro Danylyk · Answer

< dave08> think of it as when you have a library which handles all the logic and UI and expose for you suspend function `login`

dave08 · Answer

Even then any network calls should be called in the right context in their implementation no

dave08 · Answer

Especially if they re `suspend` and blocking

dave08 · Answer

So that lib would also have its context set to Main

dave08 · Answer

It might be nice to put this extra note for good practices

Dmytro Danylyk · Answer

< dave08> yes so the correct implementation is shown later on ``` suspend fun login Result = withContext Dispatcher Main view showLoading val result = withContext networkClient login view hideLoading return result ```

dave08 · Answer

Well I d move the network call s withContext into the function declaration and make it suspending

dave08 · Answer

Then the code would be much cleaner

dave08 · Answer

And safer you won t forget the context

dave08 · Answer

I liked your point about recreating the job after cancelling though wink

Tolriq · Answer

Should not do that wink Multithreading can lead to a job start between the cancel and the new creation coroutineScope cancelChildren is the proper way to avoid issues cancel all child jobs without the parent And for a workerpool with job that can fail you should really really use SupervisorJob as any job that fails will cancel the parent otherwise

Dmytro Danylyk · Answer

< Tolriq> if your job fail and you didn t handle that it will crash your app So what the point of always using SupervisorJob

Tolriq · Answer

Not necessarily you can have CoroutineExceptionHandler you can have async await that even if exception is catched will cancel the parent too

Tolriq · Answer

Tolriq · Answer

So not always use SupervisorJob but for Generic stuff like a WorkManager it s certainly a good idea And since you want users to learn it s better to explain the main concepts at that time as they will fall into this at some point

Tolriq · Answer

Coroutine docs are not yet fully end user ready and details information are a little spread disappointed

Dmytro Danylyk · Answer

< Tolriq> I didn t know that when you attach CoroutineExceptionHandler it will cancel parent job as well I need to check that

Tolriq · Answer

Not sure it happens in all cases but the main pain point is async await that does that 100

Dmytro Danylyk · Answer

Is it when you catch exception inside async function or when you wrap await with try catch ``` try work await catch e Exception ```

Tolriq · Answer

when you wrap it

Tolriq · Answer

And it s how async is meant to be used for exception handling

Dmytro Danylyk · Answer

So the code above will cancel parent job

Tolriq · Answer

yep

Tolriq · Answer

see linked isssue for details

Tolriq · Answer

well it does cancel the job that by default cancel parent unless using supervisor

Dmytro Danylyk · Answer

< Tolriq> thanks that s interesting not sure how I missed that probably because I don t use async too often I will add this a separate advice

Tolriq · Answer

There s many subtle things that are hard to spot disappointed Like <https github com Kotlin kotlinx coroutines issues 830> many many people show that as example and it s wrong With unwrapped exception rethrown meaning very very hard debugging

Tolriq · Answer

Docs will improve with time but it s better if all tutorial that are currently being written take those in account already

Dmytro Danylyk · Answer

Thanks everyone for help Send me your twitter or medium account name if you want me to add link to your account in the Special thanks section of the article

louiscad · Answer

< Dmytro Danylyk> I d add that for async await and catching errors properly you should do `async` and `await` calls inside a `coroutineScope ` lambda and catch any exception throwable outside of this very scope ``` try coroutineScope val mayFailAsync1 = async mayFail1 val mayFailAsync2 = async mayFail2 useResult mayFailAsync1 await mayFailAsync2 await catch e IOException handle this catch e AnotherException handle this too ```

louiscad · Answer

The snippet above is the real deal behind structured concurrency That s the pattern < elizarov> is showing in its latest presentations like this one

gildor · Answer

```withContext <http Dispatcher IO|Dispatcher IO> networkClient login ``` Not sure that this is a good example even with IO dispatcher Usage of blocking API of networking library is already a bad practice itself blocking additional thread no cancellation and such example does encourage this If you replace networkClient login to something like `someBlockibgCall ` it would be more explicit

gildor · Answer

Agree with point of Dave about bad style to wrap blocking calls right in coroutine block instead extracting them to own suspend functions this is bad style imo that couse pollution of business logic with thread switching

pakoito · Answer

< louiscad> how is that better than a `handleErrorWith` operator is beyond me

pakoito · Answer

for a low level API it s okay I wouldn t expect to write that code multiple times through the codebase when it could be abstracted

gildor · Answer

What is `handleErrorWith`

louiscad · Answer

< pakoito> Extracting error handling to a domain specific function to not have try catch blocks all over the place is a good practice but that doesn t free you from using structured concurrency properly

pakoito · Answer

``` useResult mayFailAsync1 await mayFailAsync2 await handleErrorWith when it is ```

pakoito · Answer

< gildor> wrapping fail but you get the point

pakoito · Answer

because suspended functions have no types there s no general extfun you can create to abstract this pattern so you have to resort to per case solutions

louiscad · Answer

< pakoito> That doesn t follow structured concurrency because not wrapped into a local `coroutineScope ` so it is plain unsafe

pakoito · Answer

okay move the handleErrorWith extfun to coroutineScope then

pakoito · Answer

same

pakoito · Answer

except you cannot handle and recover from individual errors for each of them or can you

pakoito · Answer

that s not entirely clear

pakoito · Answer

and how does it interact with cancellation

louiscad · Answer

You can do everything but you should do it outside of the `coroutineScope ` call

pakoito · Answer

so you have to create a sub scope per async call to recover from errors in it

pakoito · Answer

and make that subscope inherit from the parent scope

pakoito · Answer

is that correct

louiscad · Answer

`coroutineScope ` what you call the subscope is automatically cancelled if the `CoroutineContext` it was called from is cancelled And yes each time you want to do parallelization with `async` and `await` you have to make sure you have a local scope where you can recover from the outside but in practice you do not do parallelization so often as calling suspend functions sequentially

pakoito · Answer

thank you for the rundown < louiscad> I ll apply those to our Deferred wrapper

Jorge Castillo · Answer

I guess that s why our wrapper is not supporting cancelation properly now

louiscad · Answer

Probably half of the reason The other is that cancellation is not built into Kotlin itself but depends on kotlinx coroutines Making something without kotlinx coroutines means making your own system that is incompatible with the one in kotlinx coroutines unless you achieve implementing a two way hack I think it is possible one way on JVM only using reflection but not possible two way IMHO

Dmytro Danylyk · Answer

gt If you replace networkClient login to something like `someBlockibgCall ` it would be more explicit thumbsup

pakoito · Answer

< louiscad> we have our own implementation working end to end for our datatype `IO` already Now we have to comply with the same behavior bridging Deferred It s possible just a moving target at the moment

Jorge Castillo · Answer

I guess we re talking about this <https kotlinlang org docs reference coroutines cancellation and timeouts html cancellation is cooperative>

pakoito · Answer

that resource is outdated because it doesn t take scopes into account from what I could gather in this conversation

louiscad · Answer

< pakoito> Cancellation is still cooperative that didn t change with structured concurrency

Alexey Pushkarev · Answer

< Dmytro Danylyk> why launch here is without any scope passing

Dmytro Danylyk · Answer

< Alexey Pushkarev> because activity implements `CoroutineScope` so launch will be executed using that scope