refactor(jsonrpc): Initial refactoring for Structured Error handling …

…for JSON RPC and ViewClient (near#3805)

Partially addressing near#3517

Cargo.toml

@@ -25,11 +25,13 @@ members = [
     "chain/epoch_manager",
     "chain/pool",
     "chain/client",
+    "chain/client-primitives",
     "chain/network",
     "chain/indexer",
     "chain/jsonrpc",
     "chain/jsonrpc/client",
     "chain/jsonrpc/test-utils",
+    "chain/jsonrpc-primitives",
     "chain/rosetta-rpc",
     "test-utils/testlib",
     "test-utils/loadtester",

chain/chain-primitives/Cargo.toml

@@ -0,0 +1,15 @@
+[package]
+name = "near-chain-primitives"
+version = "0.1.0"
+authors = ["Near Inc <[email protected]>"]
+edition = "2018"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+chrono = { version = "0.4.4", features = ["serde"] }
+failure = "0.1"
+failure_derive = "0.1"
+log = "0.4"
+
+near-primitives = { path = "../../core/primitives" }

chain/chain-primitives/src/lib.rs

@@ -0,0 +1,3 @@
+pub use error::{Error, ErrorKind};
+
+pub mod error;

chain/chain/Cargo.toml

@@ -23,6 +23,7 @@ strum_macros = "0.18"
 borsh = "0.7.1"
 
 near-chain-configs = { path = "../../core/chain-configs" }
+near-chain-primitives = { path = "../chain-primitives" }
 near-crypto = { path = "../../core/crypto" }
 near-primitives = { path = "../../core/primitives" }
 near-store = { path = "../../core/store" }

chain/chain/src/chain.rs

@@ -10,7 +10,6 @@ use rand::rngs::StdRng;
 use rand::seq::SliceRandom;
 use rand::SeedableRng;
 
-use crate::error::{Error, ErrorKind, LogTransientStorageError};
 use crate::lightclient::get_epoch_block_producers_view;
 use crate::missing_chunks::{BlockLike, MissingChunksPool};
 use crate::store::{ChainStore, ChainStoreAccess, ChainStoreUpdate, GCMode};
@@ -24,6 +23,8 @@ use crate::validate::{
 };
 use crate::{byzantine_assert, create_light_client_block_view, Doomslug};
 use crate::{metrics, DoomslugThresholdMode};
+
+use near_chain_primitives::error::{Error, ErrorKind, LogTransientStorageError};
 use near_primitives::block::{genesis_chunks, Tip};
 use near_primitives::challenge::{
     BlockDoubleSign, Challenge, ChallengeBody, ChallengesResult, ChunkProofs, ChunkState,

chain/chain/src/lib.rs

@@ -3,8 +3,8 @@ extern crate lazy_static;
 
 pub use chain::{collect_receipts, Chain, MAX_ORPHAN_SIZE};
 pub use doomslug::{Doomslug, DoomslugBlockProductionReadiness, DoomslugThresholdMode};
-pub use error::{Error, ErrorKind};
 pub use lightclient::{create_light_client_block_view, get_epoch_block_producers_view};
+pub use near_chain_primitives::{Error, ErrorKind};
 pub use store::{ChainStore, ChainStoreAccess, ChainStoreUpdate};
 pub use store_validator::{ErrorMessage, StoreValidator};
 pub use types::{
@@ -13,7 +13,6 @@ pub use types::{
 
 pub mod chain;
 mod doomslug;
-mod error;
 mod lightclient;
 mod metrics;
 pub mod missing_chunks;

chain/chain/src/lightclient.rs

@@ -1,9 +1,9 @@
+use near_chain_primitives::Error;
 use near_primitives::block::BlockHeader;
 use near_primitives::hash::{hash, CryptoHash};
 use near_primitives::types::EpochId;
 use near_primitives::views::{BlockHeaderInnerLiteView, LightClientBlockView, ValidatorStakeView};
 
-use crate::error::Error;
 use crate::{ChainStoreAccess, RuntimeAdapter};
 
 pub fn get_epoch_block_producers_view(

chain/chain/src/store.rs

@@ -9,6 +9,7 @@ use cached::{Cached, SizedCache};
 use chrono::Utc;
 use tracing::debug;
 
+use near_chain_primitives::error::{Error, ErrorKind};
 use near_primitives::block::{Approval, Tip};
 use near_primitives::errors::InvalidTxError;
 use near_primitives::hash::CryptoHash;
@@ -46,7 +47,6 @@ use near_store::{
     LARGEST_TARGET_HEIGHT_KEY, LATEST_KNOWN_KEY, SHOULD_COL_GC, TAIL_KEY,
 };
 
-use crate::error::{Error, ErrorKind};
 use crate::types::{Block, BlockHeader, LatestKnown};
 use crate::{byzantine_assert, ReceiptResult};
 

chain/chain/src/test_utils.rs

@@ -9,6 +9,7 @@ use log::debug;
 use num_rational::Rational;
 use serde::Serialize;
 
+use near_chain_primitives::{Error, ErrorKind};
 use near_crypto::{KeyType, PublicKey, SecretKey, Signature};
 use near_pool::types::PoolIterator;
 use near_primitives::account::{AccessKey, Account};
@@ -38,7 +39,6 @@ use near_store::{
 };
 
 use crate::chain::{Chain, NUM_EPOCHS_TO_KEEP_STORE_DATA};
-use crate::error::{Error, ErrorKind};
 use crate::store::ChainStoreAccess;
 use crate::types::{ApplyTransactionResult, BlockHeaderInfo, ChainGenesis};
 use crate::{BlockHeader, DoomslugThresholdMode, RuntimeAdapter};

chain/chain/src/types.rs

@@ -5,6 +5,7 @@ use std::sync::Arc;
 use borsh::{BorshDeserialize, BorshSerialize};
 use serde::Serialize;
 
+use near_chain_primitives::Error;
 use near_crypto::Signature;
 use near_pool::types::PoolIterator;
 pub use near_primitives::block::{Block, BlockHeader, Tip};
@@ -26,7 +27,6 @@ use near_primitives::version::{
 use near_primitives::views::{EpochValidatorInfo, QueryRequest, QueryResponse};
 use near_store::{PartialStorage, ShardTries, Store, Trie, WrappedTrieChanges};
 
-use crate::error::Error;
 use chrono::{DateTime, Utc};
 use near_chain_configs::GenesisConfig;
 use num_rational::Rational;

chain/client-primitives/Cargo.toml

@@ -0,0 +1,22 @@
+[package]
+name = "near-client-primitives"
+version = "0.1.0"
+authors = ["Near Inc <[email protected]>"]
+edition = "2018"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+actix = "0.9"
+chrono = { version = "0.4.4", features = ["serde"] }
+serde = { version = "1", features = ["derive"] }
+strum = { version = "0.18", features = ["derive"] }
+thiserror = "1.0"
+
+near-chain-primitives = { path = "../chain-primitives" }
+near-chunks = { path = "../chunks" }
+near-network = { path = "../network" }
+near-primitives = { path = "../../core/primitives" }
+
+[features]
+metric_recorder = []

chain/client-primitives/src/lib.rs

@@ -0,0 +1 @@
+pub mod types;

chain/client/src/types.rs → chain/client-primitives/src/types.rs

@@ -29,7 +29,7 @@ pub use near_primitives::views::{StatusResponse, StatusSyncInfo};
 /// Combines errors coming from chain, tx pool and block producer.
 #[derive(Debug)]
 pub enum Error {
-    Chain(near_chain::Error),
+    Chain(near_chain_primitives::Error),
     Chunk(near_chunks::Error),
     BlockProducer(String),
     ChunkProducer(String),
@@ -50,15 +50,15 @@ impl std::fmt::Display for Error {
 
 impl std::error::Error for Error {}
 
-impl From<near_chain::Error> for Error {
-    fn from(e: near_chain::Error) -> Self {
+impl From<near_chain_primitives::Error> for Error {
+    fn from(e: near_chain_primitives::Error) -> Self {
         Error::Chain(e)
     }
 }
 
-impl From<near_chain::ErrorKind> for Error {
-    fn from(e: near_chain::ErrorKind) -> Self {
-        let error: near_chain::Error = e.into();
+impl From<near_chain_primitives::ErrorKind> for Error {
+    fn from(e: near_chain_primitives::ErrorKind) -> Self {
+        let error: near_chain_primitives::Error = e.into();
         Error::Chain(error)
     }
 }
@@ -150,14 +150,45 @@ impl SyncStatus {
 /// Actor message requesting block by id or hash.
 pub struct GetBlock(pub BlockReference);
 
+#[derive(thiserror::Error, Debug)]
+pub enum GetBlockError {
+    #[error("IO Error: {0}")]
+    IOError(String),
+    #[error("Block `{0}` is missing")]
+    BlockMissing(CryptoHash),
+    #[error("Block not found")]
+    BlockNotFound(String),
+    #[error("There are no fully synchronized blocks yet")]
+    NotSyncedYet,
+    // 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: {0}")]
+    Unreachable(String),
+}
+
+impl From<near_chain_primitives::Error> for GetBlockError {
+    fn from(error: near_chain_primitives::Error) -> GetBlockError {
+        match error.kind() {
+            near_chain_primitives::ErrorKind::IOErr(s) => GetBlockError::IOError(s),
+            near_chain_primitives::ErrorKind::DBNotFoundErr(s) => GetBlockError::BlockNotFound(s),
+            near_chain_primitives::ErrorKind::BlockMissing(hash) => {
+                GetBlockError::BlockMissing(hash)
+            }
+            _ => GetBlockError::Unreachable(error.to_string()),
+        }
+    }
+}
+
 impl GetBlock {
     pub fn latest() -> Self {
         Self(BlockReference::latest())
     }
 }
 
 impl Message for GetBlock {
-    type Result = Result<BlockView, String>;
+    type Result = Result<BlockView, GetBlockError>;
 }
 
 /// Get block with the block merkle tree. Used for testing
@@ -170,7 +201,7 @@ impl GetBlockWithMerkleTree {
 }
 
 impl Message for GetBlockWithMerkleTree {
-    type Result = Result<(BlockView, PartialMerkleTree), String>;
+    type Result = Result<(BlockView, PartialMerkleTree), GetBlockError>;
 }
 
 /// Actor message requesting a chunk by chunk hash and block hash + shard id.
@@ -254,7 +285,7 @@ pub struct TxStatus {
 
 #[derive(Debug)]
 pub enum TxStatusError {
-    ChainError(near_chain::Error),
+    ChainError(near_chain_primitives::Error),
     MissingTransaction(CryptoHash),
     InvalidTx(InvalidTxError),
     InternalError,

chain/client/Cargo.toml

@@ -30,6 +30,7 @@ near-store = { path = "../../core/store" }
 near-metrics = { path = "../../core/metrics" }
 near-chain-configs = { path = "../../core/chain-configs" }
 near-chain = { path = "../chain" }
+near-client-primitives = { path = "../client-primitives" }
 near-network = { path = "../network" }
 near-pool = { path = "../pool" }
 near-chunks = { path = "../chunks" }
@@ -49,7 +50,7 @@ neard = { path = "../../neard" }
 byzantine_asserts = ["near-chain/byzantine_asserts"]
 expensive_tests = []
 adversarial = ["near-network/adversarial", "near-chain/adversarial"]
-metric_recorder = []
+metric_recorder = ["near-client-primitives/metric_recorder"]
 delay_detector = ["near-chain/delay_detector", "near-network/delay_detector", "delay-detector"]
 protocol_feature_forward_chunk_parts = ["near-primitives/protocol_feature_forward_chunk_parts", "near-network/protocol_feature_forward_chunk_parts", "near-chunks/protocol_feature_forward_chunk_parts"]
 protocol_feature_block_ordinal = ["near-primitives/protocol_feature_block_ordinal"]

chain/client/src/client.rs

@@ -41,8 +41,8 @@ use near_primitives::validator_signer::ValidatorSigner;
 
 use crate::metrics;
 use crate::sync::{BlockSync, HeaderSync, StateSync, StateSyncResult};
-use crate::types::{Error, ShardSyncDownload};
 use crate::SyncStatus;
+use near_client_primitives::types::{Error, ShardSyncDownload};
 use near_primitives::block_header::ApprovalType;
 use near_primitives::version::{ProtocolVersion, PROTOCOL_VERSION};
 
@@ -1755,7 +1755,7 @@ mod test {
             let result = client.process_partial_encoded_chunk_forward(mock_forward);
             assert!(matches!(
                 result,
-                Err(crate::types::Error::Chunk(near_chunks::Error::UnknownChunk))
+                Err(near_client_primitives::types::Error::Chunk(near_chunks::Error::UnknownChunk))
             ));
         }
     }