Skip to content

Commit

Permalink
refactor(jsonrpc): Refactor view_client structured errors according t…
Browse files Browse the repository at this point in the history
…o removal of BlockMissing for GetBlockError, GetChunkError, QueryError (near#4079)

According to recent removal of `BlockMissing` we had to refactor some structures to be up to date
  • Loading branch information
khorolets authored Mar 12, 2021
1 parent 39a7cc5 commit 5a01db5
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 90 deletions.
1 change: 1 addition & 0 deletions CODEOWNERS
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# CODEOWNERS: https://help.github.com/articles/about-codeowners/

/chain/ @SkidanovAlex @bowenwang1996
/chain/client/src/view_client.rs @frol @khorolets
/chain/network/ @mfornet @bowenwang1996
/chain/jsonrpc/ @frol @khorolets
/chain/jsonrpc-primitives/ @frol @khorolets
Expand Down
64 changes: 36 additions & 28 deletions chain/client-primitives/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,28 +158,30 @@ 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("IO Error: {error_message}")]
IOError { error_message: String },
#[error("Block either has never been observed on the node or has been garbage collected: {error_message}")]
UnknownBlock { error_message: 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),
#[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 GetBlockError {
fn from(error: near_chain_primitives::Error) -> Self {
match error.kind() {
near_chain_primitives::ErrorKind::IOErr(s) => Self::IOError(s),
near_chain_primitives::ErrorKind::DBNotFoundErr(s) => Self::BlockNotFound(s),
_ => Self::Unreachable(error.to_string()),
near_chain_primitives::ErrorKind::IOErr(error_message) => {
Self::IOError { error_message }
}
near_chain_primitives::ErrorKind::DBNotFoundErr(error_message) => {
Self::UnknownBlock { error_message }
}
_ => Self::Unreachable { error_message: error.to_string() },
}
}
}
Expand Down Expand Up @@ -220,34 +222,38 @@ impl Message for GetChunk {

#[derive(thiserror::Error, Debug)]
pub enum GetChunkError {
#[error("IO Error: {0}")]
IOError(String),
#[error("Block has never been observed: {0}")]
UnknownBlock(String),
#[error("Block with hash `{0}` is unavailable on the node")]
UnavailableBlock(CryptoHash),
#[error("Shard ID {0} is invalid")]
InvalidShardId(u64),
#[error("Chunk with hash {0:?} has never been observed on this node")]
UnknownChunk(ChunkHash),
#[error("IO Error: {error_message}")]
IOError { error_message: String },
#[error("Block either has never been observed on the node or has been garbage collected: {error_message}")]
UnknownBlock { error_message: String },
#[error("Shard ID {shard_id} is invalid")]
InvalidShardId { shard_id: u64 },
#[error("Chunk with hash {chunk_hash:?} has never been observed on this node")]
UnknownChunk { chunk_hash: ChunkHash },
// 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),
#[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 GetChunkError {
fn from(error: near_chain_primitives::Error) -> Self {
match error.kind() {
near_chain_primitives::ErrorKind::IOErr(s) => Self::IOError(s),
near_chain_primitives::ErrorKind::DBNotFoundErr(s) => Self::UnknownBlock(s),
near_chain_primitives::ErrorKind::IOErr(error_message) => {
Self::IOError { error_message }
}
near_chain_primitives::ErrorKind::DBNotFoundErr(error_message) => {
Self::UnknownBlock { error_message }
}
near_chain_primitives::ErrorKind::InvalidShardId(shard_id) => {
Self::InvalidShardId(shard_id)
Self::InvalidShardId { shard_id }
}
near_chain_primitives::ErrorKind::ChunkMissing(hash) => Self::UnknownChunk(hash),
_ => Self::Unreachable(error.to_string()),
near_chain_primitives::ErrorKind::ChunkMissing(chunk_hash) => {
Self::UnknownChunk { chunk_hash }
}
_ => Self::Unreachable { error_message: error.to_string() },
}
}
}
Expand Down Expand Up @@ -312,6 +318,8 @@ pub enum QueryError {
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 },
// 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.
Expand Down
15 changes: 11 additions & 4 deletions chain/client/src/view_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,20 +203,27 @@ impl ViewClientActor {
self.chain.get_block_header(&block_hash)
}
BlockReference::Finality(ref finality) => {
let block_hash = self.get_block_hash_by_finality(&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::SyncCheckpoint(ref synchronization_checkpoint) => {
if let Some(block_hash) =
self.get_block_hash_by_sync_checkpoint(&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(),
})?
{
self.chain.get_block_header(&block_hash)
} else {
return Err(QueryError::NoSyncedBlocks);
}
}
};
let header = header?.clone();
let header = header
.map_err(|_| QueryError::UnknownBlock { block_reference: msg.block_reference.clone() })?
.clone();

let account_id = match &msg.request {
QueryRequest::ViewAccount { account_id, .. } => account_id,
Expand Down
44 changes: 19 additions & 25 deletions chain/jsonrpc-primitives/src/types/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,18 @@ pub enum BlockReference {

#[derive(thiserror::Error, Debug)]
pub enum RpcBlockError {
#[error("Block `{0}` is missing")]
BlockMissing(near_primitives::hash::CryptoHash),
#[error("Block not found")]
BlockNotFound(String),
#[error("Block not found: {error_message}")]
UnknownBlock { error_message: String },
#[error("There are no fully synchronized blocks yet")]
NotSyncedYet,
#[error("The node reached its limits. Try again later. More details: {0}")]
InternalError(String),
#[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
// 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),
#[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 },
}

#[derive(Serialize, Deserialize)]
Expand Down Expand Up @@ -56,44 +54,40 @@ impl From<BlockReference> for near_primitives::types::BlockReference {
impl From<near_client_primitives::types::GetBlockError> for RpcBlockError {
fn from(error: near_client_primitives::types::GetBlockError) -> Self {
match error {
near_client_primitives::types::GetBlockError::BlockMissing(block_hash) => {
Self::BlockMissing(block_hash)
}
near_client_primitives::types::GetBlockError::BlockNotFound(s) => {
Self::BlockNotFound(s)
near_client_primitives::types::GetBlockError::UnknownBlock { error_message } => {
Self::UnknownBlock { error_message }
}
near_client_primitives::types::GetBlockError::NotSyncedYet => Self::NotSyncedYet,
near_client_primitives::types::GetBlockError::IOError(s) => Self::InternalError(s),
near_client_primitives::types::GetBlockError::Unreachable(error_message) => {
near_client_primitives::types::GetBlockError::IOError { error_message } => {
Self::InternalError { error_message }
}
near_client_primitives::types::GetBlockError::Unreachable { error_message } => {
tracing::warn!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message);
near_metrics::inc_counter_vec(
&crate::metrics::RPC_UNREACHABLE_ERROR_COUNT,
&["RpcBlockError"],
);
Self::Unreachable(error_message)
Self::Unreachable { error_message }
}
}
}
}

impl From<actix::MailboxError> for RpcBlockError {
fn from(error: actix::MailboxError) -> Self {
Self::InternalError(error.to_string())
Self::InternalError { error_message: error.to_string() }
}
}

impl From<RpcBlockError> for crate::errors::RpcError {
fn from(error: RpcBlockError) -> Self {
let error_data = match error {
RpcBlockError::BlockMissing(hash) => Some(Value::String(format!(
"Block Missing (unavailable on the node): {} \n Cause: Unknown",
hash.to_string()
RpcBlockError::UnknownBlock { error_message } => Some(Value::String(format!(
"DB Not Found Error: {} \n Cause: Unknown",
error_message
))),
RpcBlockError::BlockNotFound(s) => {
Some(Value::String(format!("DB Not Found Error: {} \n Cause: Unknown", s)))
}
RpcBlockError::Unreachable(s) => Some(Value::String(s)),
RpcBlockError::NotSyncedYet | RpcBlockError::InternalError(_) => {
RpcBlockError::Unreachable { error_message } => Some(Value::String(error_message)),
RpcBlockError::NotSyncedYet | RpcBlockError::InternalError { .. } => {
Some(Value::String(error.to_string()))
}
};
Expand Down
62 changes: 29 additions & 33 deletions chain/jsonrpc-primitives/src/types/chunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,20 @@ pub struct RpcChunkResponse {

#[derive(thiserror::Error, Debug)]
pub enum RpcChunkError {
#[error("The node reached its limits. Try again later. More details: {0}")]
InternalError(String),
#[error("Block not found")]
UnknownBlock(String),
#[error("Block `{0}` is unavailable on the node")]
UnavailableBlock(near_primitives::hash::CryptoHash),
#[error("Shard id {0} does not exist")]
InvalidShardId(u64),
#[error("Chunk with hash {0:?} has never been observed on this node")]
UnknownChunk(near_primitives::sharding::ChunkHash),
#[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: {error_message}")]
UnknownBlock { error_message: String },
#[error("Shard id {shard_id} does not exist")]
InvalidShardId { shard_id: u64 },
#[error("Chunk with hash {chunk_hash:?} has never been observed on this node")]
UnknownChunk { chunk_hash: near_primitives::sharding::ChunkHash },
// 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),
#[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<ChunkReference> for near_client_primitives::types::GetChunk {
Expand Down Expand Up @@ -82,52 +80,50 @@ impl RpcChunkRequest {
impl From<near_client_primitives::types::GetChunkError> for RpcChunkError {
fn from(error: near_client_primitives::types::GetChunkError) -> Self {
match error {
near_client_primitives::types::GetChunkError::IOError(s) => Self::InternalError(s),
near_client_primitives::types::GetChunkError::UnknownBlock(s) => Self::UnknownBlock(s),
near_client_primitives::types::GetChunkError::UnavailableBlock(hash) => {
Self::UnavailableBlock(hash)
near_client_primitives::types::GetChunkError::IOError { error_message } => {
Self::InternalError { error_message }
}
near_client_primitives::types::GetChunkError::InvalidShardId(shard_id) => {
Self::InvalidShardId(shard_id)
near_client_primitives::types::GetChunkError::UnknownBlock { error_message } => {
Self::UnknownBlock { error_message }
}
near_client_primitives::types::GetChunkError::UnknownChunk(hash) => {
Self::UnknownChunk(hash)
near_client_primitives::types::GetChunkError::InvalidShardId { shard_id } => {
Self::InvalidShardId { shard_id }
}
near_client_primitives::types::GetChunkError::Unreachable(error_message) => {
near_client_primitives::types::GetChunkError::UnknownChunk { chunk_hash } => {
Self::UnknownChunk { chunk_hash }
}
near_client_primitives::types::GetChunkError::Unreachable { error_message } => {
tracing::warn!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message);
near_metrics::inc_counter_vec(
&crate::metrics::RPC_UNREACHABLE_ERROR_COUNT,
&["RpcChunkError"],
);
Self::Unreachable(error_message)
Self::Unreachable { error_message }
}
}
}
}

impl From<actix::MailboxError> for RpcChunkError {
fn from(error: actix::MailboxError) -> Self {
Self::InternalError(error.to_string())
Self::InternalError { error_message: error.to_string() }
}
}

impl From<RpcChunkError> for crate::errors::RpcError {
fn from(error: RpcChunkError) -> Self {
let error_data = match error {
RpcChunkError::InternalError(_) => Some(Value::String(error.to_string())),
RpcChunkError::UnavailableBlock(hash) => Some(Value::String(format!(
RpcChunkError::InternalError { .. } => Some(Value::String(error.to_string())),
RpcChunkError::UnknownBlock { error_message } => Some(Value::String(format!(
"DB Not Found Error: {} \n Cause: Unknown",
hash.to_string()
error_message
))),
RpcChunkError::UnknownBlock(s) => {
Some(Value::String(format!("DB Not Found Error: {} \n Cause: Unknown", s)))
}
RpcChunkError::InvalidShardId(_) => Some(Value::String(error.to_string())),
RpcChunkError::UnknownChunk(hash) => Some(Value::String(format!(
RpcChunkError::InvalidShardId { .. } => Some(Value::String(error.to_string())),
RpcChunkError::UnknownChunk { chunk_hash } => Some(Value::String(format!(
"Chunk Missing (unavailable on the node): ChunkHash(`{}`) \n Cause: Unknown",
hash.0.to_string()
chunk_hash.0.to_string()
))),
RpcChunkError::Unreachable(s) => Some(Value::String(s)),
RpcChunkError::Unreachable { error_message } => Some(Value::String(error_message)),
};

Self::new(-32_000, "Server error".to_string(), error_data)
Expand Down
5 changes: 5 additions & 0 deletions chain/jsonrpc-primitives/src/types/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ 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
Expand Down Expand Up @@ -165,6 +167,9 @@ impl From<near_client_primitives::types::QueryError> for RpcQueryError {
near_client_primitives::types::QueryError::UnavailableShard { requested_shard_id } => {
Self::UnavailableShard { requested_shard_id }
}
near_client_primitives::types::QueryError::UnknownBlock { block_reference } => {
Self::UnknownBlock { block_reference }
}
near_client_primitives::types::QueryError::InvalidAccount {
requested_account_id,
block_height,
Expand Down

0 comments on commit 5a01db5

Please sign in to comment.