From 30a094cbb2dd75b5a67adc1d93f8733f414aaef8 Mon Sep 17 00:00:00 2001 From: many Date: Wed, 3 Nov 2021 14:25:49 +0100 Subject: [PATCH] Change lacking errors --- meilisearch-error/src/lib.rs | 33 ++++-- .../src/extractors/authentication/error.rs | 8 +- meilisearch-http/tests/common/index.rs | 104 ++++++++++++++---- meilisearch-http/tests/common/server.rs | 3 +- .../tests/documents/add_documents.rs | 12 +- meilisearch-http/tests/index/create_index.rs | 1 - meilisearch-lib/src/document_formats.rs | 2 +- meilisearch-lib/src/error.rs | 6 +- meilisearch-lib/src/index/error.rs | 2 +- .../src/index_controller/dump_actor/error.rs | 6 +- .../index_controller/dump_actor/loaders/v2.rs | 2 - meilisearch-lib/src/index_controller/error.rs | 2 +- .../index_controller/index_resolver/error.rs | 6 +- .../src/index_controller/updates/error.rs | 2 +- 14 files changed, 128 insertions(+), 61 deletions(-) diff --git a/meilisearch-error/src/lib.rs b/meilisearch-error/src/lib.rs index c6a62aa70..04c6391f9 100644 --- a/meilisearch-error/src/lib.rs +++ b/meilisearch-error/src/lib.rs @@ -53,7 +53,6 @@ pub enum Code { IndexAlreadyExists, IndexNotFound, InvalidIndexUid, - OpenIndex, // invalid state error InvalidState, @@ -70,13 +69,16 @@ pub enum Code { BadParameter, BadRequest, + DatabaseSizeLimitReached, DocumentNotFound, Internal, InvalidGeoField, InvalidRankingRule, + InvalidStore, InvalidToken, MissingAuthorizationHeader, - NotFound, + NoSpaceLeftOnDevice, + DumpNotFound, TaskNotFound, PayloadTooLarge, RetrieveDocument, @@ -100,14 +102,13 @@ impl Code { match self { // index related errors // create index is thrown on internal error while creating an index. - CreateIndex => ErrCode::internal("index_creation_failed", StatusCode::BAD_REQUEST), + CreateIndex => { + ErrCode::internal("index_creation_failed", StatusCode::INTERNAL_SERVER_ERROR) + } IndexAlreadyExists => ErrCode::invalid("index_already_exists", StatusCode::CONFLICT), // thrown when requesting an unexisting index IndexNotFound => ErrCode::invalid("index_not_found", StatusCode::NOT_FOUND), InvalidIndexUid => ErrCode::invalid("invalid_index_uid", StatusCode::BAD_REQUEST), - OpenIndex => { - ErrCode::internal("index_not_accessible", StatusCode::INTERNAL_SERVER_ERROR) - } // invalid state error InvalidState => ErrCode::internal("invalid_state", StatusCode::INTERNAL_SERVER_ERROR), @@ -120,6 +121,11 @@ impl Code { // invalid ranking rule InvalidRankingRule => ErrCode::invalid("invalid_request", StatusCode::BAD_REQUEST), + // invalid database + InvalidStore => { + ErrCode::internal("invalid_store_file", StatusCode::INTERNAL_SERVER_ERROR) + } + // invalid document MaxFieldsLimitExceeded => { ErrCode::invalid("max_fields_limit_exceeded", StatusCode::BAD_REQUEST) @@ -136,17 +142,22 @@ impl Code { BadParameter => ErrCode::invalid("bad_parameter", StatusCode::BAD_REQUEST), BadRequest => ErrCode::invalid("bad_request", StatusCode::BAD_REQUEST), + DatabaseSizeLimitReached => ErrCode::internal( + "database_size_limit_reached", + StatusCode::INTERNAL_SERVER_ERROR, + ), DocumentNotFound => ErrCode::invalid("document_not_found", StatusCode::NOT_FOUND), Internal => ErrCode::internal("internal", StatusCode::INTERNAL_SERVER_ERROR), - InvalidGeoField => { - ErrCode::authentication("invalid_geo_field", StatusCode::BAD_REQUEST) - } + InvalidGeoField => ErrCode::invalid("invalid_geo_field", StatusCode::BAD_REQUEST), InvalidToken => ErrCode::authentication("invalid_api_key", StatusCode::FORBIDDEN), MissingAuthorizationHeader => { ErrCode::authentication("missing_authorization_header", StatusCode::UNAUTHORIZED) } TaskNotFound => ErrCode::invalid("task_not_found", StatusCode::NOT_FOUND), - NotFound => ErrCode::invalid("not_found", StatusCode::NOT_FOUND), + 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) @@ -158,7 +169,7 @@ impl Code { // error related to dump DumpAlreadyInProgress => { - ErrCode::invalid("dump_already_in_progress", StatusCode::CONFLICT) + ErrCode::invalid("dump_already_processing", StatusCode::CONFLICT) } DumpProcessFailed => { ErrCode::internal("dump_process_failed", StatusCode::INTERNAL_SERVER_ERROR) diff --git a/meilisearch-http/src/extractors/authentication/error.rs b/meilisearch-http/src/extractors/authentication/error.rs index 902634045..5ed473b2a 100644 --- a/meilisearch-http/src/extractors/authentication/error.rs +++ b/meilisearch-http/src/extractors/authentication/error.rs @@ -2,14 +2,14 @@ use meilisearch_error::{Code, ErrorCode}; #[derive(Debug, thiserror::Error)] pub enum AuthenticationError { - #[error("You must have an authorization token")] + #[error("The X-MEILI-API-KEY header is missing.")] MissingAuthorizationHeader, - #[error("Invalid API key")] + #[error("The provided API key is invalid.")] InvalidToken(String), // Triggered on configuration error. - #[error("Irretrievable state")] + #[error("An internal error has occurred. `Irretrievable state`.")] IrretrievableState, - #[error("Unknown authentication policy")] + #[error("An internal error has occurred. `Unknown authentication policy`.")] UnknownPolicy, } diff --git a/meilisearch-http/tests/common/index.rs b/meilisearch-http/tests/common/index.rs index cb790ba70..43eac4c07 100644 --- a/meilisearch-http/tests/common/index.rs +++ b/meilisearch-http/tests/common/index.rs @@ -7,6 +7,7 @@ use actix_web::http::StatusCode; use paste::paste; use serde_json::{json, Value}; use tokio::time::sleep; +use urlencoding::encode; use super::service::Service; @@ -14,12 +15,12 @@ macro_rules! make_settings_test_routes { ($($name:ident),+) => { $(paste! { pub async fn [](&self, value: Value) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings/{}", self.uid, stringify!($name).replace("_", "-")); + let url = format!("/indexes/{}/settings/{}", encode(self.uid.as_ref()).to_string(), stringify!($name).replace("_", "-")); self.service.post(url, value).await } pub async fn [](&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings/{}", self.uid, stringify!($name).replace("_", "-")); + let url = format!("/indexes/{}/settings/{}", encode(self.uid.as_ref()).to_string(), stringify!($name).replace("_", "-")); self.service.get(url).await } })* @@ -34,12 +35,15 @@ pub struct Index<'a> { #[allow(dead_code)] impl Index<'_> { pub async fn get(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}", self.uid); + let url = format!("/indexes/{}", encode(self.uid.as_ref()).to_string()); self.service.get(url).await } pub async fn load_test_set(&self) -> u64 { - let url = format!("/indexes/{}/documents", self.uid); + let url = format!( + "/indexes/{}/documents", + encode(self.uid.as_ref()).to_string() + ); let (response, code) = self .service .post_str(url, include_str!("../assets/test_set.json")) @@ -62,13 +66,13 @@ impl Index<'_> { let body = json!({ "primaryKey": primary_key, }); - let url = format!("/indexes/{}", self.uid); + let url = format!("/indexes/{}", encode(self.uid.as_ref()).to_string()); self.service.put(url, body).await } pub async fn delete(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}", self.uid); + let url = format!("/indexes/{}", encode(self.uid.as_ref()).to_string()); self.service.delete(url).await } @@ -78,8 +82,15 @@ impl Index<'_> { primary_key: Option<&str>, ) -> (Value, StatusCode) { let url = match primary_key { - Some(key) => format!("/indexes/{}/documents?primaryKey={}", self.uid, key), - None => format!("/indexes/{}/documents", self.uid), + Some(key) => format!( + "/indexes/{}/documents?primaryKey={}", + encode(self.uid.as_ref()).to_string(), + key + ), + None => format!( + "/indexes/{}/documents", + encode(self.uid.as_ref()).to_string() + ), }; self.service.post(url, documents).await } @@ -90,15 +101,26 @@ impl Index<'_> { primary_key: Option<&str>, ) -> (Value, StatusCode) { let url = match primary_key { - Some(key) => format!("/indexes/{}/documents?primaryKey={}", self.uid, key), - None => format!("/indexes/{}/documents", self.uid), + Some(key) => format!( + "/indexes/{}/documents?primaryKey={}", + encode(self.uid.as_ref()).to_string(), + key + ), + None => format!( + "/indexes/{}/documents", + encode(self.uid.as_ref()).to_string() + ), }; self.service.put(url, documents).await } pub async fn wait_update_id(&self, update_id: u64) -> Value { // try 10 times to get status, or panic to not wait forever - let url = format!("/indexes/{}/updates/{}", self.uid, update_id); + let url = format!( + "/indexes/{}/updates/{}", + encode(self.uid.as_ref()).to_string(), + update_id + ); for _ in 0..10 { let (response, status_code) = self.service.get(&url).await; assert_eq!(status_code, 200, "response: {}", response); @@ -113,12 +135,16 @@ impl Index<'_> { } pub async fn get_update(&self, update_id: u64) -> (Value, StatusCode) { - let url = format!("/indexes/{}/updates/{}", self.uid, update_id); + let url = format!( + "/indexes/{}/updates/{}", + encode(self.uid.as_ref()).to_string(), + update_id + ); self.service.get(url).await } pub async fn list_updates(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/updates", self.uid); + let url = format!("/indexes/{}/updates", encode(self.uid.as_ref()).to_string()); self.service.get(url).await } @@ -127,12 +153,19 @@ impl Index<'_> { id: u64, _options: Option, ) -> (Value, StatusCode) { - let url = format!("/indexes/{}/documents/{}", self.uid, id); + let url = format!( + "/indexes/{}/documents/{}", + encode(self.uid.as_ref()).to_string(), + id + ); self.service.get(url).await } pub async fn get_all_documents(&self, options: GetAllDocumentsOptions) -> (Value, StatusCode) { - let mut url = format!("/indexes/{}/documents?", self.uid); + let mut url = format!( + "/indexes/{}/documents?", + encode(self.uid.as_ref()).to_string() + ); if let Some(limit) = options.limit { url.push_str(&format!("limit={}&", limit)); } @@ -152,39 +185,58 @@ impl Index<'_> { } pub async fn delete_document(&self, id: u64) -> (Value, StatusCode) { - let url = format!("/indexes/{}/documents/{}", self.uid, id); + let url = format!( + "/indexes/{}/documents/{}", + encode(self.uid.as_ref()).to_string(), + id + ); self.service.delete(url).await } pub async fn clear_all_documents(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/documents", self.uid); + let url = format!( + "/indexes/{}/documents", + encode(self.uid.as_ref()).to_string() + ); self.service.delete(url).await } pub async fn delete_batch(&self, ids: Vec) -> (Value, StatusCode) { - let url = format!("/indexes/{}/documents/delete-batch", self.uid); + let url = format!( + "/indexes/{}/documents/delete-batch", + encode(self.uid.as_ref()).to_string() + ); self.service .post(url, serde_json::to_value(&ids).unwrap()) .await } pub async fn settings(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings", self.uid); + let url = format!( + "/indexes/{}/settings", + encode(self.uid.as_ref()).to_string() + ); self.service.get(url).await } pub async fn update_settings(&self, settings: Value) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings", self.uid); + let url = format!( + "/indexes/{}/settings", + encode(self.uid.as_ref()).to_string() + ); self.service.post(url, settings).await } pub async fn delete_settings(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings", self.uid); + let url = format!( + "/indexes/{}/settings", + encode(self.uid.as_ref()).to_string() + ); self.service.delete(url).await } pub async fn stats(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/stats", self.uid); + let url = format!("/indexes/{}/stats", encode(self.uid.as_ref()).to_string()); self.service.get(url).await } @@ -209,13 +261,17 @@ impl Index<'_> { } pub async fn search_post(&self, query: Value) -> (Value, StatusCode) { - let url = format!("/indexes/{}/search", self.uid); + let url = format!("/indexes/{}/search", encode(self.uid.as_ref()).to_string()); self.service.post(url, query).await } pub async fn search_get(&self, query: Value) -> (Value, StatusCode) { let params = serde_url_params::to_string(&query).unwrap(); - let url = format!("/indexes/{}/search?{}", self.uid, params); + let url = format!( + "/indexes/{}/search?{}", + encode(self.uid.as_ref()).to_string(), + params + ); self.service.get(url).await } diff --git a/meilisearch-http/tests/common/server.rs b/meilisearch-http/tests/common/server.rs index 82666fc57..ab4127734 100644 --- a/meilisearch-http/tests/common/server.rs +++ b/meilisearch-http/tests/common/server.rs @@ -7,7 +7,6 @@ use meilisearch_lib::options::{IndexerOpts, MaxMemory}; use once_cell::sync::Lazy; use serde_json::Value; use tempfile::TempDir; -use urlencoding::encode; use meilisearch_http::option::Opt; @@ -62,7 +61,7 @@ impl Server { /// Returns a view to an index. There is no guarantee that the index exists. pub fn index(&self, uid: impl AsRef) -> Index<'_> { Index { - uid: encode(uid.as_ref()).to_string(), + uid: uid.as_ref().to_string(), service: &self.service, } } diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index e1747d779..dcecb19d9 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -940,25 +940,29 @@ async fn error_document_field_limit_reached() { } #[actix_rt::test] -#[ignore] // // TODO: Fix in an other PR: this does not provoke any error. async fn error_add_documents_invalid_geo_field() { let server = Server::new().await; let index = server.index("test"); index.create(Some("id")).await; + index + .update_settings(json!({"sortableAttributes": ["_geo"]})) + .await; + let documents = json!([ { "id": "11", "_geo": "foobar" } ]); + index.add_documents(documents, None).await; - index.wait_update_id(0).await; - let (response, code) = index.get_update(0).await; + index.wait_update_id(1).await; + let (response, code) = index.get_update(1).await; assert_eq!(code, 200); assert_eq!(response["status"], "failed"); assert_eq!( response["message"], - r#"The document with the id: `11` contains an invalid _geo field: :syntaxErrorHelper:REPLACE_ME."# + r#"The document with the id: `11` contains an invalid _geo field: `foobar`."# ); assert_eq!(response["code"], "invalid_geo_field"); assert_eq!(response["type"], "invalid_request"); diff --git a/meilisearch-http/tests/index/create_index.rs b/meilisearch-http/tests/index/create_index.rs index 6a9fd82ab..2d001517f 100644 --- a/meilisearch-http/tests/index/create_index.rs +++ b/meilisearch-http/tests/index/create_index.rs @@ -89,7 +89,6 @@ async fn error_create_existing_index() { } #[actix_rt::test] -#[ignore] // TODO: Fix in an other PR: uid returned `test%20test%23%21` instead of `test test#!` async fn error_create_with_invalid_index_uid() { let server = Server::new().await; let index = server.index("test test#!"); diff --git a/meilisearch-lib/src/document_formats.rs b/meilisearch-lib/src/document_formats.rs index 137ecc280..c5b5f7c4e 100644 --- a/meilisearch-lib/src/document_formats.rs +++ b/meilisearch-lib/src/document_formats.rs @@ -25,7 +25,7 @@ impl fmt::Display for PayloadType { #[derive(thiserror::Error, Debug)] pub enum DocumentFormatError { - #[error("Internal error!: {0}")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), #[error("The `{1}` payload provided is malformed. `{0}`.")] MalformedPayload( diff --git a/meilisearch-lib/src/error.rs b/meilisearch-lib/src/error.rs index 689a8ddae..f245b803b 100644 --- a/meilisearch-lib/src/error.rs +++ b/meilisearch-lib/src/error.rs @@ -36,11 +36,11 @@ impl ErrorCode for MilliError<'_> { match error { // TODO: wait for spec for new error codes. UserError::SerdeJson(_) - | UserError::MaxDatabaseSizeReached - | UserError::InvalidStoreFile - | UserError::NoSpaceLeftOnDevice | UserError::DocumentLimitReached | UserError::UnknownInternalDocumentId { .. } => Code::Internal, + UserError::InvalidStoreFile => Code::InvalidStore, + UserError::NoSpaceLeftOnDevice => Code::NoSpaceLeftOnDevice, + UserError::MaxDatabaseSizeReached => Code::DatabaseSizeLimitReached, UserError::AttributeLimitReached => Code::MaxFieldsLimitExceeded, UserError::InvalidFilter(_) => Code::Filter, UserError::MissingDocumentId { .. } => Code::MissingDocumentId, diff --git a/meilisearch-lib/src/index/error.rs b/meilisearch-lib/src/index/error.rs index 92691cb14..91bf6db93 100644 --- a/meilisearch-lib/src/index/error.rs +++ b/meilisearch-lib/src/index/error.rs @@ -9,7 +9,7 @@ pub type Result = std::result::Result; #[derive(Debug, thiserror::Error)] pub enum IndexError { - #[error("Internal error: {0}")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), #[error("Document `{0}` not found.")] DocumentNotFound(String), diff --git a/meilisearch-lib/src/index_controller/dump_actor/error.rs b/meilisearch-lib/src/index_controller/dump_actor/error.rs index 49da8c90f..23616f964 100644 --- a/meilisearch-lib/src/index_controller/dump_actor/error.rs +++ b/meilisearch-lib/src/index_controller/dump_actor/error.rs @@ -7,11 +7,11 @@ pub type Result = std::result::Result; #[derive(thiserror::Error, Debug)] pub enum DumpActorError { - #[error("Another dump is already in progress")] + #[error("A dump is already processing. You must wait until the current process is finished before requesting another dump.")] DumpAlreadyRunning, #[error("Dump `{0}` not found.")] DumpDoesNotExist(String), - #[error("Internal error: {0}")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), #[error("{0}")] IndexResolver(#[from] IndexResolverError), @@ -43,7 +43,7 @@ impl ErrorCode for DumpActorError { fn error_code(&self) -> Code { match self { DumpActorError::DumpAlreadyRunning => Code::DumpAlreadyInProgress, - DumpActorError::DumpDoesNotExist(_) => Code::NotFound, + DumpActorError::DumpDoesNotExist(_) => Code::DumpNotFound, DumpActorError::Internal(_) => Code::Internal, DumpActorError::IndexResolver(e) => e.error_code(), DumpActorError::UpdateLoop(e) => e.error_code(), diff --git a/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs b/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs index 13d770f6a..ebaa7d3ff 100644 --- a/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs +++ b/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs @@ -361,7 +361,6 @@ mod compat { "index_already_exists" => Code::IndexAlreadyExists, "index_not_found" => Code::IndexNotFound, "invalid_index_uid" => Code::InvalidIndexUid, - "index_not_accessible" => Code::OpenIndex, "invalid_state" => Code::InvalidState, "missing_primary_key" => Code::MissingPrimaryKey, "primary_key_already_present" => Code::PrimaryKeyAlreadyPresent, @@ -378,7 +377,6 @@ mod compat { "invalid_geo_field" => Code::InvalidGeoField, "invalid_token" => Code::InvalidToken, "missing_authorization_header" => Code::MissingAuthorizationHeader, - "not_found" => Code::NotFound, "payload_too_large" => Code::PayloadTooLarge, "unretrievable_document" => Code::RetrieveDocument, "search_error" => Code::SearchDocuments, diff --git a/meilisearch-lib/src/index_controller/error.rs b/meilisearch-lib/src/index_controller/error.rs index 417bda01b..bc69b534f 100644 --- a/meilisearch-lib/src/index_controller/error.rs +++ b/meilisearch-lib/src/index_controller/error.rs @@ -24,7 +24,7 @@ pub enum IndexControllerError { DumpActor(#[from] DumpActorError), #[error("{0}")] IndexError(#[from] IndexError), - #[error("Internal error: {0}")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), } diff --git a/meilisearch-lib/src/index_controller/index_resolver/error.rs b/meilisearch-lib/src/index_controller/index_resolver/error.rs index 9149f801e..f404f0b60 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/error.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/error.rs @@ -19,9 +19,9 @@ pub enum IndexResolverError { UnexistingIndex(String), #[error("A primary key is already present. It's impossible to update it")] ExistingPrimaryKey, - #[error("Internal Error: `{0}`")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), - #[error("Internal Error: Index uuid `{0}` is already assigned.")] + #[error("The creation of the `{0}` index has failed due to `Index uuid is already assigned`.")] UuidAlreadyExists(Uuid), #[error("{0}")] Milli(#[from] milli::Error), @@ -60,7 +60,7 @@ impl ErrorCode for IndexResolverError { IndexResolverError::UnexistingIndex(_) => Code::IndexNotFound, IndexResolverError::ExistingPrimaryKey => Code::PrimaryKeyAlreadyPresent, IndexResolverError::Internal(_) => Code::Internal, - IndexResolverError::UuidAlreadyExists(_) => Code::Internal, + IndexResolverError::UuidAlreadyExists(_) => Code::CreateIndex, IndexResolverError::Milli(e) => MilliError(e).error_code(), IndexResolverError::BadlyFormatted(_) => Code::InvalidIndexUid, } diff --git a/meilisearch-lib/src/index_controller/updates/error.rs b/meilisearch-lib/src/index_controller/updates/error.rs index 27260079e..34ffca892 100644 --- a/meilisearch-lib/src/index_controller/updates/error.rs +++ b/meilisearch-lib/src/index_controller/updates/error.rs @@ -16,7 +16,7 @@ pub type Result = std::result::Result; pub enum UpdateLoopError { #[error("Task `{0}` not found.")] UnexistingUpdate(u64), - #[error("Internal error: {0}")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), #[error( "update store was shut down due to a fatal error, please check your logs for more info."