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

RPC errors not bubbled back to Ðapp #8483

Closed
2 tasks
danfinlay opened this issue Apr 30, 2020 · 23 comments
Closed
2 tasks

RPC errors not bubbled back to Ðapp #8483

danfinlay opened this issue Apr 30, 2020 · 23 comments
Labels
area-api Sev2-normal Normal severity; minor loss of service or inconvenience. team-wallet-api-platform type-bug

Comments

@danfinlay
Copy link
Contributor

Currently when a Ðapp initiates a call to eth_sendTransaction, if the RPC backend throws an error (like transaction underpriced), this error is not returned to the Ðapp, instead a generic internal wrapper is returned, like "Error: [ethjs-rpc] rpc error with payload {"...

Checked and this behavior continues to exist on develop.

Ideal case:

  • Return the RPC error to the Ðapp itself.
  • Show the failure reason in the transaction history to the user.
@wighawag
Copy link

Ok, seems I am getting this issue, but I am getting the following error for every metamask tx, even when I use metamask ui to send ether to another address

Here is the error I get when executing one my dapp transaction :

{code: -32603, message: "Error: [ethjs-rpc] rpc error with payload {"id":79…method":"eth_sendRawTransaction"} [object Object]", stack: "Error: Error: [ethjs-rpc] rpc error with payload {…method":"eth_sendRawTransaction"} [object Object]"}
stack : "Error: Error: [ethjs-rpc] rpc error with payload {"id":7973398896595,"jsonrpc":"2.0","params":["0xf8cb2b8504a817c80082bd5d943037e0b21e7b8e0babaf3611d6d0d5cac51e67c080b864c47f0027000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000036473640000000000000000000000000000000000000000000000000000000000820a95a04314f99297ee7e82db567bc8008c3d2739558e9010b9dac763d266a71f72bb0ca03a9aa5658bfc4dde3c754c40a5345aaba47819cc6df5b39c57144fb42c60bf85"],"method":"eth_sendRawTransaction"} [object Object]"

@wighawag
Copy link

This was on chrome and loaded a new profile with a new metamask and there my dapp function fine

@adrianmcli
Copy link

This is quite frustrating, as things work fine when I am just running scripts in Node.js but suddenly throws a cryptic error when I do it through the frontend. Is there a toString() getting called somewhere that's causing the underlying object (w/ revert reason string) to become [object Object] instead?

@MicahZoltu
Copy link

Not only is the RPC mangled, it also is wrapped in a more opaque error. When I inspect the JSON-RPC error I get back from sendAsync, I see:

{
  "code": -32603,
  "message": "[object Object]",
  "data": {
    "code": -32603,
    "message": "revert: ERC20: transfer amount exceeds allowance"
  },
  "stack": "..."
}

Note that the actual JSON-RPC error (the onet hat would be immensely useful for debugging) is burried inside the data field of the error I actually get back.

@wbt
Copy link
Contributor

wbt commented Jul 13, 2020

I am not seeing the data member in Chrome.

@Gudahtt Gudahtt added the Sev2-normal Normal severity; minor loss of service or inconvenience. label Jan 11, 2021
@Gudahtt
Copy link
Member

Gudahtt commented Jan 19, 2021

Relates to #8975 and #7142

@MicahZoltu
Copy link

I received an error report from a user today where it appears MetaMask is throwing a string instead of an error! This means I don't get a stack trace, which is critical for debugging.

This is what is showing up in my code's catch block.

{
	"code":-32000,
	"message":"err: insufficient funds for gas * price + value: address ... have ... want ... (supplied gas ...)"
}

@danjm danjm added the area-api label Sep 24, 2021
@vandan
Copy link

vandan commented Jul 26, 2022

Duplicate of #12646

@vandan vandan marked this as a duplicate of #12646 Jul 26, 2022
@MicahZoltu
Copy link

I think you mean that #12646 is a duplicate of this. 😉

@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label Sep 4, 2023
@adrianmcli
Copy link

I don't think this is stale.

@github-actions github-actions bot removed the stale issues and PRs marked as stale label Sep 4, 2023
@vandan
Copy link

vandan commented Oct 20, 2023

A related PR was created #15205 but resulted in other issues (tradeoffs). Needs further assessment on the approach here.

@wighawag
Copy link

@vandan thanks for chiming in here

Could you please do so for all issues that are being closed by bots. This is honestly alienating people like me who would live to see Metamask improve and create issue with that goal in mind, but whose only interlucutor is a bot closing issues :(

I understand that you might have too many issues in your hand, but closing issues blindly will only make the issue count down. The issue wont resolve themeselves.

Sorry if that sounds harsh but some issue are being closed while they exist in the repo for a long time with few or no comment from Metamask team

@vandan
Copy link

vandan commented Oct 21, 2023

@vandan thanks for chiming in here

Could you please do so for all issues that are being closed by bots. This is honestly alienating people like me who would live to see Metamask improve and create issue with that goal in mind, but whose only interlucutor is a bot closing issues :(

I understand that you might have too many issues in your hand, but closing issues blindly will only make the issue count down. The issue wont resolve themeselves.

Sorry if that sounds harsh but some issue are being closed while they exist in the repo for a long time with few or no comment from Metamask team

We understand your frustration. We'll see what can be done about the automated actions, but generally the best way we can avoid this is by being more responsive to these issues so they don't become stale in the first place.
We absolutely appreciate the effort our community goes through to help MetaMask improve and don't want to lose your goodwill. 🙏

@MicahZoltu
Copy link

FWIW, I have stopped filing issues with MetaMask because 90%+ of the issues I encounter just get closed by the bot and remains unfixed.

@adrianmcli
Copy link

What's most ironic is that this issue was filed by @danfinlay himself.

Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label Jan 19, 2024
@adrianmcli
Copy link

Not stale. Do we know if this new plan has changed and the necessary fixes inside ethjs-query have been implemented?

image

From here: #15205 (comment)

I'm not sure if I have the energy to say "not stale" again.

@github-actions github-actions bot removed the stale issues and PRs marked as stale label Jan 19, 2024
@MicahZoltu
Copy link

Sadly, I gave up on filing issues and commenting "not stale". 90% of the issues I follow/file just go stale and are never fixed, even after many years of the bug existing.

@adrianmcli
Copy link

Sadly, I gave up on filing issues and commenting "not stale". 90% of the issues I follow/file just go stale and are never fixed, even after many years of the bug existing.

With all the money that MetaMask makes. I wonder why they can't have someone do triage on these open source issues?

And I've said it above a few months ago, but the most ironic thing is that this is an issue @danfinlay raised himself.

Dan probably will not remember, but I have fond memories of chatting with Dan when I was part of ConsenSys in 2018. Times have really changed.

@BelfordZ
Copy link
Contributor

Not stale. Do we know if this new plan has changed and the necessary fixes inside ethjs-query have been implemented?

image From here: [#15205 (comment)](https://github.com//pull/15205#issuecomment-1199953855)

I'm not sure if I have the energy to say "not stale" again.

Sorry, I tried to get it sorted out but it turned out to be more complicated then it seemed. I had a this PR that we had to reject becasue it would cause breaking changes for those who had already written code to parse these bad errors. A new plan was hatched, but other priorities came up and I wasn't able to keep at it.

Just know that its on our radar, and it will get better. I also understand asking for patience at this point is almost insulting, so I will spare you on that.

@vandan
Copy link

vandan commented Mar 15, 2024

This issue is no longer reproducible. See our notes for the detail about why the issue is resolved.

Please re-open if you are able to reproduce.

@vandan vandan closed this as completed Mar 15, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Mar 15, 2024
@steven1227
Copy link

steven1227 commented Dec 26, 2024

Not only is the RPC mangled, it also is wrapped in a more opaque error. When I inspect the JSON-RPC error I get back from sendAsync, I see:

{
"code": -32603,
"message": "[object Object]",
"data": {
"code": -32603,
"message": "revert: ERC20: transfer amount exceeds allowance"
},
"stack": "..."
}
Note that the actual JSON-RPC error (the onet hat would be immensely useful for debugging) is burried inside the data field of the error I actually get back.

Nowadays, when invoke the contract using sendTransaction, the error.data is not returned anymore?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-api Sev2-normal Normal severity; minor loss of service or inconvenience. team-wallet-api-platform type-bug
Projects
Archived in project
Development

No branches or pull requests

11 participants