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

Throw String in Kotlin/JS leads to ClassCastException #4290

Closed
ilgonmic opened this issue Dec 4, 2024 · 5 comments
Closed

Throw String in Kotlin/JS leads to ClassCastException #4290

ilgonmic opened this issue Dec 4, 2024 · 5 comments
Labels

Comments

@ilgonmic
Copy link
Contributor

ilgonmic commented Dec 4, 2024

Describe the bug

https://youtrack.jetbrains.com/issue/KT-72174/Kotlin-JS-Coroutines-throwing-JS-string-does-not-stop-the-job

JavaScript allows to throw strings, but it leads to ClassCastException is thrown in kotlinx-coroutines-core
Looks createCauseException is the root cause of ClassCastException , String is not Throwable so it is trying to be cast to ParentJob

// cause is Throwable or ParentJob when cancelChild was invoked
private fun createCauseException(cause: Any?): Throwable = when (cause) {
is Throwable? -> cause ?: defaultCancellationException()
else -> (cause as ParentJob).getChildJobCancellationCause()
}

Provide a Reproducer

class TestClient2 {
    @Test
    fun testThrowString() = runTest {
        val job = launch {
            println("throwing a JS string")
            throw IllegalStateException("EMPTY") //In JS everything can be thrown, even a String!
            println("not executed....")
        }

        println("waiting for job")
        job.join()
    }
}
@ilgonmic ilgonmic added the bug label Dec 4, 2024
@dkhalanskyjb dkhalanskyjb added the js label Dec 4, 2024
@Aditys018
Copy link

Instead of assuming non-Throwable values are ParentJob
We can wrap them in an IllegalStateException with a descriptive message
This maintains type safety while handling JavaScript's flexible throwing behavior
( LMK if this approach is correct I'd love to work on it )

Screenshot 2024-12-05 172359

@dkhalanskyjb
Copy link
Collaborator

dkhalanskyjb commented Dec 5, 2024

Unfortunately, I see no way of fixing this on our side. To wrap an exception, you first have to catch it somehow, and I didn't succeed in doing that without writing custom js("") blocks, and certainly not in the common code of our multiplatform library. I opened an issue for Kotlin/JS: https://youtrack.jetbrains.com/issue/KT-73710/No-way-to-catch-every-JS-error-in-common-code UPD: catch (e: dynamic) does work in JS without custom js("") blocks, but this, too, doesn't compile in common code.

@dkhalanskyjb
Copy link
Collaborator

https://youtrack.jetbrains.com/issue/KT-42465/Provide-a-way-to-catch-everything-in-catch-block-in-common-code states that this is not a supported use case in Kotlin/JS.

@dkhalanskyjb dkhalanskyjb closed this as not planned Won't fix, can't repro, duplicate, stale Feb 24, 2025
@jschneider
Copy link

The main topic is:
When calling other JS libraries within a coroutine we can not guarantee, that this library is well behaved. It might just throw a string or something else.
This breaks the coroutine, which is quite bad - especially if using a supervisor job...

@dkhalanskyjb
Copy link
Collaborator

@jschneider, I see your point, and I even agree, but this out of scope of kotlinx.coroutines. Today, even runCatching { } won't catch thrown strings, which means almost nothing in the Kotlin ecosystem is prepared for this. If Kotlin/JS decides this is worth supporting, we'll certainly do our part (if it won't start working automatically, which it may; in my opinion, it is strictly the responsibility of the compiler to wrap every throwable into a Throwable).

If you want to challenge the decision, it's best to share your thoughts under https://youtrack.jetbrains.com/issue/KT-42465/Provide-a-way-to-catch-everything-in-catch-block-in-common-code

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

No branches or pull requests

4 participants