search

MetaMask / metamask-mobile

Mobile web browser providing access to websites that use the Ethereum blockchain
https://metamask.io
Other
2.12k stars 1.1k forks source link

[Sentry] Error: [ethjs-rpc] rpc error with payload {"id":2109144632762,"jsonrpc":"2.0","params":[{"to":"**","data... #9704

Open sentry-io[bot] opened 3 months ago

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2E6V

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2E7E

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2F6D

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2ERR

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2EPT

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2FXF

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2DVD

node_modules/ethjs-ens/node_modules/ethjs-rpc/lib/index.js in self.currentProvider.sendAsync$argument_1 at line 53:35

  self.currentProvider.sendAsync(parsedPayload, function (err, response) {
    var responseObject = response || {};
    if (err || responseObject.error) {
      var payloadErrorMessage = '[ethjs-rpc] ' + (responseObject.error && 'rpc' || '') + ' error with payload ' + JSON.stringify(parsedPaylo {snip}
      var payloadError = new Error(payloadErrorMessage);
      payloadError.value = err || responseObject.error;
      return cb(payloadError, null);
    }
    return cb(null, responseObject.result);
NicolasMassart commented 3 months ago

I tracked this down to Engine.ts and the code pointed at in @metamask/json-rpc-engine but I'm not sure how this ends into an exception not being caught and land in Sentry.

desi commented 3 months ago

@Gudahtt has some notes to add here.

Gudahtt commented 3 months ago

The stack trace suggests that the error is happening as a result of an ethjs-ens call, but the usage of that package in this project is very limited. The only callsites I could find were within try..catch blocks that would not allow the error to reach Sentry. I tried auditing the package for errors thrown out of band as well, and was unable to find a single instance where that seemed possible.

Maybe it's not coming from ethjs-ens, and the stack trace that Sentry is showing us is incorrect? There are issues with the source maps in the affected releases.

Gudahtt commented 3 months ago

Perhaps we could put this issue aside until the sourcemaps are fixed, so that we can verify what the real unminified stack trace is.

Or alternatively, we could manually verify each stack frame using the production bundle, but @NicolasMassart and I discussed this approach already and we ran into issues here because the bundle uses byte code rather than JavaScript (I'm not sure how to decompile it while preserving line numbers, and even if we manage to, the resulting code bears very little resemblance to the source code)

mcmire commented 3 months ago

Okay, moving to Blocked for now.

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2G16

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2G16

sentry-io[bot] commented 3 months ago

Sentry Issue: METAMASK-MOBILE-2G68

NicolasMassart commented 2 months ago

Sourcemap are now correct, we check them regularily and at least for the last ones if matches the source code. We still may have issues as it's not solved on CI side, but if we figure it out fast enough and remove the extra artifacts it works. If sourcemap is wrong, the error message is compeletely unrelated to the code pointed at and it's clearly not the case here:

Error:

[ethjs-rpc] rpc error with payload {"id":7030016583578,"jsonrpc":"2.0","params":[{"to":"**","data":"******************************************5604fb93aada9cfabd08f3c0c91e4d26"},"latest"],"method":"eth_call"} Ty...

Code for node_modules/ethjs-ens/node_modules/ethjs-rpc/lib/index.js in self.currentProvider.sendAsync$argument_1 at line 53:35

  self.currentProvider.sendAsync(parsedPayload, function (err, response) {
    var responseObject = response || {};
    if (err || responseObject.error) {
      var payloadErrorMessage = '[ethjs-rpc] ' + (responseObject.error && 'rpc' || '') + ' error with payload ' + JSON.stringify(parsedPaylo {snip}
      var payloadError = new Error(payloadErrorMessage);
      payloadError.value = err || responseObject.error;
      return cb(payloadError, null);
    }
    return cb(null, responseObject.result);
mcmire commented 2 months ago

I looked into this, and there are two functions from ethjs-rpc that are used in the mobile app. Here is where they are used:

Note that there are a few cases where doENSReverseLookup or doENSLookup is not properly awaited, and so the error may be thrown out of band.

So I have a couple of questions:

sentry-io[bot] commented 1 month ago

Sentry Issue: METAMASK-MOBILE-2GHZ

sentry-io[bot] commented 1 month ago

Sentry Issue: METAMASK-MOBILE-2GW2

MajorLift commented 3 weeks ago

There were some recent type changes in @metamask/json-rpc-engine that may have affected error handling behavior, although the intention was to avoid introducing runtime breaking changes.

Looking at the _promiseHandle method linked above that method calls #handle, whose callback parameter's error parameter was changed from optional to required in this line. Maybe this or another change made in this release is related to this bug? The callback parameter in this link would be directly downstream of cb in the sendAsync method mentioned above.

Another similar change was made here in the handle method while fixing an any type.