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

Fix: Parent Job is Cancelled #78

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

mkovalenkov9
Copy link

When using the plugin, I ran into a problem: when executing http requests to js in the suspend methods generated by the plugin, if there was any unprocessed error that led to the termination of the child job of the root, this led to the termination of all other jobs in this context. After that, when calling the same method, it threw an error: Parent Job is Cancelled. Using supervisor job in context helped me.

In Kotlin, a supervised coroutine scope is typically created to ensure that the failure of a child coroutine does not automatically cancel its sibling coroutines. This can be useful for specific structured concurrency needs.

Here’s how you can achieve a supervised coroutine scope:

  1. Use SupervisorJob with your CoroutineScope
    A SupervisorJob allows for the creation of a coroutine scope where child coroutines fail independently.

  2. Create a scope with SupervisorJob
    You can create a CoroutineScope using a SupervisorJob combined with a Dispatcher (like Dispatchers.Default or Dispatchers.IO).

@mkovalenkov9 mkovalenkov9 changed the title Supervisor job Fix: Parent Job is Cancelled Jan 27, 2025
@ForteScarlet
Copy link
Owner

ForteScarlet commented Jan 27, 2025

Hello!
Firstly, thank you very much for your advice and contribution! 😻
It's a reasonable suggestion, but I have a question now: if we now have CoroutineScope4Js as a globally used scope, is it possible to use GlobalScope directly? 🤔
Have you tried to see how GlobalScope differs from a custom SupervisorJob's scope as a global scope?

My personal opinion so far is that both are pretty much the same, so what's currently in the PR is perfectly acceptable. After a simple test, they both solve the Parent job is Cancelled problem.

Do you have any thoughts on this? If you have room for more testing and comparison then I look forward to your results.
If wish to keep the current changes then let me know and I will merge this PR and release the next version.

@ForteScarlet ForteScarlet added the bug Something isn't working label Jan 27, 2025
@ForteScarlet ForteScarlet self-requested a review January 27, 2025 07:53

private val CoroutineContext4Js: CoroutineContext = Dispatchers.Default
private val CoroutineContext4Js: CoroutineContext = Dispatchers.Default + SupervisorJob()
Copy link
Owner

@ForteScarlet ForteScarlet Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any difference between using a custom scope and GlobalScope?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I tried to use GlobalScope, but it still threw the Parent job is Cancelled exception. This only works with SupervisorJob()

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. The merged version has been released and you can try to update it now!

@ForteScarlet
Copy link
Owner

It doesn't seem like you have any more opinions. Then let's follow the current practice.

@ForteScarlet ForteScarlet merged commit 07d7a9f into ForteScarlet:dev Feb 3, 2025
1 check passed
@mkovalenkov9
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants