propagateExceptionFinalResort implementation for wasm-js and js confusing users

Both implementations call console.error and pass exception.toString(), which is basically a name of Exception class + message. In addition, console.error prints a stack trace from a call site (not from the exception!!), which users often consider a stack trace from the exception (example).

In this project, you can find a project with a simple repro and possible improvements implemented via CoroutineExceptionHandler.

I think #1 or any other way to rethrow the exception is the best option since there are standard ways inside JS runtimes to process unhandled exceptions. These might be used directly or by some library intended to handle problems on a page. In the worst case, a browser (runtime) will show the error in a standard and more convenient way.

Here is what it looks like
For Wasm

For JS

Possible improvements

Wasm

JS

(Other examples of people, including me, being confused: #4213)

Yes, thanks, this would be a welcome change. After some research, it looks like the setTimeout approach is the most idiomatic one, and it does fulfill the requirement of being a last-resort exception propagation mechanism: running setTimeout(function() { throw new Error("Crash"); }, 10000) in my browser, I do see the exception with the stack trace, and it looks like Node.JS crashes the application in this scenario, just like we do on Native and on Android, unless the unhandledrejection handler is set: https://stackoverflow.com/questions/74791016/node-js-unhandledrejection-with-async-function-calls-that-are-not-awaited . On the non-Android JVM, we also respect the installed https://docs.oracle.com/javase/7/docs/api/java/lang/Thread.UncaughtExceptionHandler.html, so this is completely consistent.

Would you like to make a pull request with this change?

Would you like to make a pull request with this change?

Not anytime soon, so I'd appreciate it if someone could take it and fix it.

@turansky shared some concerns with me privately re the merged PR #4472, summarizing here:

with setTimeout exceptions can arrive unordered, suggested to use a custom handler like this

kotlinx.coroutines/kotlinx-coroutines-core/common/src/internal/CoroutineExceptionHandlerImpl.common.kt

Line 10 in f4f519b

internal expect val platformExceptionHandlers: Collection<CoroutineExceptionHandler>
Error.name is a magic field in JS and it's dangerous to write className there (docs), usually it describes a group of similar exceptions, in this case, it could be "something from coroutines"
use external for js blocks for easier readability

I will react to this shortly

suggested to use a custom handler like this

Current implementation with setTimeout has big bonus - it shows errors by default

with setTimeout exceptions can arrive unordered

I mean common errors order.
Errors from non-coroutines code we receive at time.
Errors from coroutines code we receive with delay.

cc @murfel

@murfel
2. Overriding name for custom errors is normal practice[1] and improves DX. Often, it also implies inheritance, but I think it could be omitted in simple cases.

Could you please share more about what is going to be changed for 1 & 3?

Overriding name for custom errors is normal practice[1] and improves DX

In current implementation it can clash with standard error names - it's what we should avoid

In current implementation it can clash with standard error names - it's what we should avoid

Well, we can consider using a qualified name, but it may look unusual from JS side.
Any other suggestions?

usually it describes a group of similar exceptions, in this case, it could be "something from coroutines"

Default expectation - some constant and unique name like "UnhandledKotlinWasmCoroutinesError"

@murfel also for WasmJS it's important, that you can receive JsException, which can contains original js Error, which you can/should use without redundant wrapping.

Throwable.toJsException -> Throwable.toJsError (JsError)

Default expectation - some constant and unique name like "UnhandledKotlinWasmCoroutinesError"

A runtime usually writes something like "Uncaught Exception", everything else should be visible from the stacktrace.

Normally, we should throw the original exception, but IIRC it doesn't work well for pre 2.2.20 compiler, with 2.2.20-Beta2+ it should be good.

@murfel please use standard reportError by default.
Workaround with setTimeout required for Node.js only.
Declaration

[Discussed privately, please discard.]

@turansky I'm a bit unfamiliar with this part, do you happen to know how do I introduce JsError in the coroutine library code for the wasmJs target?

E.g. for JsAny there's kotlin.js.JsAny, but there's no JsError there, only JsException which I assume is something completely different.