From d8fb506c92de40625111968b5e1d40b22c662712 Mon Sep 17 00:00:00 2001 From: Tamo Date: Mon, 19 Dec 2022 20:50:40 +0100 Subject: [PATCH 1/6] handle most io error instead of tagging everything as an internal --- Cargo.lock | 2 + index-scheduler/src/error.rs | 12 ++--- meilisearch-types/Cargo.toml | 2 + meilisearch-types/src/document_formats.rs | 12 +++-- meilisearch-types/src/error.rs | 53 +++++++++++++++++++---- 5 files changed, 64 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bd24f7569..fb3c0daa2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2374,6 +2374,7 @@ dependencies = [ "csv", "either", "enum-iterator", + "file-store", "flate2", "fst", "insta", @@ -2386,6 +2387,7 @@ dependencies = [ "serde", "serde_json", "tar", + "tempfile", "thiserror", "time", "tokio", diff --git a/index-scheduler/src/error.rs b/index-scheduler/src/error.rs index cfbf7a25e..037f8a269 100644 --- a/index-scheduler/src/error.rs +++ b/index-scheduler/src/error.rs @@ -114,17 +114,17 @@ impl ErrorCode for Error { Error::Dump(e) => e.error_code(), Error::Milli(e) => e.error_code(), Error::ProcessBatchPanicked => Code::Internal, - // TODO: TAMO: are all these errors really internal? - Error::Heed(_) => Code::Internal, - Error::FileStore(_) => Code::Internal, - Error::IoError(_) => Code::Internal, - Error::Persist(_) => Code::Internal, + Error::Heed(e) => e.error_code(), + Error::HeedTransaction(e) => e.error_code(), + Error::FileStore(e) => e.error_code(), + Error::IoError(e) => e.error_code(), + Error::Persist(e) => e.error_code(), + // Irrecoverable errors Error::Anyhow(_) => Code::Internal, Error::CorruptedTaskQueue => Code::Internal, Error::CorruptedDump => Code::Internal, Error::TaskDatabaseUpdate(_) => Code::Internal, Error::CreateBatch(_) => Code::Internal, - Error::HeedTransaction(_) => Code::Internal, } } } diff --git a/meilisearch-types/Cargo.toml b/meilisearch-types/Cargo.toml index f265d442b..88b689af7 100644 --- a/meilisearch-types/Cargo.toml +++ b/meilisearch-types/Cargo.toml @@ -10,6 +10,7 @@ anyhow = "1.0.65" csv = "1.1.6" either = { version = "1.6.1", features = ["serde"] } enum-iterator = "1.1.3" +file-store = { path = "../file-store" } flate2 = "1.0.24" fst = "0.4.7" memmap2 = "0.5.7" @@ -20,6 +21,7 @@ roaring = { version = "0.10.0", features = ["serde"] } serde = { version = "1.0.145", features = ["derive"] } serde_json = "1.0.85" tar = "0.4.38" +tempfile = "3.3.0" thiserror = "1.0.30" time = { version = "0.3.7", features = ["serde-well-known", "formatting", "parsing", "macros"] } tokio = "1.0" diff --git a/meilisearch-types/src/document_formats.rs b/meilisearch-types/src/document_formats.rs index 5eee63afc..b68bb637b 100644 --- a/meilisearch-types/src/document_formats.rs +++ b/meilisearch-types/src/document_formats.rs @@ -13,7 +13,6 @@ use serde::{Deserialize, Deserializer}; use serde_json::error::Category; use crate::error::{Code, ErrorCode}; -use crate::internal_error; type Result = std::result::Result; @@ -36,6 +35,7 @@ impl fmt::Display for PayloadType { #[derive(Debug)] pub enum DocumentFormatError { + Io(io::Error), Internal(Box), MalformedPayload(Error, PayloadType), } @@ -43,6 +43,7 @@ pub enum DocumentFormatError { impl Display for DocumentFormatError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + Self::Io(e) => write!(f, "{e}"), Self::Internal(e) => write!(f, "An internal error has occurred: `{}`.", e), Self::MalformedPayload(me, b) => match me.borrow() { Error::Json(se) => { @@ -91,17 +92,22 @@ impl From<(PayloadType, Error)> for DocumentFormatError { } } +impl From for DocumentFormatError { + fn from(error: io::Error) -> Self { + Self::Io(error) + } +} + impl ErrorCode for DocumentFormatError { fn error_code(&self) -> Code { match self { + DocumentFormatError::Io(e) => e.error_code(), DocumentFormatError::Internal(_) => Code::Internal, DocumentFormatError::MalformedPayload(_, _) => Code::MalformedPayload, } } } -internal_error!(DocumentFormatError: io::Error); - /// Reads CSV from input and write an obkv batch to writer. pub fn read_csv(file: &File, writer: impl Write + Seek) -> Result { let mut builder = DocumentsBatchBuilder::new(writer); diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index 5c0e1d9b8..402390068 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -1,4 +1,4 @@ -use std::fmt; +use std::{fmt, io}; use actix_web::http::StatusCode; use actix_web::{self as aweb, HttpResponseBuilder}; @@ -113,6 +113,11 @@ impl fmt::Display for ErrorType { #[derive(Serialize, Deserialize, Debug, Clone, Copy)] pub enum Code { + // error related to your setup + IoError, + NoSpaceLeftOnDevice, + TooManyOpenFiles, + // index related error CreateIndex, IndexAlreadyExists, @@ -145,7 +150,6 @@ pub enum Code { InvalidToken, MissingAuthorizationHeader, MissingMasterKey, - NoSpaceLeftOnDevice, DumpNotFound, InvalidTaskDateFilter, InvalidTaskStatusesFilter, @@ -188,6 +192,15 @@ impl Code { use Code::*; match self { + // related to the setup + IoError => ErrCode::invalid("io_error", StatusCode::UNPROCESSABLE_ENTITY), + TooManyOpenFiles => { + ErrCode::invalid("too_many_open_files", StatusCode::UNPROCESSABLE_ENTITY) + } + NoSpaceLeftOnDevice => { + ErrCode::invalid("no_space_left_on_device", StatusCode::UNPROCESSABLE_ENTITY) + } + // index related errors // create index is thrown on internal error while creating an index. CreateIndex => { @@ -266,9 +279,6 @@ impl Code { ErrCode::invalid("missing_task_filters", StatusCode::BAD_REQUEST) } DumpNotFound => ErrCode::invalid("dump_not_found", StatusCode::NOT_FOUND), - NoSpaceLeftOnDevice => { - ErrCode::internal("no_space_left_on_device", StatusCode::INTERNAL_SERVER_ERROR) - } PayloadTooLarge => ErrCode::invalid("payload_too_large", StatusCode::PAYLOAD_TOO_LARGE), RetrieveDocument => { ErrCode::internal("unretrievable_document", StatusCode::BAD_REQUEST) @@ -380,7 +390,7 @@ impl ErrorCode for milli::Error { match self { Error::InternalError(_) => Code::Internal, - Error::IoError(_) => Code::Internal, + Error::IoError(e) => e.error_code(), Error::UserError(ref error) => { match error { // TODO: wait for spec for new error codes. @@ -415,13 +425,28 @@ impl ErrorCode for milli::Error { } } +impl ErrorCode for file_store::Error { + fn error_code(&self) -> Code { + match self { + Self::IoError(e) => e.error_code(), + Self::PersistError(e) => e.error_code(), + } + } +} + +impl ErrorCode for tempfile::PersistError { + fn error_code(&self) -> Code { + self.error.error_code() + } +} + impl ErrorCode for HeedError { fn error_code(&self) -> Code { match self { HeedError::Mdb(MdbError::MapFull) => Code::DatabaseSizeLimitReached, HeedError::Mdb(MdbError::Invalid) => Code::InvalidStore, - HeedError::Io(_) - | HeedError::Mdb(_) + HeedError::Io(e) => e.error_code(), + HeedError::Mdb(_) | HeedError::Encoding | HeedError::Decoding | HeedError::InvalidDatabaseTyping @@ -431,6 +456,18 @@ impl ErrorCode for HeedError { } } +impl ErrorCode for io::Error { + fn error_code(&self) -> Code { + match self.raw_os_error() { + Some(5) => Code::IoError, + Some(24) => Code::TooManyOpenFiles, + Some(28) => Code::NoSpaceLeftOnDevice, + e => todo!("missed something asshole {:?}", e), + // e => Code::Internal, + } + } +} + #[cfg(feature = "test-traits")] mod strategy { use proptest::strategy::Strategy; From 52aa34d984d0bd1ad6e26e0fa1cf83b68ace6bdf Mon Sep 17 00:00:00 2001 From: Tamo Date: Tue, 20 Dec 2022 16:32:51 +0100 Subject: [PATCH 2/6] remove an unused error handling file --- dump/src/error.rs | 6 ++--- dump/src/reader/error.rs | 42 ---------------------------------- meilisearch-types/src/error.rs | 3 +-- 3 files changed, 4 insertions(+), 47 deletions(-) delete mode 100644 dump/src/reader/error.rs diff --git a/dump/src/error.rs b/dump/src/error.rs index 0d57729ae..873a6bbfd 100644 --- a/dump/src/error.rs +++ b/dump/src/error.rs @@ -19,9 +19,9 @@ pub enum Error { impl ErrorCode for Error { fn error_code(&self) -> Code { match self { - // Are these three really Internal errors? - // TODO look at that later. - Error::Io(_) => Code::Internal, + Error::Io(e) => e.error_code(), + + // These error come from an internal mis Error::Serde(_) => Code::Internal, Error::Uuid(_) => Code::Internal, diff --git a/dump/src/reader/error.rs b/dump/src/reader/error.rs deleted file mode 100644 index 679fa2bc2..000000000 --- a/dump/src/reader/error.rs +++ /dev/null @@ -1,42 +0,0 @@ -use meilisearch_auth::error::AuthControllerError; -use meilisearch_types::error::{Code, ErrorCode}; -use meilisearch_types::internal_error; - -use crate::{index_resolver::error::IndexResolverError, tasks::error::TaskError}; - -pub type Result = std::result::Result; - -#[derive(thiserror::Error, Debug)] -pub enum DumpError { - #[error("An internal error has occurred. `{0}`.")] - Internal(Box), - #[error("{0}")] - IndexResolver(Box), -} - -internal_error!( - DumpError: milli::heed::Error, - std::io::Error, - tokio::task::JoinError, - tokio::sync::oneshot::error::RecvError, - serde_json::error::Error, - tempfile::PersistError, - fs_extra::error::Error, - AuthControllerError, - TaskError -); - -impl From for DumpError { - fn from(e: IndexResolverError) -> Self { - Self::IndexResolver(Box::new(e)) - } -} - -impl ErrorCode for DumpError { - fn error_code(&self) -> Code { - match self { - DumpError::Internal(_) => Code::Internal, - DumpError::IndexResolver(e) => e.error_code(), - } - } -} diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index 402390068..6e84378e2 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -462,8 +462,7 @@ impl ErrorCode for io::Error { Some(5) => Code::IoError, Some(24) => Code::TooManyOpenFiles, Some(28) => Code::NoSpaceLeftOnDevice, - e => todo!("missed something asshole {:?}", e), - // e => Code::Internal, + _ => Code::Internal, } } } From 304017256298f89020994097334d9a6f9cf2af8e Mon Sep 17 00:00:00 2001 From: Tamo Date: Tue, 20 Dec 2022 17:31:13 +0100 Subject: [PATCH 3/6] update the error message as well --- meilisearch-types/src/error.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index 6e84378e2..fb267abb6 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -23,7 +23,10 @@ pub struct ResponseError { } impl ResponseError { - pub fn from_msg(message: String, code: Code) -> Self { + pub fn from_msg(mut message: String, code: Code) -> Self { + if code == Code::IoError { + message.push_str(". This error generally happens when you have no space left on device or when your database doesn't have read or write right."); + } Self { code: code.http(), message, @@ -47,13 +50,7 @@ where T: ErrorCode, { fn from(other: T) -> Self { - Self { - code: other.http_status(), - message: other.to_string(), - error_code: other.error_name(), - error_type: other.error_type(), - error_link: other.error_url(), - } + Self::from_msg(other.to_string(), other.error_code()) } } @@ -111,7 +108,7 @@ impl fmt::Display for ErrorType { } } -#[derive(Serialize, Deserialize, Debug, Clone, Copy)] +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq)] pub enum Code { // error related to your setup IoError, From c637bfba3733c916a0a9e2b1cf5e6399cbc7fd16 Mon Sep 17 00:00:00 2001 From: Tamo Date: Tue, 20 Dec 2022 17:49:38 +0100 Subject: [PATCH 4/6] convert all the document format error due to io to io::Error --- dump/src/error.rs | 3 ++- meilisearch-types/src/document_formats.rs | 16 +++++----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/dump/src/error.rs b/dump/src/error.rs index 873a6bbfd..f4944bb97 100644 --- a/dump/src/error.rs +++ b/dump/src/error.rs @@ -21,7 +21,8 @@ impl ErrorCode for Error { match self { Error::Io(e) => e.error_code(), - // These error come from an internal mis + // These errors either happens when creating a dump and don't need any error code. + // These error come from a internal bad deserialization. Error::Serde(_) => Code::Internal, Error::Uuid(_) => Code::Internal, diff --git a/meilisearch-types/src/document_formats.rs b/meilisearch-types/src/document_formats.rs index b68bb637b..8011afac7 100644 --- a/meilisearch-types/src/document_formats.rs +++ b/meilisearch-types/src/document_formats.rs @@ -36,7 +36,6 @@ impl fmt::Display for PayloadType { #[derive(Debug)] pub enum DocumentFormatError { Io(io::Error), - Internal(Box), MalformedPayload(Error, PayloadType), } @@ -44,7 +43,6 @@ impl Display for DocumentFormatError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Io(e) => write!(f, "{e}"), - Self::Internal(e) => write!(f, "An internal error has occurred: `{}`.", e), Self::MalformedPayload(me, b) => match me.borrow() { Error::Json(se) => { let mut message = match se.classify() { @@ -86,7 +84,7 @@ impl std::error::Error for DocumentFormatError {} impl From<(PayloadType, Error)> for DocumentFormatError { fn from((ty, error): (PayloadType, Error)) -> Self { match error { - Error::Io(e) => Self::Internal(Box::new(e)), + Error::Io(e) => Self::Io(e), e => Self::MalformedPayload(e, ty), } } @@ -102,7 +100,6 @@ impl ErrorCode for DocumentFormatError { fn error_code(&self) -> Code { match self { DocumentFormatError::Io(e) => e.error_code(), - DocumentFormatError::Internal(_) => Code::Internal, DocumentFormatError::MalformedPayload(_, _) => Code::MalformedPayload, } } @@ -116,7 +113,7 @@ pub fn read_csv(file: &File, writer: impl Write + Seek) -> Result { builder.append_csv(csv).map_err(|e| (PayloadType::Csv, e))?; let count = builder.documents_count(); - let _ = builder.into_inner().map_err(Into::into).map_err(DocumentFormatError::Internal)?; + let _ = builder.into_inner().map_err(DocumentFormatError::Io)?; Ok(count as u64) } @@ -145,7 +142,7 @@ fn read_json_inner( // The json data has been deserialized and does not need to be processed again. // The data has been transferred to the writer during the deserialization process. Ok(Ok(_)) => (), - Ok(Err(e)) => return Err(DocumentFormatError::Internal(Box::new(e))), + Ok(Err(e)) => return Err(DocumentFormatError::Io(e)), Err(_e) => { // If we cannot deserialize the content as an array of object then we try // to deserialize it with the original method to keep correct error messages. @@ -161,16 +158,13 @@ fn read_json_inner( .map_err(|e| (payload_type, e))?; for object in content.inner.map_right(|o| vec![o]).into_inner() { - builder - .append_json_object(&object) - .map_err(Into::into) - .map_err(DocumentFormatError::Internal)?; + builder.append_json_object(&object).map_err(DocumentFormatError::Io)?; } } } let count = builder.documents_count(); - let _ = builder.into_inner().map_err(Into::into).map_err(DocumentFormatError::Internal)?; + let _ = builder.into_inner().map_err(DocumentFormatError::Io)?; Ok(count as u64) } From 336ea573841bc23f6e3f784ae91630593128619b Mon Sep 17 00:00:00 2001 From: Tamo Date: Tue, 20 Dec 2022 18:08:44 +0100 Subject: [PATCH 5/6] Update dump/src/error.rs Co-authored-by: Louis Dureuil --- dump/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dump/src/error.rs b/dump/src/error.rs index f4944bb97..77acbe348 100644 --- a/dump/src/error.rs +++ b/dump/src/error.rs @@ -21,7 +21,7 @@ impl ErrorCode for Error { match self { Error::Io(e) => e.error_code(), - // These errors either happens when creating a dump and don't need any error code. + // These errors either happen when creating a dump and don't need any error code, // These error come from a internal bad deserialization. Error::Serde(_) => Code::Internal, Error::Uuid(_) => Code::Internal, From 9e0cce5ca4e8452bdc3ae1d83d0543bfd57bfc4c Mon Sep 17 00:00:00 2001 From: Tamo Date: Tue, 20 Dec 2022 18:08:51 +0100 Subject: [PATCH 6/6] Update dump/src/error.rs Co-authored-by: Louis Dureuil --- dump/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dump/src/error.rs b/dump/src/error.rs index 77acbe348..b53dd640a 100644 --- a/dump/src/error.rs +++ b/dump/src/error.rs @@ -22,7 +22,7 @@ impl ErrorCode for Error { Error::Io(e) => e.error_code(), // These errors either happen when creating a dump and don't need any error code, - // These error come from a internal bad deserialization. + // or come from an internal bad deserialization. Error::Serde(_) => Code::Internal, Error::Uuid(_) => Code::Internal,