fix(jsonrpc): Fix some errors inconsistency with unknown blocks or un…

…tracked shards for handle_query view_client method (near#4119) This PR improves errors in `handle_query` method after refactoring. Solves some problems found in the issue near#4063
decentra1ized · Mar 17, 2021 · 6815b39 · 6815b39
1 parent 39e0f0b
commit 6815b39
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 34 deletions.
diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs
@@ -14,39 +14,39 @@ use near_primitives::types::{BlockHeight, EpochId, ShardId};
 
 #[derive(thiserror::Error, Debug)]
 pub enum QueryError {
-    #[error("Account ID #{requested_account_id} is invalid")]
+    #[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")]
+    #[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"
+        "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")]
+    #[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}")]
+    #[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}")]
+    #[error("Function call returned an error: {error_message}")]
     ContractExecutionError {
         error_message: String,
         block_height: near_primitives::types::BlockHeight,

diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs
@@ -278,8 +278,6 @@ impl Message for Query {
 
 #[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}")]
@@ -318,6 +316,8 @@ pub enum QueryError {
         block_height: near_primitives::types::BlockHeight,
         block_hash: near_primitives::hash::CryptoHash,
     },
+    #[error("The node reached its limits. Try again later. More details: {error_message}")]
+    InternalError { error_message: String },
     #[error("Block either has never been observed on the node or has been garbage collected: {block_reference:?}")]
     UnknownBlock { block_reference: near_primitives::types::BlockReference },
     // NOTE: Currently, the underlying errors are too broad, and while we tried to handle
@@ -328,17 +328,6 @@ pub enum QueryError {
     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 {
     pub is_health_check: bool,
 }

diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs
@@ -202,17 +202,22 @@ impl ViewClientActor {
             BlockReference::BlockId(BlockId::Hash(block_hash)) => {
                 self.chain.get_block_header(&block_hash)
             }
-            BlockReference::Finality(ref finality) => {
-                let block_hash = self.get_block_hash_by_finality(&finality).map_err(|_| {
-                    QueryError::UnknownBlock { block_reference: msg.block_reference.clone() }
-                })?;
-                self.chain.get_block_header(&block_hash)
-            }
+            BlockReference::Finality(ref finality) => self
+                .get_block_hash_by_finality(&finality)
+                .and_then(|block_hash| self.chain.get_block_header(&block_hash)),
             BlockReference::SyncCheckpoint(ref synchronization_checkpoint) => {
                 if let Some(block_hash) = self
                     .get_block_hash_by_sync_checkpoint(&synchronization_checkpoint)
-                    .map_err(|_| QueryError::UnknownBlock {
-                        block_reference: msg.block_reference.clone(),
+                    .map_err(|err| match err.kind() {
+                        near_chain::near_chain_primitives::ErrorKind::DBNotFoundErr(_) => {
+                            QueryError::UnknownBlock {
+                                block_reference: msg.block_reference.clone(),
+                            }
+                        }
+                        near_chain::near_chain_primitives::ErrorKind::IOErr(error_message) => {
+                            QueryError::InternalError { error_message }
+                        }
+                        _ => QueryError::Unreachable { error_message: err.to_string() },
                     })?
                 {
                     self.chain.get_block_header(&block_hash)
@@ -222,7 +227,15 @@ impl ViewClientActor {
             }
         };
         let header = header
-            .map_err(|_| QueryError::UnknownBlock { block_reference: msg.block_reference.clone() })?
+            .map_err(|err| match err.kind() {
+                near_chain::near_chain_primitives::ErrorKind::DBNotFoundErr(_) => {
+                    QueryError::UnknownBlock { block_reference: msg.block_reference.clone() }
+                }
+                near_chain::near_chain_primitives::ErrorKind::IOErr(error_message) => {
+                    QueryError::InternalError { error_message }
+                }
+                _ => QueryError::Unreachable { error_message: err.to_string() },
+            })?
             .clone();
 
         let account_id = match &msg.request {
@@ -235,7 +248,18 @@ impl ViewClientActor {
         };
         let shard_id = self.runtime_adapter.account_id_to_shard_id(account_id);
 
-        let chunk_extra = self.chain.get_chunk_extra(header.hash(), shard_id)?;
+        let chunk_extra = self.chain.get_chunk_extra(header.hash(), shard_id).map_err(|err| {
+            match err.kind() {
+                near_chain::near_chain_primitives::ErrorKind::DBNotFoundErr(_) => {
+                    QueryError::UnavailableShard { requested_shard_id: shard_id }
+                }
+                near_chain::near_chain_primitives::ErrorKind::IOErr(error_message) => {
+                    QueryError::InternalError { error_message }
+                }
+                _ => QueryError::Unreachable { error_message: err.to_string() },
+            }
+        })?;
+
         let state_root = chunk_extra.state_root;
         match self.runtime_adapter.query(
             shard_id,

diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs
@@ -14,12 +14,12 @@ pub struct RpcQueryRequest {
 
 #[derive(thiserror::Error, Debug)]
 pub enum RpcQueryError {
-    #[error("IO Error: {error_message}")]
-    IOError { error_message: String },
     #[error("There are no fully synchronized blocks on the node yet")]
     NoSyncedBlocks,
-    #[error("The node does not track the shard")]
+    #[error("The node does not track the shard ID {requested_shard_id}")]
     UnavailableShard { requested_shard_id: near_primitives::types::ShardId },
+    #[error("Block either has never been observed on the node or has been garbage collected: {block_reference:?}")]
+    UnknownBlock { block_reference: near_primitives::types::BlockReference },
     #[error("Account ID {requested_account_id} is invalid")]
     InvalidAccount {
         requested_account_id: near_primitives::types::AccountId,
@@ -52,8 +52,6 @@ pub enum RpcQueryError {
         block_height: near_primitives::types::BlockHeight,
         block_hash: near_primitives::hash::CryptoHash,
     },
-    #[error("Block either has never been observed on the node or has been garbage collected: {block_reference:?}")]
-    UnknownBlock { block_reference: near_primitives::types::BlockReference },
     #[error("The node reached its limits. Try again later. More details: {error_message}")]
     InternalError { error_message: String },
     // NOTE: Currently, the underlying errors are too broad, and while we tried to handle

diff --git a/chain/jsonrpc/src/lib.rs b/chain/jsonrpc/src/lib.rs
@@ -212,6 +212,24 @@ fn process_query_response(
                 "block_height": block_height,
                 "block_hash": block_hash,
             })),
+            near_jsonrpc_primitives::types::query::RpcQueryError::UnknownBlock {
+                ref block_reference,
+            } => match block_reference {
+                near_primitives::types::BlockReference::BlockId(block_id) => Err(RpcError::new(
+                    -32_000,
+                    "Server error".to_string(),
+                    Some(match block_id {
+                        near_primitives::types::BlockId::Height(height) => json!(format!(
+                            "DB Not Found Error: BLOCK HEIGHT: {} \n Cause: Unknown",
+                            height
+                        )),
+                        near_primitives::types::BlockId::Hash(block_hash) => {
+                            json!(format!("DB Not Found Error: BLOCK HEADER: {}", block_hash))
+                        }
+                    }),
+                )),
+                _ => Err(err.into()),
+            },
             _ => Err(err.into()),
         },
     }