Propagate tx info into ServerErrors #388
Conversation
…ced in error if missing in schema
ilblackdragon
force-pushed
the
account-errors
branch
from
a43342f to
81f6403
Compare
| @@ -57,6 +69,8 @@ function walkSubtype(errorObj, schema, result, typeName) { | |||
| } | |||
| return walkSubtype(error, schema, result, errorTypeName); | |||
| } else { | |||
| // TODO: is this the right thing to do? | |||
There was a problem hiding this comment.
unfortunately I don't know, as I have no understanding of what are possible responses from nearcore side.
also I think we should completely remove everything related to error schema #331
There was a problem hiding this comment.
@bowenwang1996 I feel like you may know a bit about the nearcore errors. Would you mind giving us your two cents when you get a moment this week?
There was a problem hiding this comment.
an error only has kind if it is an action error but there are many other errors out there
|
@vgrichina this repo is important in the dependency chain for the EVM tooling. Can we prioritize getting this looked at this week, please? |
|
Initially I thought we simply needed the section where exports happen in the second commit here: because without that, Balancer tests fail saying:
Adding those exports alone solves that, but we do need this full PR in order to get the best results from tests. 9-failing-with_account-errors_branch.txt I'm wondering how we can get this PR moving along. I don't know a ton about errors, except that it might be a heavy lift to address the line that we're commenting about here We're trying to keep a pretty aggressive timeline in terms of having the EVM-related PRs buttoned up ASAP. Wondering how I can help, and what the expected lift is for this. I'll tag @djsatok so we can make sure this project is tracked |
# Conflicts: # dist/near-api-js.js # dist/near-api-js.min.js # lib/browser-index.js # lib/common-index.js # lib/index.js # lib/providers/provider.js # lib/utils/index.js # lib/utils/rpc_errors.js # lib/utils/serialize.js
There was a problem hiding this comment.
Ran a bunch of other checks in addition to the rather thorough tests we have here.
Nonexistent account
./bin/near state nonexistent
When no connection to RPC is there:
NEAR_ENV=local ./bin/near state nonexistent
When a wrong argument type is given:
./bin/near view oracle.chainlink.testnet get_all_requests '{"max_num_accounts": "100", "max_requests": "abc"}'
Calling nonexistent method:
./bin/near view oracle.chainlink.testnet ZZZget_all_requests '{"max_num_accounts": "100", "max_requests": "abc"}'
The only test that had issues was getting cross-contract failures, but finally figured it out that we needed to explicitly add the error.type and error.message and now everything is behaving perfectly.
Also, talked to Vlad about this yesterday, and while there is discussion about completely removing the class-based way that errors are handled currently, it's better to have that be a separate task.
This PR addresses EVM closely, especially with "revert" errors, so this PR represents a major improvement for the EVM and we'd like to merge it asap.
|
Note: we are going to hold off on merging this until we've resolved this issue: |
# Conflicts: # lib/account_multisig.js # lib/browser-index.js # lib/common-index.js # lib/index.js # lib/utils/index.js # lib/utils/rpc_errors.js # lib/utils/serialize.js
No description provided.