-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Propagate exception to wasm-js and js in propagateExceptionFinalResort
#4472
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
Changes from all commits
51cde3a
be4173a
1f78da0
8d7c1ec
4a7845d
7dbce2c
b21dd05
485db2c
ed9d26b
0e480cd
6c1098f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
package kotlinx.coroutines.internal | ||
|
||
import kotlinx.coroutines.* | ||
import kotlin.js.unsafeCast | ||
|
||
internal actual fun propagateExceptionFinalResort(exception: Throwable) { | ||
// log exception | ||
console.error(exception.toString()) | ||
} | ||
internal actual external interface JsAny | ||
|
||
internal actual fun Throwable.toJsException(): JsAny = this.unsafeCast<JsAny>() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package kotlinx.coroutines | ||
|
||
import kotlinx.coroutines.testing.* | ||
import kotlin.js.* | ||
import kotlin.test.* | ||
|
||
class PropagateExceptionFinalResortTest : TestBase() { | ||
@BeforeTest | ||
private fun removeListeners() { | ||
// Remove a Node.js's internal listener, which prints the exception to stdout. | ||
js(""" | ||
globalThis.originalListeners = process.listeners('uncaughtException'); | ||
process.removeAllListeners('uncaughtException'); | ||
""") | ||
} | ||
|
||
@AfterTest | ||
private fun restoreListeners() { | ||
js(""" | ||
if (globalThis.originalListeners) { | ||
process.removeAllListeners('uncaughtException'); | ||
globalThis.originalListeners.forEach(function(listener) { | ||
process.on('uncaughtException', listener); | ||
}); | ||
} | ||
""") | ||
} | ||
|
||
/* | ||
* Test that `propagateExceptionFinalResort` re-throws the exception on JS. | ||
* | ||
* It is checked by setting up an exception handler within JS. | ||
*/ | ||
@Test | ||
fun testPropagateExceptionFinalResortReThrowsOnNodeJS() = runTest { | ||
js(""" | ||
globalThis.exceptionCaught = false; | ||
process.on('uncaughtException', function(e) { | ||
globalThis.exceptionCaught = true; | ||
}); | ||
""") | ||
val job = GlobalScope.launch { | ||
throw IllegalStateException("My ISE") | ||
} | ||
job.join() | ||
delay(1) // Let the exception be re-thrown and handled. | ||
val exceptionCaught = js("globalThis.exceptionCaught") as Boolean | ||
assertTrue(exceptionCaught) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package kotlinx.coroutines.internal | ||
|
||
import kotlinx.coroutines.* | ||
|
||
internal expect interface JsAny | ||
|
||
internal expect fun Throwable.toJsException(): JsAny | ||
|
||
/* | ||
* Schedule an exception to be thrown inside JS or Wasm/JS event loop, | ||
* rather than in the current execution branch. | ||
*/ | ||
internal fun throwAsync(e: JsAny): Unit = js("setTimeout(function () { throw e }, 0)") | ||
|
||
internal actual fun propagateExceptionFinalResort(exception: Throwable) { | ||
throwAsync(exception.toJsException()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @murfel @dkhalanskyjb can we throw an exception right from here? May it break anything inside kx-coroutines? In my demo project, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this method is not allowed to fail. It's called deep inside the coroutine internals, and if it throws anything, things will break. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From kotlin 2.2.20-Beta2, in kotlin code, you can just throw a kotlin exception, and everything else will be done by kotlin runtime/compiler, including unwrapping
But this way, users using old toolchain will get a We can try kotlin version at runtime and use a fallback for older versions, but should we? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can replace this with just |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,16 @@ | ||
package kotlinx.coroutines.internal | ||
|
||
import kotlinx.coroutines.* | ||
internal actual typealias JsAny = kotlin.js.JsAny | ||
|
||
internal actual fun propagateExceptionFinalResort(exception: Throwable) { | ||
// log exception | ||
console.error(exception.toString()) | ||
} | ||
internal actual fun Throwable.toJsException(): JsAny = | ||
toJsError(message, this::class.simpleName, stackTraceToString()) | ||
|
||
internal fun toJsError(message: String?, className: String?, stack: String?): JsAny { | ||
js(""" | ||
const error = new Error(); | ||
error.message = message; | ||
error.name = className; | ||
error.stack = stack; | ||
return error; | ||
""") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
package kotlinx.coroutines | ||
|
||
import kotlinx.coroutines.testing.TestBase | ||
import kotlin.test.* | ||
|
||
class PropagateExceptionFinalResortTest : TestBase() { | ||
@BeforeTest | ||
private fun addUncaughtExceptionHandler() { | ||
addUncaughtExceptionHandlerHelper() | ||
} | ||
|
||
@AfterTest | ||
private fun removeHandler() { | ||
removeHandlerHelper() | ||
} | ||
|
||
/* | ||
* Test that `propagateExceptionFinalResort` re-throws the exception on Wasm/JS. | ||
* | ||
* It is checked by setting up an exception handler within Wasm/JS. | ||
*/ | ||
@Test | ||
fun testPropagateExceptionFinalResortReThrowsOnWasmJS() = runTest { | ||
val job = GlobalScope.launch { | ||
throw IllegalStateException("My ISE") | ||
} | ||
job.join() | ||
delay(1) // Let the exception be re-thrown and handled. | ||
assertTrue(exceptionCaught()) | ||
} | ||
} | ||
|
||
private fun addUncaughtExceptionHandlerHelper() { | ||
js(""" | ||
globalThis.exceptionCaught = false; | ||
globalThis.exceptionHandler = function(e) { | ||
globalThis.exceptionCaught = true; | ||
}; | ||
process.on('uncaughtException', globalThis.exceptionHandler); | ||
""") | ||
} | ||
|
||
private fun removeHandlerHelper() { | ||
js(""" | ||
process.removeListener('uncaughtException', globalThis.exceptionHandler); | ||
""") | ||
} | ||
|
||
private fun exceptionCaught(): Boolean = js("globalThis.exceptionCaught") |
Uh oh!
There was an error while loading. Please reload this page.