From d7d73cc313be66ce32e953a79d3ebf04abf3f64d Mon Sep 17 00:00:00 2001 From: Nurzhan Saken Date: Thu, 19 Sep 2024 12:48:35 +0400 Subject: [PATCH] refactor: make query cursor errors more specific (#5086) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nurzhan Sakén --- .../integration/queries/smart_contract.rs | 4 +- crates/iroha_core/src/query/cursor.rs | 39 ++++++++++++------- crates/iroha_core/src/query/store.rs | 21 ++++++---- crates/iroha_data_model/src/query/mod.rs | 14 ++++--- crates/iroha_torii/src/lib.rs | 9 +++-- docs/source/references/schema.json | 16 ++++++-- 6 files changed, 67 insertions(+), 36 deletions(-) diff --git a/crates/iroha/tests/integration/queries/smart_contract.rs b/crates/iroha/tests/integration/queries/smart_contract.rs index 698b148845..a00001a914 100644 --- a/crates/iroha/tests/integration/queries/smart_contract.rs +++ b/crates/iroha/tests/integration/queries/smart_contract.rs @@ -33,9 +33,7 @@ fn live_query_is_dropped_after_smart_contract_end() -> Result<()> { assert!(matches!( err, - QueryError::Validation(ValidationFail::QueryFailed( - QueryExecutionFail::UnknownCursor - )) + QueryError::Validation(ValidationFail::QueryFailed(QueryExecutionFail::NotFound)) )); Ok(()) diff --git a/crates/iroha_core/src/query/cursor.rs b/crates/iroha_core/src/query/cursor.rs index db2378f0f2..cd572ead5c 100644 --- a/crates/iroha_core/src/query/cursor.rs +++ b/crates/iroha_core/src/query/cursor.rs @@ -2,16 +2,34 @@ use std::{fmt::Debug, num::NonZeroU64}; -use derive_more::Display; use iroha_data_model::query::QueryOutputBatchBox; use parity_scale_codec::{Decode, Encode}; use serde::{Deserialize, Serialize}; +/// An error with cursor processing. +#[derive( + Debug, + displaydoc::Display, + thiserror::Error, + Copy, + Clone, + Serialize, + Deserialize, + Encode, + Decode, +)] +pub enum Error { + /// The server's cursor does not match the provided cursor. + Mismatch, + /// There aren't enough items to proceed. + Done, +} + trait BatchedTrait { fn next_batch( &mut self, cursor: u64, - ) -> Result<(QueryOutputBatchBox, Option), UnknownCursor>; + ) -> Result<(QueryOutputBatchBox, Option), Error>; fn remaining(&self) -> u64; } @@ -29,15 +47,15 @@ where fn next_batch( &mut self, cursor: u64, - ) -> Result<(QueryOutputBatchBox, Option), UnknownCursor> { + ) -> Result<(QueryOutputBatchBox, Option), Error> { let Some(server_cursor) = self.cursor else { // the server is done with the iterator - return Err(UnknownCursor); + return Err(Error::Done); }; if cursor != server_cursor { // the cursor doesn't match - return Err(UnknownCursor); + return Err(Error::Mismatch); } let expected_batch_size: usize = self @@ -83,13 +101,6 @@ where } } -/// Unknown cursor error. -/// -/// Happens when client sends a cursor that doesn't match any server's cursor. -#[derive(Debug, Display, thiserror::Error, Copy, Clone, Serialize, Deserialize, Encode, Decode)] -#[display(fmt = "Unknown cursor")] -pub struct UnknownCursor; - /// A query output iterator that combines batching and type erasure. pub struct QueryBatchedErasedIterator { inner: Box, @@ -126,11 +137,11 @@ impl QueryBatchedErasedIterator { /// # Errors /// /// - The cursor doesn't match the server's cursor. - /// - The iterator is drained. + /// - There aren't enough items for the cursor. pub fn next_batch( &mut self, cursor: u64, - ) -> Result<(QueryOutputBatchBox, Option), UnknownCursor> { + ) -> Result<(QueryOutputBatchBox, Option), Error> { self.inner.next_batch(cursor) } diff --git a/crates/iroha_core/src/query/store.rs b/crates/iroha_core/src/query/store.rs index 771b8e5c3e..fb5e38dc81 100644 --- a/crates/iroha_core/src/query/store.rs +++ b/crates/iroha_core/src/query/store.rs @@ -22,7 +22,7 @@ use parity_scale_codec::{Decode, Encode}; use serde::{Deserialize, Serialize}; use tokio::task::JoinHandle; -use super::cursor::{QueryBatchedErasedIterator, UnknownCursor}; +use super::cursor::{Error as CursorError, QueryBatchedErasedIterator}; /// Query service error. #[derive( @@ -37,12 +37,14 @@ use super::cursor::{QueryBatchedErasedIterator, UnknownCursor}; Decode, )] pub enum Error { - /// Unknown cursor error. + /// Query not found in the live query store. + NotFound, + /// Cursor error. #[error(transparent)] - UnknownCursor(#[from] UnknownCursor), + Cursor(#[from] CursorError), /// Fetch size is too big. FetchSizeTooBig, - /// Reached limit of parallel queries. Either wait for previous queries to complete, or increase the limit in the config. + /// Reached the limit of parallel queries. Either wait for previous queries to complete, or increase the limit in the config. CapacityLimit, } @@ -50,7 +52,11 @@ pub enum Error { impl From for QueryExecutionFail { fn from(error: Error) -> Self { match error { - Error::UnknownCursor(_) => QueryExecutionFail::UnknownCursor, + Error::NotFound => QueryExecutionFail::NotFound, + Error::Cursor(error) => match error { + CursorError::Mismatch => QueryExecutionFail::CursorMismatch, + CursorError::Done => QueryExecutionFail::CursorDone, + }, Error::FetchSizeTooBig => QueryExecutionFail::FetchSizeTooBig, Error::CapacityLimit => QueryExecutionFail::CapacityLimit, } @@ -204,7 +210,7 @@ impl LiveQueryStore { mut live_query, authority, .. - } = self.remove(&query_id).ok_or(UnknownCursor)?; + } = self.remove(&query_id).ok_or(Error::NotFound)?; let (next_batch, next_cursor) = live_query.next_batch(cursor.get())?; let remaining = live_query.remaining(); if next_cursor.is_some() { @@ -278,7 +284,8 @@ impl LiveQueryStoreHandle { /// /// # Errors /// - /// - Returns [`Error::UnknownCursor`] if query id not found or cursor position doesn't match. + /// - Returns an [`Error`] if the query id is not found, + /// or if cursor position doesn't match or cannot continue. pub fn handle_iter_continue( &self, ForwardCursor { query, cursor }: ForwardCursor, diff --git a/crates/iroha_data_model/src/query/mod.rs b/crates/iroha_data_model/src/query/mod.rs index a83af27936..40d0f3970b 100644 --- a/crates/iroha_data_model/src/query/mod.rs +++ b/crates/iroha_data_model/src/query/mod.rs @@ -12,7 +12,7 @@ use iroha_macro::FromVariant; use iroha_primitives::{json::JsonString, numeric::Numeric}; use iroha_schema::IntoSchema; use iroha_version::prelude::*; -use parameters::{ForwardCursor, QueryParams, MAX_FETCH_SIZE}; +use parameters::{ForwardCursor, QueryParams}; use parity_scale_codec::{Decode, Encode}; use serde::{Deserialize, Serialize}; @@ -58,7 +58,6 @@ pub trait Query: Sealed { #[model] mod model { - use getset::Getters; use iroha_crypto::HashOf; @@ -1001,6 +1000,7 @@ pub mod error { #[model] mod model { use super::*; + use crate::query::parameters::MAX_FETCH_SIZE; /// Query errors. #[derive( @@ -1029,13 +1029,17 @@ pub mod error { #[skip_try_from] String, ), - /// Unknown query cursor - UnknownCursor, + /// Query not found in the live query store. + NotFound, + /// The server's cursor does not match the provided cursor. + CursorMismatch, + /// There aren't enough items for the cursor to proceed. + CursorDone, /// fetch_size could not be greater than {MAX_FETCH_SIZE:?} FetchSizeTooBig, /// Some of the specified parameters (filter/pagination/fetch_size/sorting) are not applicable to singular queries InvalidSingularParameters, - /// Reached limit of parallel queries. Either wait for previous queries to complete, or increase the limit in the config. + /// Reached the limit of parallel queries. Either wait for previous queries to complete, or increase the limit in the config. CapacityLimit, } diff --git a/crates/iroha_torii/src/lib.rs b/crates/iroha_torii/src/lib.rs index d3d6c87527..2ff9261d9b 100644 --- a/crates/iroha_torii/src/lib.rs +++ b/crates/iroha_torii/src/lib.rs @@ -381,9 +381,12 @@ impl Error { QueryFailed(query_error) | InstructionFailed(InstructionExecutionError::Query(query_error)) => match query_error { - Conversion(_) | UnknownCursor | FetchSizeTooBig | InvalidSingularParameters => { - StatusCode::BAD_REQUEST - } + Conversion(_) + | CursorMismatch + | CursorDone + | NotFound + | FetchSizeTooBig + | InvalidSingularParameters => StatusCode::BAD_REQUEST, Find(_) => StatusCode::NOT_FOUND, CapacityLimit => StatusCode::TOO_MANY_REQUESTS, }, diff --git a/docs/source/references/schema.json b/docs/source/references/schema.json index 6088c45dd7..f78dfa719b 100644 --- a/docs/source/references/schema.json +++ b/docs/source/references/schema.json @@ -3080,20 +3080,28 @@ "type": "String" }, { - "tag": "UnknownCursor", + "tag": "NotFound", "discriminant": 2 }, { - "tag": "FetchSizeTooBig", + "tag": "CursorMismatch", "discriminant": 3 }, { - "tag": "InvalidSingularParameters", + "tag": "CursorDone", "discriminant": 4 }, { - "tag": "CapacityLimit", + "tag": "FetchSizeTooBig", "discriminant": 5 + }, + { + "tag": "InvalidSingularParameters", + "discriminant": 6 + }, + { + "tag": "CapacityLimit", + "discriminant": 7 } ] },