From d8fb506c92de40625111968b5e1d40b22c662712 Mon Sep 17 00:00:00 2001 From: Tamo Date: Mon, 19 Dec 2022 20:50:40 +0100 Subject: [PATCH] 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;