Parse revert reason strings if present #106
Conversation
There was a problem hiding this comment.
This looks good to me! I'll let @benjamincburns chime in on this one before merging
|
@seesemichaelj Ping on this. |
|
@fabioberger Sorry, @benjamincburns was in India for a training event for the last ~6 days and is back today. Hasn't fallen off our radar! |
lib/utils/runtimeerror.js
Outdated
| @@ -52,6 +52,7 @@ RuntimeError.prototype.combine = function(transactions, vm_output) { | |||
| this.results[hash] = { | |||
| error: result.vm.exceptionError.error || result.vm.exceptionError, | |||
| program_counter: result.vm.runState.programCounter, | |||
| return: to.hex(result.vm.return), | |||
| reason: reason | |||
There was a problem hiding this comment.
@dekz can you please add a test for this?
Also be aware that this only provides a benefit when options.vmErrorsOnRPCResponse is true. This is the default today.
However as soon as geth/parity get their act together on how to handle reason string reporting for transactions, we'll probably adopt that mechanism and make options.vmErrorsOnRPCResponse default to false.
Why? Imagine writing a large dapp thinking that you'll get an RPC error back when a transaction fails, then discovering the whole eth_getTransactionReceipt/status field flow once you get to a testnet, or worse, mainnet. Your app won't fail when it should, and things will just behave wonky. Fortunately this is less of a problem with web3.js 1.0 and truffle-contract, as both check the receipt for you, but I believe there are a number of devs out there still using old versions of web3.js without truffle-contract.
dekz
force-pushed
the
feature/revert-strings
branch
from
415e3c5 to
b59e623
Compare
|
@benjamincburns added test and rebased off of develop. |
dekz
force-pushed
the
feature/revert-strings
branch
2 times, most recently
from
f6ac71f to
884f5a5
Compare
dekz
force-pushed
the
feature/revert-strings
branch
from
884f5a5 to
a04585c
Compare
|
@seesemichaelj will merge this one manually to take care of the conflicts. |
Parse the revert strings now that Solidity support this in 0.4.22.
ethereum/solidity#3364