From b81bba552c1e6bf87c0ba7a64df3cd67eff1e2f5 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Mon, 16 Dec 2024 15:04:53 -0800 Subject: [PATCH] Don't inject assertions onto the ready promise (#23176) This is a debug feature that means users who did not await the promise and tried to access stuff directly on it would see a nice error message. However with the advent of more usage of async/await, for example in \#23157 it is not always the ready promise that gets returned to the user but a language-generated promise from an `await` call. We could try to jump through more hoops to continue to support this debug feature, but I don't think added complexity in the toolchain or the generated code would be worth it. Split out from #23157. --- src/parseTools.mjs | 29 ----------------------------- src/shell.js | 3 --- src/shell_minimal.js | 3 --- test/test_other.py | 25 ------------------------- 4 files changed, 60 deletions(-) diff --git a/src/parseTools.mjs b/src/parseTools.mjs index fa6da8556e314..375b4bdae45c6 100644 --- a/src/parseTools.mjs +++ b/src/parseTools.mjs @@ -971,34 +971,6 @@ function to64(x) { return `BigInt(${x})`; } -// Add assertions to catch common errors when using the Promise object we -// return from MODULARIZE Module() invocations. -function addReadyPromiseAssertions() { - // Warn on someone doing - // - // var instance = Module(); - // ... - // instance._main(); - const properties = Array.from(EXPORTED_FUNCTIONS.values()); - // Also warn on onRuntimeInitialized which might be a common pattern with - // older MODULARIZE-using codebases. - properties.push('onRuntimeInitialized'); - const warningEnding = - ' on the Promise object, instead of the instance. Use .then() to get called back with the instance, see the MODULARIZE docs in src/settings.js'; - const res = JSON.stringify(properties); - return ( - res + - `.forEach((prop) => { - if (!Object.getOwnPropertyDescriptor(readyPromise, prop)) { - Object.defineProperty(readyPromise, prop, { - get: () => abort('You are getting ' + prop + '${warningEnding}'), - set: () => abort('You are setting ' + prop + '${warningEnding}'), - }); - } -});` - ); -} - function asyncIf(condition) { return condition ? 'async' : ''; } @@ -1118,7 +1090,6 @@ addToCompileTimeContext({ ENVIRONMENT_IS_WORKER_THREAD, addAtExit, addAtInit, - addReadyPromiseAssertions, asyncIf, awaitIf, buildStringArray, diff --git a/src/shell.js b/src/shell.js index b799095e8ebd1..36d9e9a4b042f 100644 --- a/src/shell.js +++ b/src/shell.js @@ -47,9 +47,6 @@ var readyPromise = new Promise((resolve, reject) => { readyPromiseResolve = resolve; readyPromiseReject = reject; }); -#if ASSERTIONS -{{{ addReadyPromiseAssertions() }}} -#endif #endif // Determine the runtime environment we are in. You can customize this by diff --git a/src/shell_minimal.js b/src/shell_minimal.js index 9e8b3039208f0..4ddf6b3b0303c 100644 --- a/src/shell_minimal.js +++ b/src/shell_minimal.js @@ -41,9 +41,6 @@ var readyPromise = new Promise((resolve, reject) => { readyPromiseResolve = resolve; readyPromiseReject = reject; }); -#if ASSERTIONS -{{{ addReadyPromiseAssertions() }}} -#endif #endif #if ENVIRONMENT_MAY_BE_NODE diff --git a/test/test_other.py b/test/test_other.py index ac84ed83aaa08..2648e1f1601f4 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -11962,31 +11962,6 @@ def test_assertions_on_outgoing_module_api_changes(self): ''' self.do_runf('src.cpp', expected, emcc_args=['-sASSERTIONS']) - def test_modularize_assertions_on_ready_promise(self): - # check that when assertions are on we give useful error messages for - # mistakenly thinking the Promise is an instance. I.e., once you could do - # Module()._main to get an instance and the main function, but after - # the breaking change in #10697 Module() now returns a promise, and to get - # the instance you must use .then() to get a callback with the instance. - create_file('test.js', r''' - try { - Module()._main; - } catch(e) { - console.log(e); - } - try { - Module().onRuntimeInitialized = 42; - } catch(e) { - console.log(e); - } - ''') - self.run_process([EMCC, test_file('hello_world.c'), '-sMODULARIZE', '-sASSERTIONS', '--extern-post-js', 'test.js']) - # A return code of 1 is from an uncaught exception not handled by - # the domain or the 'uncaughtException' event handler. - out = self.run_js('a.out.js', assert_returncode=1) - self.assertContained('You are getting _main on the Promise object, instead of the instance. Use .then() to get called back with the instance, see the MODULARIZE docs in src/settings.js', out) - self.assertContained('You are setting onRuntimeInitialized on the Promise object, instead of the instance. Use .then() to get called back with the instance, see the MODULARIZE docs in src/settings.js', out) - def test_modularize_assertions_on_reject_promise(self): # Check that there is an uncaught exception in modularize mode. # Once we added an `uncaughtException` handler to the global process