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

receipt is missing on error event #3129

Merged
merged 10 commits into from
Oct 16, 2019
Merged

receipt is missing on error event #3129

merged 10 commits into from
Oct 16, 2019

Conversation

nivida
Copy link
Contributor

@nivida nivida commented Oct 15, 2019

Fixes #1859 with adding of the receipt to the returned values of the error event listener. The receipt will get returned if not enough gas was provided or the EVM is reverting the transaction on other contract-specific reasons.

@nivida nivida added Bug Addressing a bug In Progress Currently being worked on 1.x 1.0 related issues labels Oct 15, 2019
@coveralls
Copy link

coveralls commented Oct 15, 2019

Coverage Status

Coverage remained the same at 84.651% when pulling 9a03dfa on issue/1859 into 571f374 on 1.x.

@nivida
Copy link
Contributor Author

nivida commented Oct 15, 2019

@cgewecke We have to add the error cases in the eth.sendTransaction and contract test cases. But I think this should be tracked with a separate issue to include a clean up of these test cases as well. They are written in a complex way that wrongly running test cases are hard to find. I like generic test cases but I think we could do this in a way better way as it is currently. (see todo)

@nivida nivida removed the In Progress Currently being worked on label Oct 15, 2019
docs/web3-eth.rst Outdated Show resolved Hide resolved
nivida and others added 2 commits October 16, 2019 01:41
Co-Authored-By: cgewecke <christophergewecke@gmail.com>
Co-Authored-By: cgewecke <christophergewecke@gmail.com>
Copy link
Collaborator

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Perhaps we should open an issue considering the suggestion in this comment, about restoring a generic emitter for any receipt, regardless of status.

docs/web3-eth-contract.rst Outdated Show resolved Hide resolved
Co-Authored-By: cgewecke <christophergewecke@gmail.com>
nachomazzara pushed a commit to nachomazzara/web3.js that referenced this pull request Jun 4, 2020
* receipt added for error event listener on sending of a transaction

* packages updated in package-lock because of security warnings from npm

* passing of parameters to _fireError updated in Method object of the web3-core-method module

* CHANGELOG.md updated

* documentation for sendTransaction and contract.methods.myMethod.send() updated

* Update packages/web3-core-method/src/index.js

Co-Authored-By: cgewecke <christophergewecke@gmail.com>

* Update docs/web3-eth.rst

Co-Authored-By: cgewecke <christophergewecke@gmail.com>

* wrongly passed parameters for the _fireError function fixed

* Update docs/web3-eth-contract.rst

Co-Authored-By: cgewecke <christophergewecke@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Bug Addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants