Skip to content

Commit

Permalink
Don't inject assertions onto the ready promise (emscripten-core#23176)
Browse files Browse the repository at this point in the history
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
\emscripten-core#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 emscripten-core#23157.
  • Loading branch information
sbc100 authored and hedwigz committed Dec 18, 2024
1 parent 010c5bf commit b81bba5
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 60 deletions.
29 changes: 0 additions & 29 deletions src/parseTools.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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' : '';
}
Expand Down Expand Up @@ -1118,7 +1090,6 @@ addToCompileTimeContext({
ENVIRONMENT_IS_WORKER_THREAD,
addAtExit,
addAtInit,
addReadyPromiseAssertions,
asyncIf,
awaitIf,
buildStringArray,
Expand Down
3 changes: 0 additions & 3 deletions src/shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions src/shell_minimal.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ var readyPromise = new Promise((resolve, reject) => {
readyPromiseResolve = resolve;
readyPromiseReject = reject;
});
#if ASSERTIONS
{{{ addReadyPromiseAssertions() }}}
#endif
#endif

#if ENVIRONMENT_MAY_BE_NODE
Expand Down
25 changes: 0 additions & 25 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b81bba5

Please sign in to comment.