Skip to content

Remove the workaround for greedy exception catcher#569

Closed
cameel wants to merge 1 commit intomasterfrom
remove-uncaught-exception-listener-workaround
Closed

Remove the workaround for greedy exception catcher#569
cameel wants to merge 1 commit intomasterfrom
remove-uncaught-exception-listener-workaround

Conversation

@cameel
Copy link
Collaborator

@cameel cameel commented Dec 3, 2021

This PR removes the old workaround that was added in #34 to solve chriseth/browser-solidity#167.

The issue was solved properly by adding NODEJS_CATCH_EXIT=0 flag to our emscripten build in 0.4.20 (argotorg/solidity#3477). According to #174 (comment) the workaround is no longer needed.

…ption

- This has been fixed in the emscripten binaries
@cameel cameel requested review from axic and ekpyron December 3, 2021 16:36
@cameel cameel self-assigned this Dec 3, 2021
@cameel
Copy link
Collaborator Author

cameel commented Dec 3, 2021

@axic #174 (comment) says we can safely remove it but won't that break on pre-0.4.20 binaries?

Well, wasm rebuilds were done long after 0.4.20 so I think they do not have this problem and also the change only affects the solcjs tool, not whole library so I created the PR anyway.

@axic
Copy link
Contributor

axic commented Jan 19, 2022

Actually we probably can't remove it if we want to keep supporting old compiler versions with this CLI tool. Those binaries still need it 😢

@cameel
Copy link
Collaborator Author

cameel commented Jan 19, 2022

Oh well, I was afraid that this would be the end result. Yeah, I think we want to support them, I guess there's no magic way to do it without this workaround :)

I'm closing it then. We can always revive if we decide to merge it after all.

@cameel cameel closed this Jan 19, 2022
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

Successfully merging this pull request may close these issues.

2 participants

Comments