Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix uncaught exceptions #10

Closed
wants to merge 3 commits into from

Conversation

hugo-dc
Copy link
Contributor

@hugo-dc hugo-dc commented Dec 15, 2017

This change removes the following code from the generated js code:

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

That code was catching all exceptions and for that reason it was showed when whenever other exception occurred in ethereumjs-vm.

Fixes #9

@axic
Copy link
Member

axic commented Dec 15, 2017

Can you make sure those two sed commands actually have at least 1 match? In a future emscripten version the output might be different and they may not replace anything.

In solc-js we had the same issue and ended up not mangling the emscripten output rather having those exceptions listener removed & re-added in a wrapper.

@hugo-dc hugo-dc changed the title Fix uncaught exceptions [WIP] Fix uncaught exceptions Dec 21, 2017
@hugo-dc
Copy link
Contributor Author

hugo-dc commented Dec 21, 2017

@axic there is a emcc flag (NODEJS_CATCH_EXIT) which avoids generating the code which is currently catching all exceptions. I submitted a new PR #11 including this change and the new generated code.

Regarding the existing sed command in makefile I created a new issue for that and will submit a new PR to validate at least one match.

@hugo-dc hugo-dc changed the title [WIP] Fix uncaught exceptions Fix uncaught exceptions Dec 21, 2017
@hugo-dc
Copy link
Contributor Author

hugo-dc commented Dec 28, 2017

Created PR #14 to add the sed validation to have at least one match. This PR is no longer necessary.

@hugo-dc hugo-dc closed this Dec 28, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants