From f79159a69c6e47563ca5423372c23078110898ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Sun, 20 Aug 2017 06:13:00 +0200 Subject: [PATCH] Improve some RPC error messages. (#6311) --- rpc/src/v1/impls/eth_pubsub.rs | 18 ++++++++++++++---- rpc/src/v1/types/block.rs | 2 +- rpc/src/v1/types/block_number.rs | 8 ++++++-- rpc/src/v1/types/bytes.rs | 4 ++-- rpc/src/v1/types/derivation.rs | 2 +- rpc/src/v1/types/filter.rs | 2 +- rpc/src/v1/types/index.rs | 8 ++++++-- rpc/src/v1/types/pubsub.rs | 2 +- 8 files changed, 32 insertions(+), 14 deletions(-) diff --git a/rpc/src/v1/impls/eth_pubsub.rs b/rpc/src/v1/impls/eth_pubsub.rs index 6ba88be21..f918a44ea 100644 --- a/rpc/src/v1/impls/eth_pubsub.rs +++ b/rpc/src/v1/impls/eth_pubsub.rs @@ -245,17 +245,27 @@ impl EthPubSub for EthPubSubClient { kind: pubsub::Kind, params: Trailing, ) { - match (kind, params.into()) { + let error = match (kind, params.into()) { (pubsub::Kind::NewHeads, None) => { - self.heads_subscribers.write().push(subscriber) + self.heads_subscribers.write().push(subscriber); + return; }, (pubsub::Kind::Logs, Some(pubsub::Params::Logs(filter))) => { self.logs_subscribers.write().push(subscriber, filter.into()); + return; + }, + (pubsub::Kind::NewHeads, _) => { + errors::invalid_params("newHeads", "Expected no parameters.") + }, + (pubsub::Kind::Logs, _) => { + errors::invalid_params("logs", "Expected a filter object.") }, _ => { - let _ = subscriber.reject(errors::unimplemented(None)); + errors::unimplemented(None) }, - } + }; + + let _ = subscriber.reject(error); } fn unsubscribe(&self, id: SubscriptionId) -> BoxFuture { diff --git a/rpc/src/v1/types/block.rs b/rpc/src/v1/types/block.rs index 525d2c090..5a56f962d 100644 --- a/rpc/src/v1/types/block.rs +++ b/rpc/src/v1/types/block.rs @@ -214,7 +214,7 @@ impl Serialize for Rich { // and serialize value.serialize(serializer) } else { - Err(S::Error::custom("Unserializable structures.")) + Err(S::Error::custom("Unserializable structures: expected objects")) } } } diff --git a/rpc/src/v1/types/block_number.rs b/rpc/src/v1/types/block_number.rs index 8f3f09f51..334bf360d 100644 --- a/rpc/src/v1/types/block_number.rs +++ b/rpc/src/v1/types/block_number.rs @@ -79,8 +79,12 @@ impl<'a> Visitor<'a> for BlockNumberVisitor { "latest" => Ok(BlockNumber::Latest), "earliest" => Ok(BlockNumber::Earliest), "pending" => Ok(BlockNumber::Pending), - _ if value.starts_with("0x") => u64::from_str_radix(&value[2..], 16).map(BlockNumber::Num).map_err(|_| Error::custom("invalid block number")), - _ => value.parse::().map(BlockNumber::Num).map_err(|_| Error::custom("invalid block number")) + _ if value.starts_with("0x") => u64::from_str_radix(&value[2..], 16).map(BlockNumber::Num).map_err(|e| { + Error::custom(format!("Invalid block number: {}", e)) + }), + _ => value.parse::().map(BlockNumber::Num).map_err(|e| { + Error::custom(format!("Invalid block number: {}", e)) + }), } } diff --git a/rpc/src/v1/types/bytes.rs b/rpc/src/v1/types/bytes.rs index deaa234fc..4bd10af44 100644 --- a/rpc/src/v1/types/bytes.rs +++ b/rpc/src/v1/types/bytes.rs @@ -81,9 +81,9 @@ impl<'a> Visitor<'a> for BytesVisitor { ); Ok(Bytes::new(Vec::new())) } else if value.len() >= 2 && &value[0..2] == "0x" && value.len() & 1 == 0 { - Ok(Bytes::new(FromHex::from_hex(&value[2..]).map_err(|_| Error::custom("invalid hex"))?)) + Ok(Bytes::new(FromHex::from_hex(&value[2..]).map_err(|e| Error::custom(format!("Invalid hex: {}", e)))?)) } else { - Err(Error::custom("invalid format")) + Err(Error::custom("Invalid bytes format. Expected a 0x-prefixed hex string with even length")) } } diff --git a/rpc/src/v1/types/derivation.rs b/rpc/src/v1/types/derivation.rs index cf0f03c85..76becbaeb 100644 --- a/rpc/src/v1/types/derivation.rs +++ b/rpc/src/v1/types/derivation.rs @@ -121,7 +121,7 @@ impl<'a> Visitor<'a> for DerivationTypeVisitor { match value { "soft" => Ok(DerivationType::Soft), "hard" => Ok(DerivationType::Hard), - _ => Err(Error::custom("invalid derivation type")), + v => Err(Error::custom(format!("invalid derivation type: {:?}", v))), } } diff --git a/rpc/src/v1/types/filter.rs b/rpc/src/v1/types/filter.rs index 8a1181eb8..058d14d7e 100644 --- a/rpc/src/v1/types/filter.rs +++ b/rpc/src/v1/types/filter.rs @@ -43,7 +43,7 @@ impl<'a, T> Deserialize<'a> for VariadicValue where T: DeserializeOwned { from_value(v.clone()).map(VariadicValue::Single) .or_else(|_| from_value(v).map(VariadicValue::Multiple)) - .map_err(|_| D::Error::custom("Invalid type.")) + .map_err(|err| D::Error::custom(format!("Invalid variadic value type: {}", err))) } } diff --git a/rpc/src/v1/types/index.rs b/rpc/src/v1/types/index.rs index 3c9d56f6e..4e44ce49c 100644 --- a/rpc/src/v1/types/index.rs +++ b/rpc/src/v1/types/index.rs @@ -47,8 +47,12 @@ impl<'a> Visitor<'a> for IndexVisitor { fn visit_str(self, value: &str) -> Result where E: Error { match value { - _ if value.starts_with("0x") => usize::from_str_radix(&value[2..], 16).map(Index).map_err(|_| Error::custom("invalid index")), - _ => value.parse::().map(Index).map_err(|_| Error::custom("invalid index")), + _ if value.starts_with("0x") => usize::from_str_radix(&value[2..], 16).map(Index).map_err(|e| { + Error::custom(format!("Invalid index: {}", e)) + }), + _ => value.parse::().map(Index).map_err(|e| { + Error::custom(format!("Invalid index: {}", e)) + }), } } diff --git a/rpc/src/v1/types/pubsub.rs b/rpc/src/v1/types/pubsub.rs index ce9d6d38d..e7deeba4f 100644 --- a/rpc/src/v1/types/pubsub.rs +++ b/rpc/src/v1/types/pubsub.rs @@ -84,7 +84,7 @@ impl<'a> Deserialize<'a> for Params { } from_value(v.clone()).map(Params::Logs) - .map_err(|_| D::Error::custom("Invalid type.")) + .map_err(|e| D::Error::custom(format!("Invalid Pub-Sub parameters: {}", e))) } }