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

Contract.methods.myMethod.send receipt is missing on error event #1859

Closed
juhah opened this issue Aug 9, 2018 · 15 comments
Closed

Contract.methods.myMethod.send receipt is missing on error event #1859

juhah opened this issue Aug 9, 2018 · 15 comments
Assignees
Labels
1.x 1.0 related issues Bug Addressing a bug
Projects

Comments

@juhah
Copy link

juhah commented Aug 9, 2018

From docs (
http://web3js.readthedocs.io/en/1.0/web3-eth-contract.html#methods-mymethod-send):

"error" returns Error: is fired if an error occurs during sending. If a out of gas error, the second parameter is the receipt.

However, there is no second parameter and the receipt is a string and part of the error message:

"Transaction has been reverted by the EVM:
{
  "blockHash": "0x90310734419309a5b23d40bc0cccf1a64c4863ecd06f1ed0812e1a32f1552056",
  "blockNumber": 2782305,
  "contractAddress": null,
  "cumulativeGasUsed": 1255548,
  "from": "0x13581255ee2d20e780b0cd3d07fac018241b5e03",
  "gasUsed": 40260,
  "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
  "status": false,
  "to": "0x0af64558670a3b761b57e465cb80b62254b39619",
  "transactionHash": "0x84d494da9e224e6c23c7c30840f4c022a4b01ca2702b902a4a42353af5899cb6",
  "transactionIndex": 8,
  "events": {}
}"

The receipt event is never fired for this transaction.

Used version: 1.0.0-beta.35

@juhah juhah changed the title Contract.methods.myMethod.send receipt is missing on error Contract.methods.myMethod.send receipt is missing on error event Aug 9, 2018
@nivida nivida self-assigned this Aug 10, 2018
@nivida nivida added the Documentation Relates to project wiki or documentation label Aug 10, 2018
@nicolashardy
Copy link

Same error.

@nivida nivida added this to To do in 1.0 Nov 28, 2018
@nivida
Copy link
Contributor

nivida commented Apr 17, 2019

The PromiEvent object is now returning the receipt in the error listener:

contract.methods.myMethod().send().on('error',  (error, receipt, confirmations) => { ... });

web3.js-1.0.0.beta-52

@nivida nivida added the 1.x 1.0 related issues label Jun 20, 2019
@mbiegert
Copy link

mbiegert commented Oct 7, 2019

Is it possible this never made it to the 1.x branch? I don't get any further arguments apart from an Error object on v1.2.1.

This would be really helpful, at the moment I have to parse the error and extract the receipt from the message string.

@cgewecke
Copy link
Collaborator

cgewecke commented Oct 7, 2019

@mbiegert Hi, have added this to #3070 where things to back-port from 2.x to 1.x are being tracked. In the interim, another option is to collect the receipt from the receipt event, which should also fire when there's an error.

@mbiegert
Copy link

mbiegert commented Oct 7, 2019

Thanks for doing that. I've just read up about the situation with the two branches.
I'll keep using the parsing since the 'receipt' event does not fire.

@cgewecke
Copy link
Collaborator

cgewecke commented Oct 7, 2019

the 'receipt' event does not fire.

@mbiegert Ah! Out of curiosity, how are you connecting to the client?

@mbiegert
Copy link

mbiegert commented Oct 8, 2019

With HDWalletProvider from the trufflesuite:

import Web3 from 'web3';
import HDWalletProvider from '@truffle/hdwallet-provider';

const privateKey = process.env.BACKEND_WALLET_PRIVATE_KEY;

const infuraApiKey = '<mykey>';

const provider = new HDWalletProvider(privateKey, `https://rinkeby.infura.io/v3/${infuraApiKey}`);
const web3 = new Web3(provider);

export default web3;

For now this is the dev setup, but the advantage over web3.eth.accounts.wallet is that we can easily switch later to some other provider where the key es securely stored somewhere else.

@truffle/hdwallet-provider is at version 1.0.18 (slightly outdated, current version: 1.0.20, but don't know the changelog)

@cgewecke
Copy link
Collaborator

cgewecke commented Oct 8, 2019

Thanks so much @mbiegert.

@nivida nivida added Bug Addressing a bug and removed Documentation Relates to project wiki or documentation labels Oct 8, 2019
@nventuro
Copy link

nventuro commented Oct 8, 2019

From #2612 (and my local testing), it does seem like receipt is not fired on reverted transactions. My setup is different, I'm running a standard HttpProvider.

@cgewecke
Copy link
Collaborator

cgewecke commented Oct 8, 2019

@nventuro Yeah, it's not being fired - I believe it used to be at some point in beta.30's.

For logical consistency reasons it makes sense that it should fire whenever a receipt is available, regardless of status.

Changing the current behavior is a little tricky because people may now be assuming that receipt and error events are mutually exclusive.

@nventuro
Copy link

nventuro commented Oct 8, 2019

From the events described in the docs, I'd expect confirmation to be the one that is mutually exclusive with error. Otherwise .on('receipt', handler) becomes simply a shorthand for

.on('confirmation', (number, receipt) => 
  if (number === 1) { 
    handler(receipt);
  }
)

That said, I get the intent behind trying to not break existing dependents. In that case, I'd suggest adding a replacement for receipt, since getting receipts for all transactions (regardless of success status) is a very reasonable use case, and requiring two subscriptions to handle this can be annoying.

@nivida
Copy link
Contributor

nivida commented Oct 8, 2019

The receipt event will only get fired if the transaction got mined successfully. We are able to extend the error callback listener with the receipt if existing. This means it will return a receipt if the status property is false (reverted by the EVM) or if it ran out of gas. To achieve this would we have to extend the _fireError function and we would also have to pass the receipt in the Method class.

This implementation would be aligned with 2.x which is already in usage from some projects.

Edit:
This functionality got removed with this commit (beta.18).

@mbiegert
Copy link

mbiegert commented Oct 8, 2019

The documentation should probably also be clarified, specifying that .on('receipt') guarantees receipt.status === true and will only be fired for a successfull transaction and .on('error') gets a receipt object, if it was a failed transaction. (Could also have been a network error, is there a simple way to differentiate then?)

@nventuro
Copy link

nventuro commented Oct 8, 2019

The receipt event will only get fired if the transaction got mined successfully.

'Mined successfully' as in a successful execution? If not, shouldn't reverted (including out of gas) transactions still emit the event then?

@nivida
Copy link
Contributor

nivida commented Oct 16, 2019

The error event does now also return the receipt as second parameter. Changing and adding of events as proposed by @nventuro will be done on 1.3.0 or even on 2.0. I wrote it down on my todo list and will create an issue with an idea asap.

@nivida nivida closed this as completed Oct 16, 2019
1.0 automation moved this from Backlog to Done Oct 16, 2019
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
No open projects
1.0
  
Done
Development

No branches or pull requests

6 participants