Skip to content
This repository has been archived by the owner on Feb 7, 2018. It is now read-only.

solc package greedy on exception handling, causes extra error information in stack traces in Node #167

Closed
tcoulter opened this issue May 5, 2016 · 9 comments

Comments

@tcoulter
Copy link

tcoulter commented May 5, 2016

After compiling with browser solidity in Node, all uncaught exceptions (i.e., bad code or application errors) will result in an error message similar to the following:

/Users/tim/Documents/workspace/Consensys/ether-pudding/node_modules/solc/bin/soljson-latest.js:1
(function (exports, require, module, __filename, __dirname) { var Module;if(!Module)Module=(typeof Module!=="undefined"?Module:null)||{};var moduleOverrides={};for(var key in Module){if(Module.hasOwnProperty(key)){moduleOverrides[key]=Module[key]}}var ENVIRONMENT_IS_WEB=typeof window==="object";var ENVIRONMENT_IS_WORKER=typeof importScripts==="function";var ENVIRONMENT_IS_NODE=typeof process==="object"&&typeof require==="function"&&!ENVIRONMENT_IS_WEB&&!ENVIRONMENT_IS_WORKER;var ENVIRONMENT_IS_SHELL=!ENVIRONMENT_IS_WEB&&!ENVIRONMENT_IS_NODE&&!ENVIRONMENT_IS_WORKER;if(ENVIRONMENT_IS_NODE){if(!Module["print"])Module["print"]=function print(x){process["stdout"].write(x+"\n")};if(!Module["printErr"])Module["printErr"]=function printErr(x){process["stderr"].write(x+"\n")};var nodeFS=require("fs");var nodePath=require("path");Module["read"]=function read(filename,binary){filename=nodePath["normalize"](filename);var re

TypeError: Cannot read property 'abi' of undefined
    at Function.Contract.setNetwork (/Users/tim/Documents/workspace/Consensys/ether-pudding/tmp-test-contract-11645-42321-5temmx/Example.sol.js:330:68)
    at Function.mutate (/Users/tim/Documents/workspace/Consensys/ether-pudding/tmp-test-contract-11645-42321-5temmx/Example.sol.js:174:10)
    at temporary.Contract (/Users/tim/Documents/workspace/Consensys/ether-pudding/tmp-test-contract-11645-42321-5temmx/Example.sol.js:184:21)
    at new temporary (/Users/tim/Documents/workspace/Consensys/ether-pudding/tmp-test-contract-11645-42321-5temmx/Example.sol.js:156:49)
    at intermediary (/Users/tim/Documents/workspace/Consensys/ether-pudding/tmp-test-contract-11645-42321-5temmx/Example.sol.js:234:18)
    <snip>

Ignore the actual error message ("TypeError: ...") -- it doesn't matter in this case. However, notice the stack trace is prefixed with a long line of extra code. This extra code obfuscates the error message and makes it harder to read the stack trace to determine what went wrong. On this github ticket, you can scroll to the right in the code box above to see how long this output is. Now imagine this output in an environment where the output wraps, like in a terminal window when running automated tests.

Fortunately there's a way to remove this extra code information from stack traces:

// Clean up after solidity. Note you must run this after calling solc.compile().
process.removeAllListeners("uncaughtException");

This works because it prevents the solc package from catching the error and rethrowing, which is the cause of the extra stack trace information above. This method is good in that it makes error messages that occur after compiling much cleaner, but it in itself is greedy. What if there are uncaughtException handlers that still need to exist as a function of my application? In most cases this isn't an issue, but it's easy to imagine cases where this becomes a problem.

To fix this, I think as part of the compile function the solc package should remove its own listener on process's uncaughtException event. Since the solc package has access to the handler that was added on the uncaughtException event, it should be able to remove that handler without affecting any other event handlers, and it should do so before the compile function returns.

Here's a quick script that exposes the issue:

var solc = require("solc");

solc.compile("contract E {}", 1);

// Call a function that doesn't exist.
process.explode();

Notice you get the following output:

/Users/tim/Documents/workspace/Consensys/ether-pudding/node_modules/solc/bin/soljson-latest.js:1
(function (exports, require, module, __filename, __dirname) { var Module;if(!Module)Module=(typeof Module!=="undefined"?Module:null)||{};var moduleOverrides={};for(var key in Module){if(Module.hasOwnProperty(key)){moduleOverrides[key]=Module[key]}}var ENVIRONMENT_IS_WEB=typeof window==="object";var ENVIRONMENT_IS_WORKER=typeof importScripts==="function";var ENVIRONMENT_IS_NODE=typeof process==="object"&&typeof require==="function"&&!ENVIRONMENT_IS_WEB&&!ENVIRONMENT_IS_WORKER;var ENVIRONMENT_IS_SHELL=!ENVIRONMENT_IS_WEB&&!ENVIRONMENT_IS_NODE&&!ENVIRONMENT_IS_WORKER;if(ENVIRONMENT_IS_NODE){if(!Module["print"])Module["print"]=function print(x){process["stdout"].write(x+"\n")};if(!Module["printErr"])Module["printErr"]=function printErr(x){process["stderr"].write(x+"\n")};var nodeFS=require("fs");var nodePath=require("path");Module["read"]=function read(filename,binary){filename=nodePath["normalize"](filename);var re

TypeError: process.explode is not a function
    at Object.<anonymous> (/Users/tim/Documents/workspace/Consensys/ether-pudding/test.js:6:9)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Function.Module.runMain (module.js:447:10)
    at startup (node.js:139:18)
    at node.js:999:3

To hit the point home once more, here's what it looks like in my terminal window:

screen shot 2016-05-05 at 8 10 54 am

Thanks!

@redsquirrel
Copy link

// Clean up after solidity. Note you must run this after calling solc.compile().

When I run process.removeAllListeners("uncaughtException") after the require but before solc.compile, the wall of error-text does no longer appears.

@tcoulter
Copy link
Author

tcoulter commented May 5, 2016

Interesting. I made an assumption - that compile was the culprit. In that case solc likely shouldn't rely on uncaughtException.

@axic
Copy link
Collaborator

axic commented May 10, 2016

Would be nice to solve this, I was always annoyed by that.

The reason for that code seems to be a bit more complex. The catch for uncaughtException is in the node.js specific part of Emscripten's preamble code.

All the relevant sources:

Also slightly relevant:

@axic
Copy link
Collaborator

axic commented May 26, 2016

@tcoulter @redsquirrel did you had a chance to check in Emscripten if there's a proper solution to this?

@redsquirrel
Copy link

@axic No, I'm still using removeAllListeners. Haven't taken the time to investigate more deeply.

@tcoulter
Copy link
Author

Same. I just added this to Truffle, removing the listener right after requiring solc:

var solc = require("solc");

// Clean up after solc.
var listeners = process.listeners("uncaughtException");
var solc_listener = listeners[listeners.length - 1];
process.removeListener("uncaughtException", solc_listener);

I keep a reference to the listener and add it back before compiling, and remove it after compiling, just in case it's needed.

@axic
Copy link
Collaborator

axic commented May 26, 2016

Would be nice to find a proper solution and have it included in https://github.com/ethereum/solc-js. The above seems very node specific and wouldn't possible work well in the browser 😃

@tcoulter
Copy link
Author

Agreed.

@axic
Copy link
Collaborator

axic commented Aug 1, 2016

This is tracked now in ethereum/solc-js#34.

@axic axic closed this as completed Aug 1, 2016
axic pushed a commit to axic/browser-solidity that referenced this issue Aug 30, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants