refactor(jsonrpc): Add structured errors to query method in ViewClien…

…t and RPC (near#3944) * Introduce some error struct into runtime/runtime/state_viewer. Refactoring query method to add structurred errors there WIP * Fix rosetta-rpc, update runtime's adapter trait * Add missed handler from rebasing with master * Refactor new error structs to be more explicit. Runtime call function, view state and access key methods return typical errors instead of QueryError * Extend missed error variants to be explicit * Address review suggestion, create dummy for testing rpc query method * Fix some naming issues * Adjust error structures * Update rpc_query tests * Add block_height and block_hash to error variants where it is possible * Address review suggestions. Backward compatible resposne from query * Make backward compatibility in `query` method via near_client::QueryError instead of RpcQueryError * Fix some tests * Address review suggestion * Fix tests according to changes in jsonrpc query method refactoring * Drop QueryResponseKind::Error variant and add backward compatibility on JSON-RPC side * Remove redundant commented code * Add tests for query method. Address issue with wrong refactor logic * Clean up and make more strict test check * run cargo fmt * Mark rpc routing tests as ignore * Fix tests
decentra1ized · Mar 9, 2021 · f68140e · f68140e
1 parent dc73b0f
commit f68140e
Show file tree

Hide file tree

Showing 40 changed files with 1,182 additions and 558 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/chain/chain-primitives/Cargo.toml b/chain/chain-primitives/Cargo.toml
@@ -11,5 +11,7 @@ chrono = { version = "0.4.4", features = ["serde"] }
 failure = "0.1"
 failure_derive = "0.1"
 log = "0.4"
+thiserror = "1.0"
 
 near-primitives = { path = "../../core/primitives" }
+near-crypto = { path = "../../core/crypto" }
diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs
@@ -12,6 +12,48 @@ use near_primitives::serialize::to_base;
 use near_primitives::sharding::{ChunkHash, ShardChunkHeader};
 use near_primitives::types::{BlockHeight, EpochId, ShardId};
 
+#[derive(thiserror::Error, Debug)]
+pub enum QueryError {
+    #[error("Account ID #{requested_account_id} is invalid")]
+    InvalidAccount {
+        requested_account_id: near_primitives::types::AccountId,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    #[error("Account #{requested_account_id} does not exist while viewing")]
+    UnknownAccount {
+        requested_account_id: near_primitives::types::AccountId,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    #[error(
+        "Contract code for contract ID #{contract_account_id} has never been observed on the node"
+    )]
+    NoContractCode {
+        contract_account_id: near_primitives::types::AccountId,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    #[error("Access key for public key #{public_key} does not exist while viewing")]
+    UnknownAccessKey {
+        public_key: near_crypto::PublicKey,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    #[error("Internal error occurred: #{error_message}")]
+    InternalError {
+        error_message: String,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    #[error("Function call returned an error: #{error_message}")]
+    ContractExecutionError {
+        error_message: String,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+}
+
 #[derive(Debug)]
 pub struct Error {
     inner: Context<ErrorKind>,

diff --git a/chain/chain/src/lib.rs b/chain/chain/src/lib.rs
@@ -4,7 +4,7 @@ extern crate lazy_static;
 pub use chain::{collect_receipts, Chain, MAX_ORPHAN_SIZE};
 pub use doomslug::{Doomslug, DoomslugBlockProductionReadiness, DoomslugThresholdMode};
 pub use lightclient::{create_light_client_block_view, get_epoch_block_producers_view};
-pub use near_chain_primitives::{Error, ErrorKind};
+pub use near_chain_primitives::{self, Error, ErrorKind};
 pub use store::{ChainStore, ChainStoreAccess, ChainStoreUpdate};
 pub use store_validator::{ErrorMessage, StoreValidator};
 pub use types::{

diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs
@@ -787,7 +787,7 @@ impl RuntimeAdapter for KeyValueRuntime {
         block_hash: &CryptoHash,
         _epoch_id: &EpochId,
         request: &QueryRequest,
-    ) -> Result<QueryResponse, Box<dyn std::error::Error>> {
+    ) -> Result<QueryResponse, near_chain_primitives::error::QueryError> {
         match request {
             QueryRequest::ViewAccount { account_id, .. } => Ok(QueryResponse {
                 kind: QueryResponseKind::ViewAccount(

diff --git a/chain/chain/src/types.rs b/chain/chain/src/types.rs
@@ -575,7 +575,7 @@ pub trait RuntimeAdapter: Send + Sync {
         block_hash: &CryptoHash,
         epoch_id: &EpochId,
         request: &QueryRequest,
-    ) -> Result<QueryResponse, Box<dyn std::error::Error>>;
+    ) -> Result<QueryResponse, near_chain_primitives::error::QueryError>;
 
     fn get_validator_info(
         &self,

diff --git a/chain/client-primitives/Cargo.toml b/chain/client-primitives/Cargo.toml
@@ -17,6 +17,7 @@ near-chain-primitives = { path = "../chain-primitives" }
 near-chain-configs = { path = "../../core/chain-configs" }
 
 near-chunks = { path = "../chunks" }
+near-crypto = { path = "../../core/crypto" }
 near-network = { path = "../network" }
 near-primitives = { path = "../../core/primitives" }
 

diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs
@@ -267,7 +267,68 @@ impl Query {
 }
 
 impl Message for Query {
-    type Result = Result<Option<QueryResponse>, String>;
+    type Result = Result<QueryResponse, QueryError>;
+}
+
+#[derive(thiserror::Error, Debug)]
+pub enum QueryError {
+    #[error("Internal error: {error_message}")]
+    InternalError { error_message: String },
+    #[error("There are no fully synchronized blocks on the node yet")]
+    NoSyncedBlocks,
+    #[error("The node does not track the shard ID {requested_shard_id}")]
+    UnavailableShard { requested_shard_id: near_primitives::types::ShardId },
+    #[error("Account ID {requested_account_id} is invalid")]
+    InvalidAccount {
+        requested_account_id: near_primitives::types::AccountId,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    #[error(
+        "Account {requested_account_id} does not exist while viewing at block #{block_height}"
+    )]
+    UnknownAccount {
+        requested_account_id: near_primitives::types::AccountId,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    #[error(
+        "Contract code for contract ID {contract_account_id} has never been observed on the node at block #{block_height}"
+    )]
+    NoContractCode {
+        contract_account_id: near_primitives::types::AccountId,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    #[error("Access key for public key {public_key} has never been observed on the node at block #{block_height}")]
+    UnknownAccessKey {
+        public_key: near_crypto::PublicKey,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    #[error("Function call returned an error: {vm_error}")]
+    ContractExecutionError {
+        vm_error: String,
+        block_height: near_primitives::types::BlockHeight,
+        block_hash: near_primitives::hash::CryptoHash,
+    },
+    // NOTE: Currently, the underlying errors are too broad, and while we tried to handle
+    // expected cases, we cannot statically guarantee that no other errors will be returned
+    // in the future.
+    // TODO #3851: Remove this variant once we can exhaustively match all the underlying errors
+    #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: {error_message}")]
+    Unreachable { error_message: String },
+}
+
+impl From<near_chain_primitives::Error> for QueryError {
+    fn from(error: near_chain_primitives::Error) -> Self {
+        match error.kind() {
+            near_chain_primitives::ErrorKind::IOErr(error_message) => {
+                Self::InternalError { error_message }
+            }
+            _ => Self::Unreachable { error_message: error.to_string() },
+        }
+    }
 }
 
 pub struct Status {

diff --git a/chain/client/Cargo.toml b/chain/client/Cargo.toml
@@ -23,6 +23,7 @@ borsh = "0.8.1"
 reed-solomon-erasure = "4"
 num-rational = "0.3"
 linked-hash-map = "0.5.3"
+thiserror = "1.0"
 
 near-crypto = { path = "../../core/crypto" }
 near-primitives = { path = "../../core/primitives" }

diff --git a/chain/client/src/lib.rs b/chain/client/src/lib.rs
@@ -6,7 +6,7 @@ pub use near_client_primitives::types::{
     GetExecutionOutcome, GetExecutionOutcomeResponse, GetExecutionOutcomesForBlock, GetGasPrice,
     GetNetworkInfo, GetNextLightClientBlock, GetProtocolConfig, GetReceipt, GetStateChanges,
     GetStateChangesInBlock, GetStateChangesWithCauseInBlock, GetValidatorInfo, GetValidatorOrdered,
-    Query, Status, StatusResponse, SyncStatus, TxStatus, TxStatusError,
+    Query, QueryError, Status, StatusResponse, SyncStatus, TxStatus, TxStatusError,
 };
 
 pub use crate::client::Client;