[murfel]

As requested by @turansky in #4451

[turansky]

My common request - avoid js blocks - they are redundant :(

[murfel]

Yes, I remember our discussion, but I'd like to merge this as is for now, since it's been a long-standing issue. Later we could apply a refactoring to all our web codebase.

[dkhalanskyjb]

Please describe at least a manual testing procedure for this (it would be challenging to test automatically, since we don't have browser tests enabled, that much is clear). Figuring out from scratch any way to check the validity of this code every time we need to may take a lot of effort, given that Bun and Deno are not part of our daily lives.

[fzhinkin]

It's unclear why we should choose it over setTimeout supported everywhere. If there are any reasons to use reportError if it is available, it's worth explicitly mentioning them here.

[dkhalanskyjb]

The benefit should be clear if we look at this example:

suspend fun main() {
    registerErrorHandler()
    supervisorScope {
        launch {
            val suppressed = IllegalStateException("suppressed")
            val exception = Exception("test")
            exception.addSuppressed(suppressed)
            throw exception
        }
    }
    println("Hello, Kotlin/JS!")
    var i = 0
    while (true) { // HANG FOREVER!!!
        ++i
    }
}

Trying to open this in a browser will lead to a hang. The browser will suggest stopping the script execution on the page. After you do, there's no error in sight, because we terminated the JS runtime before the setTimeout lambda managed to run.

On the other hand, reportError reports the error reliably.

For hanging pages, I'd want to learn of the critical errors that happened right before the hang, so reportError is a clear improvement.

[fzhinkin]

JFTR, there was a way to configure Wasm target to use Deno (https://github.com/Kotlin/kotlin-wasm-wasi-template/blob/ilgonmic/prototype-runtimes/buildSrc/src/main/kotlin/deno/declare.kt), perhaps, a similar trick could be applied to JS too.

[murfel]

Manual testing for browser

I prepped a small demo. Open in IDEA and click run in the run configuration dropdown, a page in browser will open, right-click inspect (in Chrome), go to the console tab, and see the error printed to the console.

To test the proposed behaviour, checkout this branch, publish this branch locally with ./gradlew publishToMavenLocal, and uncomment mavenLocal() and the library dependency in build.gradle.kts.

https://github.com/Kotlin/kmp-js-coroutines

The console printing style changes significantly (see screenshots). Currently, you can clearly see that it came from progagateExceptionFinalResort once uncollapsed. With reportError you can see the suppressed exception (although buried in a stacktrace).

Another behaviour difference is that currently an error event listener wouldn't intercept the exception, but with reportError, it would. (Play around in the demo, registerErrorHandler.)

Current behaviour (uncollapsed error dropdown):

Proposed behaviour with reportError:

[murfel]

@fzhinkin would you like to discuss or add something else? If not, I'll merge.