Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code generated by emscripten is catching all uncaught exceptions #5957

Closed
hugo-dc opened this issue Dec 20, 2017 · 2 comments
Closed

Code generated by emscripten is catching all uncaught exceptions #5957

hugo-dc opened this issue Dec 20, 2017 · 2 comments

Comments

@hugo-dc
Copy link

hugo-dc commented Dec 20, 2017

I have a JS library which is generated from rust code using emscripten, when an exception is thrown the trace points to the emscripten generated code, even if the error is not in the library itself.

I have created this small repository to replicate the issue:
https://github.com/hugo-dc/wasm-demo

the index.js file just loads the generated js code, and then throw an exception, when the exception is thrown it shows the following output pointing out to the generated JS code:

/Users/hugo/workspace/wasm-demo/site/site.js:1
(function (exports, require, module, __filename, __dirname) { var Module=typeof Module!=="undefined"?Module:{};var moduleOverrides={};var key;for(key in Module){if(Module.hasOwnProperty(key)){moduleOverrides[key]=Module[key]}}var ENVIRONMENT_IS_WEB=false;var ENVIRONMENT_IS_WORKER=false;var ENVIRONMENT_IS_NODE=false;var ENVIRONMENT_IS_SHELL=false;if(Module["ENVIRONMENT"]){if(Module["ENVIRONMENT"]==="WEB"){ENVIRONMENT_IS_WEB=true}else if(Module["ENVIRONMENT"]==="WORKER"){ENVIRONMENT_IS_WORKER=true}else if(Module["ENVIRONMENT"]==="NODE"){ENVIRONMENT_IS_NODE=true}else if(Module["ENVIRONMENT"]==="SHELL"){ENVIRONMENT_IS_SHELL=true}else{throw new Error("The provided Module['ENVIRONMENT'] value is not valid. It must be one of: WEB|WORKER|NODE|SHELL.")}}else{ENVIRONMENT_IS_WEB=typeof window==="object";ENVIRONMENT_IS_WORKER=typeof importScripts==="function";ENVIRONMENT_IS_NODE=typeof process==="object"&&typeof require==="function"&&!ENVIRONMENT_IS_WEB&&!ENVIRONMENT_IS_WO
MyException

I see the generated JS code contains the following:

process["on"]("uncaughtException",(function(ex){if(!(ex instanceof ExitStatus)){throw ex}}))

If that code is removed, then I get different output, which is what I would expect:

/Users/hugo/workspace/wasm-demo/index.js:4
throw "MyException"
^
MyException

So, is there a way to get rid of that part of the code which is currently catching all exceptions? is there a way to tell emscripten to not generate it?.

@kripken
Copy link
Member

kripken commented Dec 20, 2017

Looking at the place where we add that code, it is gated behind NODEJS_CATCH_EXIT, which looks like what you want,

var NODEJS_CATCH_EXIT = 1; // By default we handle exit() in node, by catching the Exit exception. However,
                           // this means we catch all process exceptions. If you disable this, then we no
                           // longer do that, and exceptions work normally, which can be useful for libraries
                           // or programs that don't need exit() to work.

Does that work?

@hugo-dc
Copy link
Author

hugo-dc commented Dec 21, 2017

That works great!, Thank you very much!

@hugo-dc hugo-dc closed this as completed Dec 21, 2017
robhogan added a commit to robhogan/hermes that referenced this issue Mar 13, 2022
Summary:
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: e8b355e07022f4c377d918aa7dfdefe3d5272639
robhogan added a commit to robhogan/hermes that referenced this issue Mar 16, 2022
…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
facebook-github-bot pushed a commit to facebook/hermes that referenced this issue Mar 16, 2022
…N=0 (#698)

Summary:
Pull Request resolved: #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: 645e514db4107127b849e3a0baae088bd1ed9b50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants