Skip to content

Commit f98a4bb

Browse files
authored
Propagate errors through val coroutine hierarchy. (#23653)
Fix bug when failure or exception happening in a >1 level of the val coroutine call hierarchy is not being propagated. As a result only deep most level coroutine promise is rejected, other val promises including top most one remain in a perpetual pending state. The bug can be reproduced by the following example: ``` cpp emscripten::val fetch(const std::string& url) { co_return co_await emscripten::val::global("fetch")(url); } emscripten::val json(const std::string& url) { auto response = co_await fetch(url); co_return co_await response.call<emscripten::val>("json"); } EMSCRIPTEN_BINDINGS(module) { function("json", &json); } ``` ```js let p = Module.json('invalid.url'); // JS error is printed, but p remains pending try { let p = await Module.json('invalid.url'); // JS error is printed } catch(e) { console.log('Caught coro error!'); // this line is never printed } ```
1 parent 6ac5f36 commit f98a4bb

File tree

8 files changed

+116
-30
lines changed

8 files changed

+116
-30
lines changed

src/lib/libemval.js

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -437,33 +437,33 @@ var LibraryEmVal = {
437437
return result.done ? 0 : Emval.toHandle(result.value);
438438
},
439439

440-
_emval_coro_suspend__deps: ['$Emval', '_emval_coro_resume'],
441-
_emval_coro_suspend: async (promiseHandle, awaiterPtr) => {
442-
var result = await Emval.toValue(promiseHandle);
443-
__emval_coro_resume(awaiterPtr, Emval.toHandle(result));
440+
_emval_coro_suspend__deps: ['$Emval', '_emval_coro_resume', '_emval_coro_reject'],
441+
_emval_coro_suspend: (promiseHandle, awaiterPtr) => {
442+
Emval.toValue(promiseHandle)
443+
.then((result) => __emval_coro_resume(awaiterPtr, Emval.toHandle(result)),
444+
(error) => __emval_coro_reject(awaiterPtr, Emval.toHandle(error)));
444445
},
445446

446-
_emval_coro_make_promise__deps: ['$Emval', '__cxa_rethrow'],
447+
_emval_coro_make_promise__deps: ['$Emval'],
447448
_emval_coro_make_promise: (resolveHandlePtr, rejectHandlePtr) => {
448449
return Emval.toHandle(new Promise((resolve, reject) => {
449-
const rejectWithCurrentException = () => {
450-
try {
451-
// Use __cxa_rethrow which already has mechanism for generating
452-
// user-friendly error message and stacktrace from C++ exception
453-
// if EXCEPTION_STACK_TRACES is enabled and numeric exception
454-
// with metadata optimised out otherwise.
455-
___cxa_rethrow();
456-
} catch (e) {
457-
// But catch it so that it rejects the promise instead of throwing
458-
// in an unpredictable place during async execution.
459-
reject(e);
460-
}
461-
};
462-
463450
{{{ makeSetValue('resolveHandlePtr', '0', 'Emval.toHandle(resolve)', '*') }}};
464-
{{{ makeSetValue('rejectHandlePtr', '0', 'Emval.toHandle(rejectWithCurrentException)', '*') }}};
451+
{{{ makeSetValue('rejectHandlePtr', '0', 'Emval.toHandle(reject)', '*') }}};
465452
}));
466453
},
454+
455+
_emval_from_current_cxa_exception__deps: ['$Emval', '__cxa_rethrow'],
456+
_emval_from_current_cxa_exception: () => {
457+
try {
458+
// Use __cxa_rethrow which already has mechanism for generating
459+
// user-friendly error message and stacktrace from C++ exception
460+
// if EXCEPTION_STACK_TRACES is enabled and numeric exception
461+
// with metadata optimised out otherwise.
462+
___cxa_rethrow();
463+
} catch (e) {
464+
return Emval.toHandle(e);
465+
}
466+
},
467467
};
468468

469469
addToLibrary(LibraryEmVal);

src/lib/libsigs.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,7 @@ sigs = {
345345
_emval_decref__sig: 'vp',
346346
_emval_delete__sig: 'ipp',
347347
_emval_equals__sig: 'ipp',
348+
_emval_from_current_cxa_exception__sig: 'p',
348349
_emval_get_global__sig: 'pp',
349350
_emval_get_method_caller__sig: 'pipi',
350351
_emval_get_module_property__sig: 'pp',

system/include/emscripten/val.h

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@
2121
#include <pthread.h>
2222
#if __cplusplus >= 202002L
2323
#include <coroutine>
24+
#include <exception>
2425
#include <variant>
2526
#endif
2627

27-
2828
namespace emscripten {
2929

3030
class val;
@@ -118,6 +118,7 @@ EM_VAL _emval_iter_next(EM_VAL iterator);
118118

119119
#if __cplusplus >= 202002L
120120
void _emval_coro_suspend(EM_VAL promise, void* coro_ptr);
121+
EM_VAL _emval_from_current_cxa_exception();
121122
EM_VAL _emval_coro_make_promise(EM_VAL *resolve, EM_VAL *reject);
122123
#endif
123124

@@ -729,7 +730,8 @@ class val::awaiter {
729730
bool await_ready() { return false; }
730731

731732
// On suspend, store the coroutine handle and invoke a helper that will do
732-
// a rough equivalent of `promise.then(value => this.resume_with(value))`.
733+
// a rough equivalent of
734+
// `promise.then(value => this.resume_with(value)).catch(error => this.reject_with(error))`.
733735
void await_suspend(std::coroutine_handle<val::promise_type> handle) {
734736
internal::_emval_coro_suspend(std::get<STATE_PROMISE>(state).as_handle(), this);
735737
state.emplace<STATE_CORO>(handle);
@@ -743,9 +745,16 @@ class val::awaiter {
743745
coro.resume();
744746
}
745747

748+
// When JS invokes `reject_with` with some error value, reject currently suspended
749+
// coroutine's promise with the error value and destroy coroutine frame, because
750+
// in this scenario coroutine never reaches final_suspend point to be destroyed automatically.
751+
void reject_with(val&& error);
752+
746753
// `await_resume` finalizes the awaiter and should return the result
747754
// of the `co_await ...` expression - in our case, the stored value.
748-
val await_resume() { return std::move(std::get<STATE_RESULT>(state)); }
755+
val await_resume() {
756+
return std::move(std::get<STATE_RESULT>(state));
757+
}
749758
};
750759

751760
inline val::awaiter val::operator co_await() const {
@@ -756,7 +765,7 @@ inline val::awaiter val::operator co_await() const {
756765
// that compiler uses to drive the coroutine itself
757766
// (`T::promise_type` is used for any coroutine with declared return type `T`).
758767
class val::promise_type {
759-
val promise, resolve, reject_with_current_exception;
768+
val promise, resolve, reject;
760769

761770
public:
762771
// Create a `new Promise` and store it alongside the `resolve` and `reject`
@@ -766,7 +775,7 @@ class val::promise_type {
766775
EM_VAL reject_handle;
767776
promise = val(internal::_emval_coro_make_promise(&resolve_handle, &reject_handle));
768777
resolve = val(resolve_handle);
769-
reject_with_current_exception = val(reject_handle);
778+
reject = val(reject_handle);
770779
}
771780

772781
// Return the stored promise as the actual return value of the coroutine.
@@ -779,7 +788,19 @@ class val::promise_type {
779788
// On an unhandled exception, reject the stored promise instead of throwing
780789
// it asynchronously where it can't be handled.
781790
void unhandled_exception() {
782-
reject_with_current_exception();
791+
try {
792+
std::rethrow_exception(std::current_exception());
793+
} catch (const val& error) {
794+
reject(error);
795+
} catch (...) {
796+
val error = val(internal::_emval_from_current_cxa_exception());
797+
reject(error);
798+
}
799+
}
800+
801+
// Reject the stored promise due to rejection deeper in the call chain
802+
void reject_with(val&& error) {
803+
reject(std::move(error));
783804
}
784805

785806
// Resolve the stored promise on `co_return value`.
@@ -788,6 +809,14 @@ class val::promise_type {
788809
resolve(std::forward<T>(value));
789810
}
790811
};
812+
813+
inline void val::awaiter::reject_with(val&& error) {
814+
auto coro = std::move(std::get<STATE_CORO>(state));
815+
auto& promise = coro.promise();
816+
promise.reject_with(std::move(error));
817+
coro.destroy();
818+
}
819+
791820
#endif
792821

793822
// Declare a custom type that can be used in conjunction with

system/lib/embind/bind.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ void _emval_coro_resume(val::awaiter* awaiter, EM_VAL result) {
6767
awaiter->resume_with(val::take_ownership(result));
6868
}
6969

70+
void _emval_coro_reject(val::awaiter* awaiter, EM_VAL error) {
71+
awaiter->reject_with(val::take_ownership(error));
72+
}
73+
7074
}
7175

7276
namespace {

test/embind/test_val_coro.cpp

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,24 @@ EM_JS(EM_VAL, promise_sleep_impl, (int ms, int result), {
1616
return handle;
1717
});
1818

19+
EM_JS(EM_VAL, promise_fail_impl, (), {
20+
let promise = new Promise((_, reject) => setTimeout(reject, 1, new Error("bang from JS promise!")));
21+
let handle = Emval.toHandle(promise);
22+
// FIXME. See https://github.com/emscripten-core/emscripten/issues/16975.
23+
#if __wasm64__
24+
handle = BigInt(handle);
25+
#endif
26+
return handle;
27+
});
28+
1929
val promise_sleep(int ms, int result = 0) {
2030
return val::take_ownership(promise_sleep_impl(ms, result));
2131
}
2232

33+
val promise_fail() {
34+
return val::take_ownership(promise_fail_impl());
35+
}
36+
2337
// Test that we can subclass and make custom awaitable types.
2438
template <typename T>
2539
class typed_promise: public val {
@@ -37,7 +51,13 @@ class typed_promise: public val {
3751
}
3852
};
3953

54+
template <size_t N>
4055
val asyncCoro() {
56+
co_return co_await asyncCoro<N - 1>();
57+
}
58+
59+
template <>
60+
val asyncCoro<0>() {
4161
// check that just sleeping works
4262
co_await promise_sleep(1);
4363
// check that sleeping and receiving value works
@@ -50,12 +70,32 @@ val asyncCoro() {
5070
co_return 34;
5171
}
5272

73+
template <size_t N>
5374
val throwingCoro() {
75+
co_await throwingCoro<N - 1>();
76+
co_return 56;
77+
}
78+
79+
template <>
80+
val throwingCoro<0>() {
5481
throw std::runtime_error("bang from throwingCoro!");
5582
co_return 56;
5683
}
5784

85+
template <size_t N>
86+
val failingPromise() {
87+
co_await failingPromise<N - 1>();
88+
co_return 65;
89+
}
90+
91+
template <>
92+
val failingPromise<0>() {
93+
co_await promise_fail();
94+
co_return 65;
95+
}
96+
5897
EMSCRIPTEN_BINDINGS(test_val_coro) {
59-
function("asyncCoro", asyncCoro);
60-
function("throwingCoro", throwingCoro);
98+
function("asyncCoro", asyncCoro<3>);
99+
function("throwingCoro", throwingCoro<3>);
100+
function("failingPromise", failingPromise<3>);
61101
}

test/test_core.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7585,7 +7585,7 @@ def test_embind_val_coro(self):
75857585
self.emcc_args += ['-std=c++20', '--bind', '--pre-js=pre.js', '-sINCOMING_MODULE_JS_API=[onRuntimeInitialized]', '--no-entry']
75867586
self.do_runf('embind/test_val_coro.cpp', '34\n')
75877587

7588-
def test_embind_val_coro_caught(self):
7588+
def test_embind_val_coro_propogate_cpp_exception(self):
75897589
self.set_setting('EXCEPTION_STACK_TRACES')
75907590
create_file('pre.js', r'''Module.onRuntimeInitialized = () => {
75917591
Module.throwingCoro().then(
@@ -7596,6 +7596,17 @@ def test_embind_val_coro_caught(self):
75967596
self.emcc_args += ['-std=c++20', '--bind', '--pre-js=pre.js', '-fexceptions', '-sINCOMING_MODULE_JS_API=[onRuntimeInitialized]', '--no-entry']
75977597
self.do_runf('embind/test_val_coro.cpp', 'rejected with: std::runtime_error: bang from throwingCoro!\n')
75987598

7599+
def test_embind_val_coro_propogate_js_error(self):
7600+
self.set_setting('EXCEPTION_STACK_TRACES')
7601+
create_file('post.js', r'''Module.onRuntimeInitialized = () => {
7602+
Module.failingPromise().then(
7603+
console.log,
7604+
err => console.error(`rejected with: ${err.message}`)
7605+
);
7606+
}''')
7607+
self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js', '-fexceptions']
7608+
self.do_runf('embind/test_val_coro.cpp', 'rejected with: bang from JS promise!\n')
7609+
75997610
def test_embind_dynamic_initialization(self):
76007611
self.emcc_args += ['-lembind']
76017612
self.do_run_in_out_file_test('embind/test_dynamic_initialization.cpp')

tools/emscripten.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,6 +1164,7 @@ def create_pointer_conversion_wrappers(metadata):
11641164
'emscripten_proxy_finish': '_p',
11651165
'emscripten_proxy_execute_queue': '_p',
11661166
'_emval_coro_resume': '_pp',
1167+
'_emval_coro_reject': '_pp',
11671168
'emscripten_main_runtime_thread_id': 'p',
11681169
'_emscripten_set_offscreencanvas_size_on_thread': '_pp__',
11691170
'fileno': '_p',

tools/system_libs.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1932,7 +1932,7 @@ class libwebgpu_cpp(MTLibrary):
19321932
src_files = ['webgpu_cpp.cpp']
19331933

19341934

1935-
class libembind(Library):
1935+
class libembind(MTLibrary):
19361936
name = 'libembind'
19371937
never_force = True
19381938

0 commit comments

Comments
 (0)