change some error codes jsonrpc error handling

This commit is contained in:
Bryan Stitt 2023-04-10 22:29:02 -07:00
parent b5ed0c4710
commit b1120e635a
6 changed files with 89 additions and 53 deletions

View File

@ -399,6 +399,8 @@ These are not yet ordered. There might be duplicates. We might not actually need
- [x] improve "archive_needed" boolean. change to "block_depth"
- [x] keep score of new_head timings for all rpcs
- [x] having the whole block in /status is very verbose. trim it down
- [x] maybe we shouldn't route eth_getLogs to syncing nodes. serving queries slows down sync significantly
- change the send_best function to only include servers that are at least close to fully synced
- [-] proxy mode for benchmarking all backends
- [-] proxy mode for sending to multiple backends
- [-] let users choose a % of reverts to log (or maybe x/second). someone like curve logging all reverts will be a BIG database very quickly
@ -416,9 +418,11 @@ These are not yet ordered. There might be duplicates. We might not actually need
- then sites like curve.fi don't have to worry about their user count
- it does mean we will have a harder time capacity planning from the number of keys
- [ ] have the healthcheck get the block over http. if it errors, or doesn't match what the websocket says, something is wrong (likely a deadlock in the websocket code)
- [ ] has_block_data is too simple. it needs to know what kind of data is being requested
- all nodes have all blocks
- most nodes have all receipts
- only archives have old state
- [ ] don't use new_head_provider anywhere except new head subscription
- [x] maybe we shouldn't route eth_getLogs to syncing nodes. serving queries slows down sync significantly
- change the send_best function to only include servers that are at least close to fully synced
- [ ] enable mev protected transactions with either a /protect/ url (instead of /private/) or the database (when on /rpc/)
- [-] have private transactions be enabled by a url setting rather than a setting on the key
- [ ] eth_sendRawTransaction should only forward if the chain_id matches what we are running

View File

@ -143,7 +143,8 @@ fn main() -> anyhow::Result<()> {
vec![
"info",
"ethers=debug",
"ethers_providers=warn",
// TODO: even error is too verbose for our purposes. how can we turn off this logging entirely?
"ethers_providers=error",
"redis_rate_limit=debug",
"web3_proxy=debug",
"web3_proxy_cli=debug",
@ -229,6 +230,8 @@ fn main() -> anyhow::Result<()> {
};
log::set_max_level(max_level);
info!("RUST_LOG={}", rust_log);
}
info!("{}", APP_USER_AGENT);

View File

@ -67,7 +67,9 @@ pub enum Web3ProxyError {
JoinError(JoinError),
#[display(fmt = "{:?}", _0)]
#[error(ignore)]
JsonRpc(crate::jsonrpc::JsonRpcErrorData),
JsonRpcForwardedError(JsonRpcForwardedResponse),
#[display(fmt = "{:?}", _0)]
#[error(ignore)]
MsgPackEncode(rmp_serde::encode::Error),
NoBlockNumberOrHash,
NoBlocksKnown,
@ -247,6 +249,7 @@ impl Web3ProxyError {
),
)
}
Self::JsonRpcForwardedError(x) => (StatusCode::OK, x),
Self::GasEstimateNotU256 => {
warn!("GasEstimateNotU256");
(
@ -413,24 +416,13 @@ impl Web3ProxyError {
),
)
}
Self::JsonRpc(err) => {
debug!("JsonRpc err={:?}", err);
(
StatusCode::BAD_REQUEST,
JsonRpcForwardedResponse::from_str(
"json rpc error!",
Some(StatusCode::BAD_REQUEST.as_u16().into()),
None,
),
)
}
Self::MsgPackEncode(err) => {
debug!("MsgPackEncode Error: {}", err);
warn!("MsgPackEncode Error: {}", err);
(
StatusCode::BAD_REQUEST,
StatusCode::INTERNAL_SERVER_ERROR,
JsonRpcForwardedResponse::from_str(
&format!("msgpack encode error: {}", err),
Some(StatusCode::BAD_REQUEST.as_u16().into()),
Some(StatusCode::INTERNAL_SERVER_ERROR.as_u16().into()),
None,
),
)
@ -438,10 +430,10 @@ impl Web3ProxyError {
Self::NoBlockNumberOrHash => {
warn!("NoBlockNumberOrHash");
(
StatusCode::BAD_REQUEST,
StatusCode::INTERNAL_SERVER_ERROR,
JsonRpcForwardedResponse::from_str(
"Blocks here must have a number or hash",
Some(StatusCode::BAD_REQUEST.as_u16().into()),
Some(StatusCode::INTERNAL_SERVER_ERROR.as_u16().into()),
None,
),
)
@ -449,10 +441,10 @@ impl Web3ProxyError {
Self::NoBlocksKnown => {
error!("NoBlocksKnown");
(
StatusCode::INTERNAL_SERVER_ERROR,
StatusCode::BAD_GATEWAY,
JsonRpcForwardedResponse::from_str(
"no blocks known",
Some(StatusCode::INTERNAL_SERVER_ERROR.as_u16().into()),
Some(StatusCode::BAD_GATEWAY.as_u16().into()),
None,
),
)
@ -460,10 +452,10 @@ impl Web3ProxyError {
Self::NoConsensusHeadBlock => {
error!("NoConsensusHeadBlock");
(
StatusCode::INTERNAL_SERVER_ERROR,
StatusCode::BAD_GATEWAY,
JsonRpcForwardedResponse::from_str(
"no consensus head block",
Some(StatusCode::INTERNAL_SERVER_ERROR.as_u16().into()),
Some(StatusCode::BAD_GATEWAY.as_u16().into()),
None,
),
)
@ -506,7 +498,7 @@ impl Web3ProxyError {
}
Self::NotFound => {
// TODO: emit a stat?
// TODO: instead of an error, show a normal html page for 404
// TODO: instead of an error, show a normal html page for 404?
(
StatusCode::NOT_FOUND,
JsonRpcForwardedResponse::from_str(
@ -528,7 +520,7 @@ impl Web3ProxyError {
)
}
Self::OriginRequired => {
warn!("OriginRequired");
trace!("OriginRequired");
(
StatusCode::BAD_REQUEST,
JsonRpcForwardedResponse::from_str(
@ -539,7 +531,7 @@ impl Web3ProxyError {
)
}
Self::OriginNotAllowed(origin) => {
warn!("OriginNotAllowed origin={}", origin);
trace!("OriginNotAllowed origin={}", origin);
(
StatusCode::FORBIDDEN,
JsonRpcForwardedResponse::from_string(
@ -550,7 +542,7 @@ impl Web3ProxyError {
)
}
Self::ParseBytesError(err) => {
warn!("ParseBytesError err={:?}", err);
trace!("ParseBytesError err={:?}", err);
(
StatusCode::BAD_REQUEST,
JsonRpcForwardedResponse::from_str(
@ -561,7 +553,7 @@ impl Web3ProxyError {
)
}
Self::ParseMsgError(err) => {
warn!("ParseMsgError err={:?}", err);
trace!("ParseMsgError err={:?}", err);
(
StatusCode::BAD_REQUEST,
JsonRpcForwardedResponse::from_str(
@ -572,9 +564,9 @@ impl Web3ProxyError {
)
}
Self::ParseAddressError => {
warn!("ParseAddressError");
trace!("ParseAddressError");
(
StatusCode::UNAUTHORIZED,
StatusCode::BAD_REQUEST,
JsonRpcForwardedResponse::from_str(
"unable to parse address",
Some(StatusCode::BAD_REQUEST.as_u16().into()),
@ -775,6 +767,7 @@ impl Web3ProxyError {
}
Self::UserIdZero => {
warn!("UserIdZero");
// TODO: this might actually be an application error and not a BAD_REQUEST
(
StatusCode::BAD_REQUEST,
JsonRpcForwardedResponse::from_str(
@ -785,7 +778,7 @@ impl Web3ProxyError {
)
}
Self::VerificationError(err) => {
warn!("VerificationError err={:?}", err);
trace!("VerificationError err={:?}", err);
(
StatusCode::BAD_REQUEST,
JsonRpcForwardedResponse::from_str(
@ -818,7 +811,7 @@ impl Web3ProxyError {
)
}
Self::WebsocketOnly => {
warn!("WebsocketOnly");
trace!("WebsocketOnly");
(
StatusCode::BAD_REQUEST,
JsonRpcForwardedResponse::from_str(
@ -828,20 +821,23 @@ impl Web3ProxyError {
),
)
}
Self::WithContext(err, msg) => {
info!("in context: {}", msg);
match err {
Some(err) => err.into_response_parts(),
None => (
Self::WithContext(err, msg) => match err {
Some(err) => {
warn!("{:#?} w/ context {}", err, msg);
err.into_response_parts()
}
None => {
warn!("error w/ context {}", msg);
(
StatusCode::INTERNAL_SERVER_ERROR,
JsonRpcForwardedResponse::from_string(
msg,
Some(StatusCode::INTERNAL_SERVER_ERROR.as_u16().into()),
None,
),
),
)
}
}
},
}
}
}

View File

@ -4,6 +4,7 @@
use super::authorization::{ip_is_authorized, key_is_authorized, Authorization, RequestMetadata};
use super::errors::{Web3ProxyError, Web3ProxyResponse};
use crate::jsonrpc::JsonRpcId;
use crate::stats::RpcQueryStats;
use crate::{
app::Web3ProxyApp,
@ -29,7 +30,6 @@ use hashbrown::HashMap;
use http::StatusCode;
use log::{info, trace, warn};
use serde_json::json;
use serde_json::value::to_raw_value;
use std::sync::Arc;
use std::{str::from_utf8_mut, sync::atomic::AtomicUsize};
use tokio::sync::{broadcast, OwnedSemaphorePermit, RwLock};
@ -420,9 +420,7 @@ async fn handle_socket_payload(
(id, response)
}
Err(err) => {
// TODO: move this logic somewhere else and just set id to None here
let id =
to_raw_value(&json!(None::<Option::<()>>)).expect("None can always be a RawValue");
let id = JsonRpcId::None.to_raw_value();
(id, Err(err.into()))
}
};

View File

@ -7,8 +7,6 @@ use serde_json::json;
use serde_json::value::{to_raw_value, RawValue};
use std::fmt;
// this is used by serde
#[allow(dead_code)]
fn default_jsonrpc() -> String {
"2.0".to_string()
}
@ -24,6 +22,45 @@ pub struct JsonRpcRequest {
pub params: Option<serde_json::Value>,
}
#[derive(From)]
pub enum JsonRpcId {
None,
Number(u64),
String(String),
}
impl JsonRpcId {
pub fn to_raw_value(&self) -> Box<RawValue> {
// TODO: is this a good way to do this? we should probably use references
match self {
Self::None => {
to_raw_value(&json!(None::<Option<()>>)).expect("null id should always work")
}
Self::Number(x) => {
serde_json::from_value(json!(x)).expect("number id should always work")
}
Self::String(x) => serde_json::from_str(x).expect("string id should always work"),
}
}
}
impl JsonRpcRequest {
pub fn new(
id: JsonRpcId,
method: String,
params: Option<serde_json::Value>,
) -> anyhow::Result<Self> {
let x = Self {
jsonrpc: default_jsonrpc(),
id: id.to_raw_value(),
method,
params,
};
Ok(x)
}
}
impl fmt::Debug for JsonRpcRequest {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// TODO: the default formatter takes forever to write. this is too quiet though
@ -206,9 +243,7 @@ impl JsonRpcForwardedResponse {
// TODO: can we somehow get the initial request here? if we put that into a tracing span, will things slow down a ton?
JsonRpcForwardedResponse {
jsonrpc: "2.0".to_string(),
id: id.unwrap_or_else(|| {
to_raw_value(&json!(None::<Option<()>>)).expect("null id should always work")
}),
id: id.unwrap_or_else(|| JsonRpcId::None.to_raw_value()),
result: None,
error: Some(JsonRpcErrorData {
code: code.unwrap_or(-32099),

View File

@ -251,8 +251,8 @@ impl Web3Rpcs {
)
.await?;
if let Some(err) = response.error {
return Err(err).web3_context("failed fetching block");
if response.error.is_some() {
return Err(response.into());
}
let block = response
@ -347,8 +347,8 @@ impl Web3Rpcs {
.try_send_best_consensus_head_connection(authorization, request, None, Some(num), None)
.await?;
if let Some(err) = response.error {
debug!("could not find canonical block {}: {:?}", num, err);
if response.error.is_some() {
return Err(response.into());
}
let raw_block = response.result.web3_context("no cannonical block result")?;