Skip to content

Commit

Permalink
Build emhermesc.js with NODEJS_CATCH_EXIT=0 and NODEJS_CATCH_REJECTIO…
Browse files Browse the repository at this point in the history
…N=0 (facebook#698)

Summary:
Pull Request resolved: facebook#698

Currently, initialising `emhermesc.js` (eg by requiring `metro-hermes-compiler`, or just `metro`) sets an uncaught exception handler on the global NodeJS host process:

```
process.on(
  'uncaughtException',
  function (ex) {if (!(ex instanceof ExitStatus)) {throw ex;}},
);
```

And similarly, an `unhandledRejection` handler
```
process["on"]("unhandledRejection", function (reason) {
  throw reason;
});
```

Because these are persistent, global handlers, they affect the behaviour of the host application or other libraries at any time after `emhermesc.js` is initialised - even if it's never used.

As far as NodeJS is concerned, `uncaughtException` listeners should never throw. Node prints the stack trace starting with the entirety of the emhermesc source, and then exits with [code 7 (Internal exception handler runtime failure)](https://nodejs.org/api/process.html#exit-codes). In cases where this output is truncated (eg, CI), the cause of the original exception is lost.

The `unhandledRejection` listener is [apparently](emscripten-core/emscripten#7855 (comment)) to avoid "awkward" output in old (<=10) Node versions. It [serves no purpose at all since Node 15](https://developer.ibm.com/blogs/nodejs-15-release-blog/), but still messes up stack traces when the `throw` comes from a huge single line of minified code. It's particularly unnecessary in `emhermesc`'s case since I don't think we expose any async API.

The injection of this code is gated in the emscripten build step behind `#ifdef NODEJS_CATCH_EXIT` and `#ifdef NODEJS_CATCH_REJECTION` respectively.

Context from emscripten
 - [Handler addition code in `shell.js`](https://github.com/emscripten-core/emscripten/blob/main/src/shell.js#L217-L233)
 - [Issue: Code generated by emscripten is catching all uncaught exceptions](emscripten-core/emscripten#5957)
 - [Issue: Why catch unhandled rejections?](emscripten-core/emscripten#7855)
 - [Docs for the flags](https://github.com/emscripten-core/emscripten/blob/main/src/settings.js#L704-L722)

Reviewed By: neildhar

Differential Revision: D34790221

fbshipit-source-id: 19782de6af0d4e26a91746443abb528a239932a0
  • Loading branch information
robhogan authored and facebook-github-bot committed Mar 16, 2022
1 parent aed192a commit 4cfcb68
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions tools/emhermesc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,6 @@ if (EMSCRIPTEN)
target_link_options(emhermesc PRIVATE "SHELL: -s EXPORT_NAME=createHermesc")
target_link_options(emhermesc PRIVATE
"SHELL: -s BINARYEN_ASYNC_COMPILATION=0 -s SINGLE_FILE=1 -s WASM_ASYNC_COMPILATION=0")
target_link_options(emhermesc PRIVATE
"SHELL: -s NODEJS_CATCH_EXIT=0 -s NODEJS_CATCH_REJECTION=0")
endif ()

0 comments on commit 4cfcb68

Please sign in to comment.