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

Ammo needs Emcscripten flag NODEJS_CATCH_REJECTION #349

Open
NullSoldier opened this issue Jan 23, 2021 · 2 comments
Open

Ammo needs Emcscripten flag NODEJS_CATCH_REJECTION #349

NullSoldier opened this issue Jan 23, 2021 · 2 comments

Comments

@NullSoldier
Copy link

NullSoldier commented Jan 23, 2021

Ammo is not currently compiled with NODEJS_CATCH_REJECTION, which it needs to. emscripten-core/emscripten#9061

Without it, emscripten generates JS to inappropriately catch all errors and do heinous things with them. We can see it in the current checked in build https://github.com/kripken/ammo.js/blob/master/builds/ammo.wasm.js if you just search for "unhandledRejection" and "unhandledException".

I work on a cloud based game engine and server, and we use ammo. When there are errors, emscriptem dumps the entire source code to ammo.wasm.js to the logs / terminal, and crashes instead of hitting our error reporting tool. It's because it's minified to be on one line, so the stack trace just prints the entire source code of ammo wasm js. This is really absurd, because we have our own error handlers that are not being called at all.

My workaround is to delete it out of the generated build, or compile it myself with these flags. Anyone using ammo will get screwed by this emscriptem feature, though. We need to use the two flags to disable these handlers from being generated at all. There's 0 use case to want them.

See also emscripten-core/emscripten#7855

@ianpurvis
Copy link
Collaborator

Thanks for the heads-up... would you be able to make a PR that adds this to the cmake config? Should be around here: https://github.com/kripken/ammo.js/blob/master/CMakeLists.txt#L36-L51

@NullSoldier
Copy link
Author

I'll put up the PR soon if someone else doesn't beat me to it. I've been super busy lately but I'll definitely find time.

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