Skip to content

Commit

Permalink
refactor: make query cursor errors more specific (#5086)
Browse files Browse the repository at this point in the history
Signed-off-by: Nurzhan Sakén <[email protected]>
  • Loading branch information
nxsaken committed Sep 19, 2024
1 parent e680397 commit d7d73cc
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 36 deletions.
4 changes: 1 addition & 3 deletions crates/iroha/tests/integration/queries/smart_contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
39 changes: 25 additions & 14 deletions crates/iroha_core/src/query/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<NonZeroU64>), UnknownCursor>;
) -> Result<(QueryOutputBatchBox, Option<NonZeroU64>), Error>;
fn remaining(&self) -> u64;
}

Expand All @@ -29,15 +47,15 @@ where
fn next_batch(
&mut self,
cursor: u64,
) -> Result<(QueryOutputBatchBox, Option<NonZeroU64>), UnknownCursor> {
) -> Result<(QueryOutputBatchBox, Option<NonZeroU64>), 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
Expand Down Expand Up @@ -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<dyn BatchedTrait + Send + Sync>,
Expand Down Expand Up @@ -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<NonZeroU64>), UnknownCursor> {
) -> Result<(QueryOutputBatchBox, Option<NonZeroU64>), Error> {
self.inner.next_batch(cursor)
}

Expand Down
21 changes: 14 additions & 7 deletions crates/iroha_core/src/query/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -37,20 +37,26 @@ 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,
}

#[allow(clippy::fallible_impl_from)]
impl From<Error> 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,
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 9 additions & 5 deletions crates/iroha_data_model/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -58,7 +58,6 @@ pub trait Query: Sealed {

#[model]
mod model {

use getset::Getters;
use iroha_crypto::HashOf;

Expand Down Expand Up @@ -1001,6 +1000,7 @@ pub mod error {
#[model]
mod model {
use super::*;
use crate::query::parameters::MAX_FETCH_SIZE;

/// Query errors.
#[derive(
Expand Down Expand Up @@ -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,
}

Expand Down
9 changes: 6 additions & 3 deletions crates/iroha_torii/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
16 changes: 12 additions & 4 deletions docs/source/references/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
]
},
Expand Down

0 comments on commit d7d73cc

Please sign in to comment.