Skip to content

Commit d74f269

Browse files
authored
Remove NODEJS_CATCH_EXIT setting (emscripten-core#26326)
This setting has not been enabled by default since emscripten-core#22257. It is not used anywhere internally in emscripten test suite anymore. Its is problematic because it installs process-wide global exit handler (so it doesn't work well with other modules or other code). It should not be needed in general since most code paths that enter the module are wrapped in try/catch that handled ExitStatus. Its trivial to replace with a `--pre-js` or some other external JS snippet.
1 parent 9c0f34c commit d74f269

File tree

7 files changed

+6
-80
lines changed

7 files changed

+6
-80
lines changed

ChangeLog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ See docs/process.md for more on how version tagging works.
2020

2121
5.0.2 (in development)
2222
----------------------
23+
- The `NODEJS_CATCH_EXIT` setting was removed. This setting was disabled by
24+
default in #22257, and is no longer used by emscripten itself. It is also
25+
problematic as it injects a global process.on handler. It is easy to replace
26+
with a simple `--pre-js` file for those that require it. (#26326)
2327
- Several low level emscripten APIs that return success/failure now return the
2428
C `bool` type rather than `int`. For example `emscripten_proxy_sync` and
2529
`emscripten_is_main_runtime_thread`. (#26316)

site/source/docs/tools_reference/settings_reference.rst

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,22 +1168,6 @@ https://github.com/WebAssembly/exception-handling/blob/main/proposals/exception-
11681168

11691169
Default value: true
11701170

1171-
.. _nodejs_catch_exit:
1172-
1173-
NODEJS_CATCH_EXIT
1174-
=================
1175-
1176-
Emscripten throws an ExitStatus exception to unwind when exit() is called.
1177-
Without this setting enabled this can show up as a top level unhandled
1178-
exception.
1179-
1180-
With this setting enabled a global uncaughtException handler is used to
1181-
catch and handle ExitStatus exceptions. However, this means all other
1182-
uncaught exceptions are also caught and re-thrown, which is not always
1183-
desirable.
1184-
1185-
Default value: false
1186-
11871171
.. _nodejs_catch_rejection:
11881172

11891173
NODEJS_CATCH_REJECTION
@@ -3563,3 +3547,4 @@ for backwards compatibility with older versions:
35633547
- ``ASYNCIFY_LAZY_LOAD_CODE``: No longer supported (Valid values: [0])
35643548
- ``USE_WEBGPU``: No longer supported; replaced by --use-port=emdawnwebgpu, which implements a newer (but incompatible) version of webgpu.h - see tools/ports/emdawnwebgpu.py (Valid values: [0])
35653549
- ``PROXY_TO_WORKER``: No longer supported (Valid values: [0])
3550+
- ``NODEJS_CATCH_EXIT``: No longer supported (Valid values: [0])

src/settings.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -789,18 +789,6 @@ var EXCEPTION_STACK_TRACES = false;
789789
// [compile+link]
790790
var WASM_LEGACY_EXCEPTIONS = true;
791791

792-
// Emscripten throws an ExitStatus exception to unwind when exit() is called.
793-
// Without this setting enabled this can show up as a top level unhandled
794-
// exception.
795-
//
796-
// With this setting enabled a global uncaughtException handler is used to
797-
// catch and handle ExitStatus exceptions. However, this means all other
798-
// uncaught exceptions are also caught and re-thrown, which is not always
799-
// desirable.
800-
//
801-
// [link]
802-
var NODEJS_CATCH_EXIT = false;
803-
804792
// Catch unhandled rejections in node. This only affects versions of node older
805793
// than 15. Without this, old version node will print a warning, but exit
806794
// with a zero return code. With this setting enabled, we handle any unhandled

src/shell.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -225,18 +225,6 @@ if (ENVIRONMENT_IS_NODE) {
225225
}
226226
#endif
227227

228-
#if NODEJS_CATCH_EXIT
229-
process.on('uncaughtException', (ex) => {
230-
// suppress ExitStatus exceptions from showing an error
231-
#if RUNTIME_DEBUG
232-
dbg(`node: uncaughtException: ${ex}`)
233-
#endif
234-
if (ex !== 'unwind' && !(ex instanceof ExitStatus) && !(ex.context instanceof ExitStatus)) {
235-
throw ex;
236-
}
237-
});
238-
#endif
239-
240228
#if NODEJS_CATCH_REJECTION
241229
// Without this older versions of node (< v15) will log unhandled rejections
242230
// but return 0, which is not normally the desired behaviour. This is

test/test_other.py

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,12 +1220,6 @@ def test_failure_modularize_and_catch_rejection(self, compiler):
12201220
self.expect_fail([compiler, test_file('hello_world.c'), '-sMODULARIZE', '-sNODEJS_CATCH_REJECTION', '-o', 'out.js'])
12211221
self.assertFalse(os.path.exists('out.js'))
12221222

1223-
@with_both_compilers
1224-
def test_failure_modularize_and_catch_exit(self, compiler):
1225-
# Test that if sMODULARIZE and sNODEJS_CATCH_EXIT are both enabled, then emcc shouldn't succeed, and shouldn't produce an output file.
1226-
self.expect_fail([compiler, test_file('hello_world.c'), '-sMODULARIZE', '-sNODEJS_CATCH_EXIT', '-o', 'out.js'])
1227-
self.assertFalse(os.path.exists('out.js'))
1228-
12291223
def test_use_cxx(self):
12301224
create_file('empty_file', ' ')
12311225
dash_xc = self.run_process([EMCC, '-v', '-xc', 'empty_file'], stderr=PIPE).stderr
@@ -4206,37 +4200,6 @@ def test_module_exports_with_closure(self):
42064200
# in order to check that the exports are indeed functioning correctly.
42074201
self.assertContained('bufferTest finished', self.run_js('main.js'))
42084202

4209-
@requires_node
4210-
def test_node_catch_exit(self):
4211-
# Test that in top level JS exceptions are caught and rethrown when NODEJS_EXIT_CATCH=1
4212-
# is set but not by default.
4213-
create_file('count.c', '''
4214-
#include <string.h>
4215-
int count(const char *str) {
4216-
return (int)strlen(str);
4217-
}
4218-
''')
4219-
4220-
create_file('index.js', '''
4221-
const count = require('./count.js');
4222-
4223-
console.log(xxx); //< here is the ReferenceError
4224-
''')
4225-
4226-
reference_error_text = 'console.log(xxx); //< here is the ReferenceError'
4227-
4228-
self.run_process([EMCC, 'count.c', '-o', 'count.js', '-sNODEJS_CATCH_EXIT=1'])
4229-
4230-
# Check that the ReferenceError is caught and rethrown and thus the original error line is masked
4231-
self.assertNotContained(reference_error_text,
4232-
self.run_js('index.js', assert_returncode=NON_ZERO))
4233-
4234-
self.run_process([EMCC, 'count.c', '-o', 'count.js'])
4235-
4236-
# Check that the ReferenceError is not caught, so we see the error properly
4237-
self.assertContained(reference_error_text,
4238-
self.run_js('index.js', assert_returncode=NON_ZERO))
4239-
42404203
@requires_node
42414204
def test_exported_runtime_methods(self):
42424205
# Test with node.js that the EXPORTED_RUNTIME_METHODS setting is

tools/link.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1859,8 +1859,6 @@ def get_full_import_name(name):
18591859
diagnostics.warning('unused-command-line-argument', 'NODERAWFS ignored since `node` not in `ENVIRONMENT`')
18601860
if settings.NODE_CODE_CACHING:
18611861
diagnostics.warning('unused-command-line-argument', 'NODE_CODE_CACHING ignored since `node` not in `ENVIRONMENT`')
1862-
if settings.NODEJS_CATCH_EXIT:
1863-
diagnostics.warning('unused-command-line-argument', 'NODEJS_CATCH_EXIT ignored since `node` not in `ENVIRONMENT`')
18641862
if settings.NODEJS_CATCH_REJECTION and 'NODEJS_CATCH_REJECTION' in user_settings:
18651863
diagnostics.warning('unused-command-line-argument', 'NODEJS_CATCH_REJECTION ignored since `node` not in `ENVIRONMENT`')
18661864

tools/settings.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@
146146
('SEPARATE_DWARF', 'WASM2JS', 'as there is no wasm file'),
147147
('GL_SUPPORT_AUTOMATIC_ENABLE_EXTENSIONS', 'NO_GL_SUPPORT_SIMPLE_ENABLE_EXTENSIONS', None),
148148
('MODULARIZE', 'NODEJS_CATCH_REJECTION', None),
149-
('MODULARIZE', 'NODEJS_CATCH_EXIT', None),
150149
('LEGACY_VM_SUPPORT', 'MEMORY64', None),
151150
('CROSS_ORIGIN', 'NO_DYNAMIC_EXECUTION', None),
152151
('CROSS_ORIGIN', 'NO_PTHREADS', None),
@@ -252,6 +251,7 @@
252251
['ASYNCIFY_LAZY_LOAD_CODE', [0], 'No longer supported'],
253252
['USE_WEBGPU', [0], 'No longer supported; replaced by --use-port=emdawnwebgpu, which implements a newer (but incompatible) version of webgpu.h - see tools/ports/emdawnwebgpu.py'],
254253
['PROXY_TO_WORKER', [0], 'No longer supported'],
254+
['NODEJS_CATCH_EXIT', [0], 'No longer supported'],
255255
]
256256

257257
user_settings: dict[str, str] = {}

0 commit comments

Comments
 (0)