diff --git a/Cargo.lock b/Cargo.lock index b0ae44b2286..09099a1f9e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1749,6 +1749,7 @@ dependencies = [ "smallvec", "tempfile", "test_network", + "thiserror", "tokio", "tungstenite 0.16.0", ] diff --git a/client/Cargo.toml b/client/Cargo.toml index 0bc53d7ba2e..e313c253529 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -23,6 +23,7 @@ iroha_data_model = { version = "=2.0.0-pre-rc.4", path = "../data_model", featur iroha_logger = { version = "=2.0.0-pre-rc.4", path = "../logger"} iroha_telemetry = { version ="=2.0.0-pre-rc.4", path = "../telemetry" } iroha_version = { version = "=2.0.0-pre-rc.4", path = "../version" } +iroha_core = { version = "=2.0.0-pre-rc.4", path = "../core", features = ["dev-telemetry", "telemetry"]} attohttpc = "0.18" eyre = "0.6.5" @@ -34,10 +35,9 @@ tungstenite = { version = "0.16" } smallvec = { version = "1.8.0", features = ["serde", "union"] } smallstr = { version = "0.3.0", default-features = false, features = ["serde", "union"] } base64 = "0.13.0" - +thiserror = "1.0.30" [dev-dependencies] -iroha_core = { version = "=2.0.0-pre-rc.4", path = "../core", features = ["dev-telemetry", "telemetry"]} iroha_permissions_validators = { version = "=2.0.0-pre-rc.4", path = "../permissions_validators" } iroha = { path = "../cli", features = ["dev-telemetry", "telemetry"] } diff --git a/client/src/client.rs b/client/src/client.rs index 91e3ec5491c..fab3067e10e 100644 --- a/client/src/client.rs +++ b/client/src/client.rs @@ -12,6 +12,7 @@ use std::{ use eyre::{eyre, Result, WrapErr}; use http_default::WebSocketStream; use iroha_config::{GetConfiguration, PostConfiguration}; +use iroha_core::smartcontracts::isi::query::Error as QueryError; use iroha_crypto::{HashOf, KeyPair}; use iroha_data_model::{prelude::*, query::SignedQueryRequest}; use iroha_logger::prelude::*; @@ -32,11 +33,8 @@ pub trait ResponseHandler> { /// What is the output of the handler type Output; - /// Function to parse HTTP response with body `T` to output `O` - /// - /// # Errors - /// Implementation dependent. - fn handle(self, response: Response) -> Result; + /// Handles HTTP response + fn handle(self, response: Response) -> Self::Output; } /// Phantom struct that handles responses of Query API. @@ -50,21 +48,68 @@ impl Default for QueryResponseHandler { } } +/// `Result` with [`ClientQueryError`] as an error +pub type QueryHandlerResult = core::result::Result; + impl ResponseHandler for QueryResponseHandler where R: Query + Into + Debug, >::Error: Into, { - type Output = PaginatedQueryOutput; - - fn handle(self, resp: Response>) -> Result> { - if resp.status() != StatusCode::OK { - return Err(ResponseReport::with_msg("Failed to make query request", &resp).into()); + type Output = QueryHandlerResult>; + + fn handle(self, resp: Response>) -> Self::Output { + // Separate-compilation friendly response handling + fn _handle_query_response_base( + resp: &Response>, + ) -> QueryHandlerResult { + match resp.status() { + StatusCode::OK => VersionedPaginatedQueryResult::decode_versioned(resp.body()) + .wrap_err("Failed to decode response body as VersionedPaginatedQueryResult") + .map_err(Into::into), + StatusCode::BAD_REQUEST + | StatusCode::UNAUTHORIZED + | StatusCode::FORBIDDEN + | StatusCode::NOT_FOUND => { + let err = QueryError::decode(&mut resp.body().as_ref()) + .wrap_err("Failed to decode response body as QueryError")?; + Err(ClientQueryError::QueryError(err)) + } + _ => Err(ResponseReport::with_msg("Unexpected query response", resp).into()), + } } - let result = VersionedPaginatedQueryResult::decode_versioned(resp.body())?; - let VersionedPaginatedQueryResult::V1(result) = result; - PaginatedQueryOutput::try_from(result) + _handle_query_response_base(&resp).and_then(|VersionedPaginatedQueryResult::V1(result)| { + ClientQueryOutput::try_from(result).map_err(Into::into) + }) + } +} + +/// Different errors as a result of query response handling +#[derive(Debug, thiserror::Error)] +// `QueryError` variant is too large (32 bytes), but I think that this enum is not +// very frequently constructed, so boxing here is unnecessary. +#[allow(variant_size_differences)] +pub enum ClientQueryError { + /// Certain Iroha query error + #[error("Query error: {0}")] + QueryError(QueryError), + /// Some other error + #[error("Other error: {0}")] + Other(eyre::Error), +} + +impl From for ClientQueryError { + #[inline] + fn from(err: eyre::Error) -> Self { + Self::Other(err) + } +} + +impl From for ClientQueryError { + #[inline] + fn from(ResponseReport(err): ResponseReport) -> Self { + Self::Other(err) } } @@ -73,13 +118,13 @@ where pub struct TransactionResponseHandler; impl ResponseHandler for TransactionResponseHandler { - type Output = (); + type Output = Result<()>; - fn handle(self, resp: Response>) -> Result<()> { + fn handle(self, resp: Response>) -> Self::Output { if resp.status() == StatusCode::OK { Ok(()) } else { - Err(ResponseReport::with_msg("Failed to submit instructions", &resp).into()) + Err(ResponseReport::with_msg("Unexpected transaction response", &resp).into()) } } } @@ -89,11 +134,11 @@ impl ResponseHandler for TransactionResponseHandler { pub struct StatusResponseHandler; impl ResponseHandler for StatusResponseHandler { - type Output = Status; + type Output = Result; - fn handle(self, resp: Response>) -> Result { + fn handle(self, resp: Response>) -> Self::Output { if resp.status() != StatusCode::OK { - return Err(ResponseReport::with_msg("Failed to get status", &resp).into()); + return Err(ResponseReport::with_msg("Unexpected status response", &resp).into()); } serde_json::from_slice(resp.body()).wrap_err("Failed to decode body") } @@ -117,6 +162,7 @@ impl ResponseReport { } impl From for eyre::Report { + #[inline] fn from(report: ResponseReport) -> Self { report.0 } @@ -125,7 +171,8 @@ impl From for eyre::Report { /// More convenient version of [`iroha_data_model::prelude::PaginatedQueryResult`]. /// The only difference is that this struct has `output` field extracted from the result /// accordingly to the source query. -pub struct PaginatedQueryOutput +#[derive(Clone, Debug)] +pub struct ClientQueryOutput where R: Query + Into + Debug, >::Error: Into, @@ -138,7 +185,7 @@ where pub total: u64, } -impl PaginatedQueryOutput +impl ClientQueryOutput where R: Query + Into + Debug, >::Error: Into, @@ -149,7 +196,7 @@ where } } -impl TryFrom for PaginatedQueryOutput +impl TryFrom for ClientQueryOutput where R: Query + Into + Debug, >::Error: Into, @@ -569,7 +616,7 @@ impl Client { &self, request: R, pagination: Pagination, - ) -> Result> + ) -> QueryHandlerResult> where R: Query + Into + Debug, >::Error: Into, @@ -585,13 +632,13 @@ impl Client { /// # Errors /// Fails if sending request fails #[log] - pub fn request(&self, request: R) -> Result + pub fn request(&self, request: R) -> QueryHandlerResult where R: Query + Into + Debug, >::Error: Into, { self.request_with_pagination(request, Pagination::default()) - .map(PaginatedQueryOutput::only_output) + .map(ClientQueryOutput::only_output) } /// Connects through `WebSocket` to listen for `Iroha` pipeline and data events. @@ -1011,4 +1058,65 @@ mod tests { let expected_value = format!("Basic {}", ENCRYPTED_CREDENTIALS); assert_eq!(value, &expected_value); } + + #[cfg(test)] + mod query_errors_handling { + use http::Response; + use iroha_core::smartcontracts::isi::query::UnsupportedVersionError; + + use super::*; + + #[test] + fn certain_errors() -> Result<()> { + let sut = QueryResponseHandler::::default(); + let responses = vec![ + ( + StatusCode::BAD_REQUEST, + QueryError::Version(UnsupportedVersionError { version: 19 }), + ), + ( + StatusCode::UNAUTHORIZED, + QueryError::Signature("whatever".to_owned()), + ), + ( + StatusCode::FORBIDDEN, + QueryError::Permission("whatever".to_owned()), + ), + ( + StatusCode::NOT_FOUND, + // Here should be `Find`, but actually handler doesn't care + QueryError::Evaluate("whatever".to_owned()), + ), + ]; + + for (status_code, err) in responses { + let resp = Response::builder() + .status(status_code) + .body(err.clone().encode())?; + + match sut.handle(resp) { + Err(ClientQueryError::QueryError(actual)) => { + // PartialEq isn't implemented, so asserting by encoded repr + assert_eq!(actual.encode(), err.encode()); + } + x => return Err(eyre!("Wrong output for {:?}: {:?}", (status_code, err), x)), + } + } + + Ok(()) + } + + #[test] + fn indeterminate() -> Result<()> { + let sut = QueryResponseHandler::::default(); + let response = Response::builder() + .status(StatusCode::INTERNAL_SERVER_ERROR) + .body(Vec::::new())?; + + match sut.handle(response) { + Err(ClientQueryError::Other(_)) => Ok(()), + x => Err(eyre!("Expected indeterminate, found: {:?}", x)), + } + } + } } diff --git a/client/tests/integration/mod.rs b/client/tests/integration/mod.rs index a9c1636f281..7f9ad5ec1a0 100644 --- a/client/tests/integration/mod.rs +++ b/client/tests/integration/mod.rs @@ -15,6 +15,7 @@ mod non_mintable; mod offline_peers; mod pagination; mod permissions; +mod query_errors; mod restart_peer; mod roles; mod transfer_asset; diff --git a/client/tests/integration/query_errors.rs b/client/tests/integration/query_errors.rs new file mode 100644 index 00000000000..66ae11e4a26 --- /dev/null +++ b/client/tests/integration/query_errors.rs @@ -0,0 +1,27 @@ +#![allow(clippy::restriction)] + +use std::str::FromStr; + +use iroha_client::client::{self, ClientQueryError}; +use iroha_core::smartcontracts::{isi::query::Error as QueryError, FindError}; +use iroha_data_model::prelude::*; + +#[test] +fn non_existent_account_is_specific_error() { + let (_rt, _peer, client) = ::start_test_with_runtime(); + // we can not wait for genesis committment + + let err = client + .request(client::account::by_id( + AccountId::from_str("john_doe@regalia").unwrap(), + )) + .expect_err("Should error"); + + match err { + ClientQueryError::QueryError(QueryError::Find(err)) => match *err { + FindError::Domain(id) => assert_eq!(id.name.as_ref(), "regalia"), + x => panic!("FindError::Domain expected, got {:?}", x), + }, + x => panic!("Unexpected error: {:?}", x), + }; +}