Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error messages refactoring #1854

Merged
merged 61 commits into from
Jan 22, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
775c141
use VMError instead of String in FunctionCall error
lexfrl Dec 12, 2019
5f2226f
some progress on errors
lexfrl Dec 18, 2019
a986bc7
errors refactor
lexfrl Dec 18, 2019
bc9910c
revert changes (rename fields)
lexfrl Dec 18, 2019
ad1e830
test pass
lexfrl Dec 18, 2019
4cd0ff5
error view update
lexfrl Dec 19, 2019
5fc2e61
WIP
lexfrl Dec 19, 2019
5696c99
make it compile
lexfrl Dec 19, 2019
6515670
tests pass
lexfrl Dec 19, 2019
27ab39b
action index added to ActionError
lexfrl Dec 20, 2019
3062291
action index added: test pass
lexfrl Dec 20, 2019
8657d92
merge statging
lexfrl Dec 23, 2019
6d7c555
FunctionCall(String) -> FunctionCall(VMError)
lexfrl Dec 23, 2019
52ebc4d
rpctypegen start
lexfrl Dec 25, 2019
a339c34
continue with RpcError derive_macro
lexfrl Dec 26, 2019
c32ddb9
continue working
lexfrl Dec 26, 2019
63e7cc4
errors JSON schema generation done
lexfrl Dec 27, 2019
3aebd75
Do not rewrite the schema file. Merge error schemas instead.
lexfrl Dec 30, 2019
695c11b
small fix
lexfrl Dec 30, 2019
212a430
VMErrors refactoring
lexfrl Dec 31, 2019
08f601f
expose errors schema
lexfrl Jan 3, 2020
614287f
rename rpc_errors_schema.json
lexfrl Jan 6, 2020
c07565d
merge staging
lexfrl Jan 6, 2020
95ce423
specify a nearlib revision to test against
lexfrl Jan 8, 2020
c09cb6c
fix tests
lexfrl Jan 8, 2020
cd6e006
fix merge
lexfrl Jan 8, 2020
03c0ee8
fix rpctypegen build
lexfrl Jan 8, 2020
5979f68
Merge branch 'staging' into fckt/rpc-errors
lexfrl Jan 8, 2020
86ad272
fix ci: exclude near_rpc_error_macro from tests
lexfrl Jan 10, 2020
7815156
Merge branch 'staging' into fckt/rpc-errors
lexfrl Jan 10, 2020
f09766e
Merge branch 'staging' into fckt/rpc-errors
lexfrl Jan 10, 2020
a82fa00
add Errors docs, rename error types fix Balance formatting
lexfrl Jan 13, 2020
9694cf6
fix formatting
lexfrl Jan 13, 2020
216c808
Merge branch 'staging' into fckt/rpc-errors
lexfrl Jan 13, 2020
7090763
make schema deterministic
lexfrl Jan 13, 2020
c15391c
Update core/primitives/src/errors.rs
lexfrl Jan 13, 2020
d3f7e7b
Update core/primitives/src/errors.rs
lexfrl Jan 13, 2020
6f30669
Update core/primitives/src/errors.rs
lexfrl Jan 13, 2020
3c8f755
Apply suggestions from code review
lexfrl Jan 13, 2020
006bdf8
return ActionErrorKind::ActorNoPermission on AccountDelete and not sir
lexfrl Jan 13, 2020
68a3a99
Merge branch 'fckt/rpc-errors' of github.com:nearprotocol/nearcore in…
lexfrl Jan 13, 2020
4c476f1
StorageError doc comment addition
lexfrl Jan 13, 2020
75f5383
update BalanceMismatch doc comment
lexfrl Jan 13, 2020
90c1f44
Apply suggestions from code review
lexfrl Jan 13, 2020
7fb443d
Merge branch 'staging' into fckt/rpc-errors
lexfrl Jan 13, 2020
1a7eb06
ActionError index doc added
lexfrl Jan 13, 2020
4b749d7
Merge branch 'fckt/rpc-errors' of github.com:nearprotocol/nearcore in…
lexfrl Jan 13, 2020
a2ae077
fix check_actor_permissions for DeleteAccount
lexfrl Jan 13, 2020
879dffe
no cargo clean in build_errors_schema.sh and move to scripts
lexfrl Jan 13, 2020
c641f4a
feature-gated schema dump generation
lexfrl Jan 14, 2020
b9ae3fd
Merge branch 'staging' into fckt/rpc-errors
lexfrl Jan 16, 2020
18ccdf3
Update tools/rpctypegen/Cargo.toml
lexfrl Jan 17, 2020
361319b
removed rpc_error_variant attribute
lexfrl Jan 20, 2020
7bbba23
split rpctypegen into core and macro crated + tests added
lexfrl Jan 20, 2020
615121d
Merge branch 'staging' into fckt/rpc-errors
lexfrl Jan 20, 2020
addf11f
ServerError Display impl
lexfrl Jan 20, 2020
c1a094b
moved build_errors_schema.sh and res/rpc_errors_schema.json to a json…
lexfrl Jan 20, 2020
d9f3c6d
fix rpc-errors test
lexfrl Jan 21, 2020
e44e157
ActionError -> RequiresFullAccess
lexfrl Jan 21, 2020
d56236f
Merge branch 'staging' into fckt/rpc-errors
lexfrl Jan 22, 2020
983478b
fix comment
lexfrl Jan 22, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 32 additions & 13 deletions chain/jsonrpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ pub use near_jsonrpc_client as client;
use near_jsonrpc_client::{message, BlockId, ChunkId};
use near_metrics::{Encoder, TextEncoder};
use near_network::{NetworkClientMessages, NetworkClientResponses};
use near_primitives::errors::{ExecutionError, InvalidTxError};
use near_primitives::hash::CryptoHash;
use near_primitives::serialize::{from_base, from_base64, BaseEncode};
use near_primitives::transaction::SignedTransaction;
use near_primitives::types::AccountId;
use near_primitives::views::{ExecutionErrorView, FinalExecutionStatus};
use near_primitives::views::FinalExecutionStatus;

mod metrics;
pub mod test_utils;
Expand Down Expand Up @@ -118,15 +119,36 @@ fn parse_hash(params: Option<Value>) -> Result<CryptoHash, RpcError> {
})
}

fn convert_mailbox_error(e: MailboxError) -> ExecutionErrorView {
ExecutionErrorView { error_message: e.to_string(), error_type: "MailBoxError".to_string() }
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
pub enum ServerError {
TxExecutionError(ExecutionError),
Timeout,
Closed,
}

impl From<InvalidTxError> for ServerError {
fn from(e: InvalidTxError) -> ServerError {
ServerError::TxExecutionError(ExecutionError::InvalidTx(e))
}
}

impl From<MailboxError> for ServerError {
fn from(e: MailboxError) -> Self {
match e {
MailboxError::Closed => ServerError::Closed,
MailboxError::Timeout => ServerError::Timeout,
}
}
}

impl From<ServerError> for RpcError {
fn from(e: ServerError) -> RpcError {
RpcError::server_error(Some(e))
}
}

fn timeout_err() -> RpcError {
RpcError::server_error(Some(ExecutionErrorView {
error_message: "send_tx_commit has timed out".to_string(),
error_type: "TimeoutError".to_string(),
}))
RpcError::server_error(Some(ServerError::Timeout))
}

struct JsonRpcHandler {
Expand Down Expand Up @@ -211,21 +233,18 @@ impl JsonRpcHandler {
let result = self
.client_addr
.send(NetworkClientMessages::Transaction(tx))
.map_err(|err| RpcError::server_error(Some(convert_mailbox_error(err))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bowenwang1996 @frol In what cases RPC will return a MailboxError? AFAIU it only happens if the communication between the actors has timed out or the channel has been closed. The latter one can happen when the node is shutting down (btw, we need to implement a way to gracefully shutdown the node). The former one is not supposed to happen in our infra, since the communication between the actors does not have a timeout , correct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think this error can only happen under extreme circumstances, but @evgenykuzyakov insisted that I handle it. I think it can also happen if the node receives too much traffic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. @bowenwang1996 is it correct that communication between our Actix actors uses bounded channels that can overflow and drop the messages? Is it possible to arrive to an unforeseen state due to some messages being lost between the Actix actors? CC @SkidanovAlex

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure. I think @frol knows actix better, but we in general should be resistant to lost messages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like Actix uses unbounded channels by the default, so it should be good unless we explicitly changed it. actix/actix#14

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unbounded channels are the major source of unbounded memory consumption, and they tend to hide problems until it is too late.

.map_err(|err| RpcError::server_error(Some(ServerError::from(err))))
.compat()
.await?;
match result {
NetworkClientResponses::ValidTx | NetworkClientResponses::RequestRouted => {
self.tx_polling(tx_hash, signer_account_id).await
}
NetworkClientResponses::InvalidTx(err) => {
Err(RpcError::server_error(Some(ExecutionErrorView::from(err))))
Err(RpcError::server_error(Some(ServerError::TxExecutionError(err.into()))))
}
NetworkClientResponses::NoResponse => {
Err(RpcError::server_error(Some(ExecutionErrorView {
error_message: "send_tx_commit has timed out".to_string(),
error_type: "TimeoutError".to_string(),
})))
Err(RpcError::server_error(Some(ServerError::Timeout)))
}
_ => unreachable!(),
}
Expand Down
1 change: 1 addition & 0 deletions core/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ actix = "0.8.1"
borsh = "0.2.10"

near-crypto = { path = "../crypto" }
near-vm-errors = { path = "../../runtime/near-vm-errors" }

[features]
default = ["jemallocator"]
Expand Down
Loading