From 1e4592dd7e0dead0ebb6a06b6fc2af7187b10e7b Mon Sep 17 00:00:00 2001 From: marin postma Date: Mon, 21 Jun 2021 18:42:47 +0200 Subject: [PATCH] enable errors in updates --- meilisearch-http/src/error.rs | 98 ++++++------------- meilisearch-http/src/index/update_handler.rs | 2 +- .../src/index_controller/error.rs | 5 + meilisearch-http/src/index_controller/mod.rs | 2 +- .../update_actor/store/mod.rs | 2 +- .../src/index_controller/updates.rs | 12 +-- meilisearch-http/src/routes/mod.rs | 17 ++-- 7 files changed, 52 insertions(+), 86 deletions(-) diff --git a/meilisearch-http/src/error.rs b/meilisearch-http/src/error.rs index 161aa6e8c..42ddeca3a 100644 --- a/meilisearch-http/src/error.rs +++ b/meilisearch-http/src/error.rs @@ -1,4 +1,3 @@ -use std::error; use std::error::Error; use std::fmt; @@ -8,9 +7,7 @@ use actix_web::dev::BaseHttpResponseBuilder; use actix_web::http::StatusCode; use meilisearch_error::{Code, ErrorCode}; use milli::UserError; -use serde::ser::{Serialize, SerializeStruct, Serializer}; - -use crate::index_controller::error::IndexControllerError; +use serde::{Serialize, Deserialize}; #[derive(Debug, thiserror::Error)] pub enum AuthenticationError { @@ -29,56 +26,34 @@ impl ErrorCode for AuthenticationError { } } -#[derive(Debug)] +#[derive(Debug, Serialize, Deserialize, Clone)] +#[serde(rename_all = "camelCase")] pub struct ResponseError { - inner: Box, -} - -impl error::Error for ResponseError {} - -impl ErrorCode for ResponseError { - fn error_code(&self) -> Code { - self.inner.error_code() - } + #[serde(skip)] + code: StatusCode, + message: String, + error_code: String, + error_type: String, + error_link: String, } impl fmt::Display for ResponseError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) + self.message.fmt(f) } } -macro_rules! response_error { - ($($other:path), *) => { - $( - impl From<$other> for ResponseError { - fn from(error: $other) -> ResponseError { - ResponseError { - inner: Box::new(error), - } - } - } - - )* - }; -} - -response_error!(IndexControllerError, AuthenticationError); - -impl Serialize for ResponseError { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let struct_name = "ResponseError"; - let field_count = 4; - - let mut state = serializer.serialize_struct(struct_name, field_count)?; - state.serialize_field("message", &self.to_string())?; - state.serialize_field("errorCode", &self.error_name())?; - state.serialize_field("errorType", &self.error_type())?; - state.serialize_field("errorLink", &self.error_url())?; - state.end() +impl From for ResponseError +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(), + } } } @@ -89,7 +64,7 @@ impl aweb::error::ResponseError for ResponseError { } fn status_code(&self) -> StatusCode { - self.http_status() + self.code } } @@ -110,25 +85,6 @@ impl ErrorCode for PayloadError { } } -impl From> for ResponseError -where - E: Error + Sync + Send + 'static, -{ - fn from(other: PayloadError) -> Self { - ResponseError { - inner: Box::new(other), - } - } -} - -pub fn payload_error_handler(err: E) -> ResponseError -where - E: Error + Sync + Send + 'static, -{ - let error = PayloadError(err); - error.into() -} - macro_rules! internal_error { ($target:ty : $($other:path), *) => { $( @@ -168,7 +124,7 @@ impl ErrorCode for MilliError<'_> { | UserError::InvalidDocumentId { .. } | UserError::InvalidStoreFile | UserError::NoSpaceLeftOnDevice - | UserError::DocumentLimitReached => todo!(), + | UserError::DocumentLimitReached => Code::Internal, UserError::InvalidFilter(_) => Code::Filter, UserError::InvalidFilterAttribute(_) => Code::Filter, UserError::MissingDocumentId { .. } => Code::MissingDocumentId, @@ -181,3 +137,11 @@ impl ErrorCode for MilliError<'_> { } } } + +pub fn payload_error_handler(err: E) -> ResponseError +where + E: Error + Sync + Send + 'static, +{ + let error = PayloadError(err); + error.into() +} diff --git a/meilisearch-http/src/index/update_handler.rs b/meilisearch-http/src/index/update_handler.rs index 10b28bcc9..9896c9391 100644 --- a/meilisearch-http/src/index/update_handler.rs +++ b/meilisearch-http/src/index/update_handler.rs @@ -86,7 +86,7 @@ impl UpdateHandler { match result { Ok(result) => Ok(meta.process(result)), - Err(e) => Err(meta.fail(e.to_string())), + Err(e) => Err(meta.fail(e.into())), } } } diff --git a/meilisearch-http/src/index_controller/error.rs b/meilisearch-http/src/index_controller/error.rs index 956a755ab..c01eb24c0 100644 --- a/meilisearch-http/src/index_controller/error.rs +++ b/meilisearch-http/src/index_controller/error.rs @@ -1,6 +1,8 @@ use meilisearch_error::Code; use meilisearch_error::ErrorCode; +use crate::index::error::IndexError; + use super::dump_actor::error::DumpActorError; use super::index_actor::error::IndexActorError; use super::update_actor::error::UpdateActorError; @@ -20,6 +22,8 @@ pub enum IndexControllerError { UpdateActor(#[from] UpdateActorError), #[error("error with dump: {0}")] DumpActor(#[from] DumpActorError), + #[error("error with index: {0}")] + IndexError(#[from] IndexError), } impl ErrorCode for IndexControllerError { @@ -30,6 +34,7 @@ impl ErrorCode for IndexControllerError { IndexControllerError::IndexActor(e) => e.error_code(), IndexControllerError::UpdateActor(e) => e.error_code(), IndexControllerError::DumpActor(e) => e.error_code(), + IndexControllerError::IndexError(e) => e.error_code(), } } } diff --git a/meilisearch-http/src/index_controller/mod.rs b/meilisearch-http/src/index_controller/mod.rs index 1d1a8f1cc..b51a6c5b0 100644 --- a/meilisearch-http/src/index_controller/mod.rs +++ b/meilisearch-http/src/index_controller/mod.rs @@ -30,7 +30,7 @@ use self::error::IndexControllerError; mod dump_actor; pub mod error; -mod index_actor; +pub mod index_actor; mod snapshot; mod update_actor; mod updates; diff --git a/meilisearch-http/src/index_controller/update_actor/store/mod.rs b/meilisearch-http/src/index_controller/update_actor/store/mod.rs index 6aa336835..67329b054 100644 --- a/meilisearch-http/src/index_controller/update_actor/store/mod.rs +++ b/meilisearch-http/src/index_controller/update_actor/store/mod.rs @@ -332,7 +332,7 @@ impl UpdateStore { let result = match handle.block_on(index_handle.update(index_uuid, processing.clone(), file)) { Ok(result) => result, - Err(e) => Err(processing.fail(e.to_string())), + Err(e) => Err(processing.fail(e.into())), }; // Once the pending update have been successfully processed diff --git a/meilisearch-http/src/index_controller/updates.rs b/meilisearch-http/src/index_controller/updates.rs index 296ef9b42..13ad9f7e5 100644 --- a/meilisearch-http/src/index_controller/updates.rs +++ b/meilisearch-http/src/index_controller/updates.rs @@ -3,9 +3,7 @@ use milli::update::{DocumentAdditionResult, IndexDocumentsMethod, UpdateFormat}; use serde::{Deserialize, Serialize}; use uuid::Uuid; -use crate::index::{Settings, Unchecked}; - -pub type UpdateError = String; +use crate::{error::ResponseError, index::{Settings, Unchecked}}; #[derive(Debug, Clone, Serialize, Deserialize)] pub enum UpdateResult { @@ -116,7 +114,7 @@ impl Processing { } } - pub fn fail(self, error: UpdateError) -> Failed { + pub fn fail(self, error: ResponseError) -> Failed { Failed { from: self, error, @@ -143,12 +141,12 @@ impl Aborted { } } -#[derive(Debug, Serialize, Deserialize, Clone)] +#[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Failed { #[serde(flatten)] pub from: Processing, - pub error: UpdateError, + pub error: ResponseError, pub failed_at: DateTime, } @@ -162,7 +160,7 @@ impl Failed { } } -#[derive(Debug, Serialize, Deserialize, Clone)] +#[derive(Debug, Serialize, Deserialize)] #[serde(tag = "status", rename_all = "camelCase")] pub enum UpdateStatus { Processing(Processing), diff --git a/meilisearch-http/src/routes/mod.rs b/meilisearch-http/src/routes/mod.rs index a896653ef..f4581ebcb 100644 --- a/meilisearch-http/src/routes/mod.rs +++ b/meilisearch-http/src/routes/mod.rs @@ -4,6 +4,7 @@ use actix_web::{get, HttpResponse}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; +use crate::error::ResponseError; use crate::index::{Settings, Unchecked}; use crate::index_controller::{UpdateMeta, UpdateResult, UpdateStatus}; @@ -89,10 +90,8 @@ pub struct FailedUpdateResult { pub update_id: u64, #[serde(rename = "type")] pub update_type: UpdateType, - pub error: String, - pub error_type: String, - pub error_code: String, - pub error_link: String, + #[serde(flatten)] + pub response: ResponseError, pub duration: f64, // in seconds pub enqueued_at: DateTime, pub processed_at: DateTime, @@ -181,13 +180,13 @@ impl From for UpdateStatusResponse { // necessary since chrono::duration don't expose a f64 secs method. let duration = Duration::from_millis(duration as u64).as_secs_f64(); + let update_id = failed.id(); + let response = failed.error; + let content = FailedUpdateResult { - update_id: failed.id(), + update_id, update_type, - error: failed.error, - error_type: String::from("todo"), - error_code: String::from("todo"), - error_link: String::from("todo"), + response, duration, enqueued_at: failed.from.from.enqueued_at, processed_at: failed.failed_at,