diff --git a/Cargo.lock b/Cargo.lock index bddde66f4..a323c86de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1783,7 +1783,7 @@ dependencies = [ [[package]] name = "milli" version = "0.19.0" -source = "git+https://github.com/meilisearch/milli.git?tag=v0.19.0#d7943fe22553b8205b86c32a0f2656d9e42de351" +source = "git+https://github.com/meilisearch/milli.git?branch=update-error-format#3599df77f05f4e89a28c0160411e95a840e0b227" dependencies = [ "bimap", "bincode", diff --git a/meilisearch-error/src/lib.rs b/meilisearch-error/src/lib.rs index 3a3e2229a..8c9ee411c 100644 --- a/meilisearch-error/src/lib.rs +++ b/meilisearch-error/src/lib.rs @@ -62,6 +62,7 @@ pub enum Code { MaxFieldsLimitExceeded, MissingDocumentId, + InvalidDocumentId, Facet, Filter, @@ -76,6 +77,7 @@ pub enum Code { InvalidToken, MissingAuthorizationHeader, NotFound, + TaskNotFound, PayloadTooLarge, RetrieveDocument, SearchDocuments, @@ -99,7 +101,7 @@ impl Code { // index related errors // create index is thrown on internal error while creating an index. CreateIndex => ErrCode::internal("index_creation_failed", StatusCode::BAD_REQUEST), - IndexAlreadyExists => ErrCode::invalid("index_already_exists", StatusCode::BAD_REQUEST), + 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), @@ -113,7 +115,7 @@ impl Code { MissingPrimaryKey => ErrCode::invalid("missing_primary_key", StatusCode::BAD_REQUEST), // error thrown when trying to set an already existing primary key PrimaryKeyAlreadyPresent => { - ErrCode::invalid("primary_key_already_present", StatusCode::BAD_REQUEST) + ErrCode::invalid("index_primary_key_already_exists", StatusCode::BAD_REQUEST) } // invalid ranking rule InvalidRankingRule => ErrCode::invalid("invalid_request", StatusCode::BAD_REQUEST), @@ -123,6 +125,7 @@ impl Code { ErrCode::invalid("max_fields_limit_exceeded", StatusCode::BAD_REQUEST) } MissingDocumentId => ErrCode::invalid("missing_document_id", StatusCode::BAD_REQUEST), + InvalidDocumentId => ErrCode::invalid("invalid_document_id", StatusCode::BAD_REQUEST), // error related to facets Facet => ErrCode::invalid("invalid_facet", StatusCode::BAD_REQUEST), @@ -142,6 +145,7 @@ impl Code { 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), PayloadTooLarge => ErrCode::invalid("payload_too_large", StatusCode::PAYLOAD_TOO_LARGE), RetrieveDocument => { diff --git a/meilisearch-http/src/error.rs b/meilisearch-http/src/error.rs index c3c8d5785..39dace4fc 100644 --- a/meilisearch-http/src/error.rs +++ b/meilisearch-http/src/error.rs @@ -12,11 +12,11 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, thiserror::Error)] pub enum MeilisearchHttpError { #[error("A Content-Type header is missing. Accepted values for the Content-Type header are: {}", - .0.iter().map(|s| format!("\"{}\"", s)).collect::>().join(", "))] + .0.iter().map(|s| format!("`{}`", s)).collect::>().join(", "))] MissingContentType(Vec), #[error( - "The Content-Type \"{0}\" is invalid. Accepted values for the Content-Type header are: {}", - .1.iter().map(|s| format!("\"{}\"", s)).collect::>().join(", ") + "The Content-Type `{0}` is invalid. Accepted values for the Content-Type header are: {}", + .1.iter().map(|s| format!("`{}`", s)).collect::>().join(", ") )] InvalidContentType(String, Vec), } diff --git a/meilisearch-http/tests/common/index.rs b/meilisearch-http/tests/common/index.rs index cb790ba70..2e126e67d 100644 --- a/meilisearch-http/tests/common/index.rs +++ b/meilisearch-http/tests/common/index.rs @@ -99,7 +99,7 @@ impl Index<'_> { 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); - for _ in 0..10 { + for _ in 0..20 { let (response, status_code) = self.service.get(&url).await; assert_eq!(status_code, 200, "response: {}", response); diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index 14f5c6dee..eaeabe417 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -80,7 +80,7 @@ async fn error_add_documents_test_bad_content_types() { assert_eq!( response["message"], json!( - r#"The Content-Type "text/plain" is invalid. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + r#"The Content-Type `text/plain` is invalid. Accepted values for the Content-Type header are: `application/json`, `application/x-ndjson`, `text/csv`"# ) ); assert_eq!(response["code"], "invalid_content_type"); @@ -104,7 +104,7 @@ async fn error_add_documents_test_bad_content_types() { assert_eq!( response["message"], json!( - r#"The Content-Type "text/plain" is invalid. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + r#"The Content-Type `text/plain` is invalid. Accepted values for the Content-Type header are: `application/json`, `application/x-ndjson`, `text/csv`"# ) ); assert_eq!(response["code"], "invalid_content_type"); @@ -145,7 +145,7 @@ async fn error_add_documents_test_no_content_type() { assert_eq!( response["message"], json!( - r#"A Content-Type header is missing. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + r#"A Content-Type header is missing. Accepted values for the Content-Type header are: `application/json`, `application/x-ndjson`, `text/csv`"# ) ); assert_eq!(response["code"], "missing_content_type"); @@ -168,7 +168,7 @@ async fn error_add_documents_test_no_content_type() { assert_eq!( response["message"], json!( - r#"A Content-Type header is missing. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + r#"A Content-Type header is missing. Accepted values for the Content-Type header are: `application/json`, `application/x-ndjson`, `text/csv`"# ) ); assert_eq!(response["code"], "missing_content_type"); @@ -204,7 +204,7 @@ async fn error_add_malformed_csv_documents() { assert_eq!( response["message"], json!( - r#"The csv payload provided is malformed. CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields."# + r#"The `csv` payload provided is malformed. `CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -228,7 +228,7 @@ async fn error_add_malformed_csv_documents() { assert_eq!( response["message"], json!( - r#"The csv payload provided is malformed. CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields."# + r#"The `csv` payload provided is malformed. `CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -264,7 +264,7 @@ async fn error_add_malformed_json_documents() { assert_eq!( response["message"], json!( - r#"The json payload provided is malformed. key must be a string at line 1 column 14."# + r#"The `json` payload provided is malformed. `Couldn't serialize document value: key must be a string at line 1 column 14`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -288,7 +288,7 @@ async fn error_add_malformed_json_documents() { assert_eq!( response["message"], json!( - r#"The json payload provided is malformed. key must be a string at line 1 column 14."# + r#"The `json` payload provided is malformed. `Couldn't serialize document value: key must be a string at line 1 column 14`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -324,7 +324,7 @@ async fn error_add_malformed_ndjson_documents() { assert_eq!( response["message"], json!( - r#"The ndjson payload provided is malformed. key must be a string at line 2 column 2."# + r#"The `ndjson` payload provided is malformed. `Couldn't serialize document value: key must be a string at line 1 column 2`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -348,7 +348,7 @@ async fn error_add_malformed_ndjson_documents() { assert_eq!( response["message"], json!( - r#"The ndjson payload provided is malformed. key must be a string at line 2 column 2."# + r#"The `ndjson` payload provided is malformed. `Couldn't serialize document value: key must be a string at line 1 column 2`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -359,6 +359,162 @@ async fn error_add_malformed_ndjson_documents() { ); } +#[actix_rt::test] +async fn error_add_missing_payload_csv_documents() { + let document = ""; + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/csv")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!(response["message"], json!(r#"A csv payload is missing."#)); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/csv")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!(response["message"], json!(r#"A csv payload is missing."#)); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); +} + +#[actix_rt::test] +async fn error_add_missing_payload_json_documents() { + let document = ""; + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/json")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!(response["message"], json!(r#"A json payload is missing."#)); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/json")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!(response["message"], json!(r#"A json payload is missing."#)); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); +} + +#[actix_rt::test] +async fn error_add_missing_payload_ndjson_documents() { + let document = ""; + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/x-ndjson")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"A ndjson payload is missing."#) + ); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/x-ndjson")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"A ndjson payload is missing."#) + ); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); +} + #[actix_rt::test] async fn add_documents_no_index_creation() { let server = Server::new().await; @@ -411,7 +567,7 @@ async fn error_document_add_create_index_bad_uid() { let (response, code) = index.add_documents(json!([]), None).await; let expected_response = json!({ - "message": "883 fj! is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", + "message": "`883 fj!` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", "code": "invalid_index_uid", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_index_uid" @@ -428,7 +584,7 @@ async fn error_document_update_create_index_bad_uid() { let (response, code) = index.update_documents(json!([]), None).await; let expected_response = json!({ - "message": "883 fj! is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", + "message": "`883 fj!` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", "code": "invalid_index_uid", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_index_uid" @@ -646,13 +802,13 @@ async fn error_add_documents_bad_document_id() { index.wait_update_id(0).await; let (response, code) = index.get_update(0).await; assert_eq!(code, 200); - assert_eq!(response["status"], "failed"); - assert_eq!(response["message"], "Document identifier foo & bar is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_)."); - assert_eq!(response["code"], "invalid_document_id"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["status"], json!("failed")); + assert_eq!(response["message"], json!("Document identifier `foo & bar` is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_).")); + assert_eq!(response["code"], json!("invalid_document_id")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#invalid_document_id" + json!("https://docs.meilisearch.com/errors#invalid_document_id") ); } @@ -671,13 +827,13 @@ async fn error_update_documents_bad_document_id() { index.wait_update_id(0).await; let (response, code) = index.get_update(0).await; assert_eq!(code, 200); - assert_eq!(response["status"], "failed"); - assert_eq!(response["message"], "Document identifier foo & bar is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_)."); - assert_eq!(response["code"], "invalid_document_id"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["status"], json!("failed")); + assert_eq!(response["message"], json!("Document identifier `foo & bar` is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_).")); + assert_eq!(response["code"], json!("invalid_document_id")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#invalid_document_id" + json!("https://docs.meilisearch.com/errors#invalid_document_id") ); } @@ -699,13 +855,13 @@ async fn error_add_documents_missing_document_id() { assert_eq!(response["status"], "failed"); assert_eq!( response["message"], - r#"Document doesn't have a docid attribute: {"id":"11","content":"foobar"}."# + json!(r#"Document doesn't have a `docid` attribute: `{"id":"11","content":"foobar"}`."#) ); - assert_eq!(response["code"], "missing_document_id"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["code"], json!("missing_document_id")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#missing_document_id" + json!("https://docs.meilisearch.com/errors#missing_document_id") ); } @@ -727,7 +883,7 @@ async fn error_update_documents_missing_document_id() { assert_eq!(response["status"], "failed"); assert_eq!( response["message"], - r#"Document doesn't have a docid attribute: {"id":"11","content":"foobar"}."# + r#"Document doesn't have a `docid` attribute: `{"id":"11","content":"foobar"}`."# ); assert_eq!(response["code"], "missing_document_id"); assert_eq!(response["type"], "invalid_request"); @@ -737,109 +893,41 @@ async fn error_update_documents_missing_document_id() { ); } -#[actix_rt::test] -async fn error_add_documents_with_primary_key_and_primary_key_already_exists() { - let server = Server::new().await; - let index = server.index("test"); +// #[actix_rt::test] +// async fn error_document_field_limit_reached() { +// let server = Server::new().await; +// let index = server.index("test"); - index.create(Some("primary")).await; - let documents = json!([ - { - "id": 1, - "content": "foo", - } - ]); +// index.create(Some("id")).await; - let (_response, code) = index.add_documents(documents, Some("id")).await; - assert_eq!(code, 202); +// let mut big_object = std::collections::HashMap::new(); +// big_object.insert("id".to_owned(), "wow"); +// for i in 0..65535 { +// let key = i.to_string(); +// big_object.insert(key, "I am a text!"); +// } - index.wait_update_id(0).await; +// let documents = json!([big_object]); - let (response, code) = index.get_update(0).await; - assert_eq!(code, 200); - assert_eq!(response["status"], "failed"); - assert_eq!(response["message"], "Index test already has a primary key."); - assert_eq!(response["code"], "index_primary_key_already_exists"); - assert_eq!(response["type"], "invalid_request"); - assert_eq!( - response["link"], - "https://docs.meilisearch.com/errors#index_primary_key_already_exists" - ); +// let (_response, code) = index.update_documents(documents, Some("id")).await; +// assert_eq!(code, 202); - let (response, code) = index.get().await; - assert_eq!(code, 200); - assert_eq!(response["primaryKey"], "primary"); -} - -#[actix_rt::test] -async fn error_update_documents_with_primary_key_and_primary_key_already_exists() { - let server = Server::new().await; - let index = server.index("test"); - - index.create(Some("primary")).await; - let documents = json!([ - { - "id": 1, - "content": "foo", - } - ]); - - let (_response, code) = index.update_documents(documents, Some("id")).await; - assert_eq!(code, 202); - - index.wait_update_id(0).await; - let (response, code) = index.get_update(0).await; - assert_eq!(code, 200); - // Documents without a primary key are not accepted. - assert_eq!(response["status"], "failed"); - assert_eq!(response["message"], "Index test already has a primary key."); - assert_eq!(response["code"], "index_primary_key_already_exists"); - assert_eq!(response["type"], "invalid_request"); - assert_eq!( - response["link"], - "https://docs.meilisearch.com/errors#index_primary_key_already_exists" - ); - - let (response, code) = index.get().await; - assert_eq!(code, 200); - assert_eq!(response["primaryKey"], "primary"); -} - -#[actix_rt::test] -async fn error_document_field_limit_reached() { - let server = Server::new().await; - let index = server.index("test"); - - index.create(Some("id")).await; - - let mut big_object = std::collections::HashMap::new(); - big_object.insert("id".to_owned(), "wow"); - for i in 0..65535 { - let key = i.to_string(); - big_object.insert(key, "I am a text!"); - } - - let documents = json!([big_object]); - - let (_response, code) = index.update_documents(documents, Some("id")).await; - assert_eq!(code, 202); - - index.wait_update_id(0).await; - let (response, code) = index.get_update(0).await; - assert_eq!(code, 200); - // Documents without a primary key are not accepted. - assert_eq!(response["status"], "failed"); - assert_eq!( - response["message"], - "A document cannot contain more than 65,535 fields." - ); - assert_eq!(response["code"], "document_fields_limit_reached"); - assert_eq!(response["type"], "invalid_request"); - assert_eq!( - response["link"], - "https://docs.meilisearch.com/errors#document_fields_limit_reached" - ); -} +// index.wait_update_id(0).await; +// let (response, code) = index.get_update(0).await; +// assert_eq!(code, 200); +// // Documents without a primary key are not accepted. +// assert_eq!(response["status"], "failed"); +// assert_eq!( +// response["message"], +// "A document cannot contain more than 65,535 fields." +// ); +// assert_eq!(response["code"], "document_fields_limit_reached"); +// assert_eq!(response["type"], "invalid_request"); +// assert_eq!( +// response["link"], +// "https://docs.meilisearch.com/errors#document_fields_limit_reached" +// ); +// } #[actix_rt::test] async fn error_add_documents_invalid_geo_field() { @@ -885,7 +973,7 @@ async fn error_add_documents_payload_size() { let (response, code) = index.add_documents(documents, None).await; let expected_response = json!({ - "message": "The payload cannot exceed 10MB.", + "message": "The provided payload reached the size limit.", "code": "payload_too_large", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#payload_too_large" diff --git a/meilisearch-http/tests/documents/delete_documents.rs b/meilisearch-http/tests/documents/delete_documents.rs index 81125e580..56210059b 100644 --- a/meilisearch-http/tests/documents/delete_documents.rs +++ b/meilisearch-http/tests/documents/delete_documents.rs @@ -87,7 +87,7 @@ async fn error_delete_batch_unexisting_index() { let server = Server::new().await; let (response, code) = server.index("test").delete_batch(vec![]).await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/documents/get_documents.rs b/meilisearch-http/tests/documents/get_documents.rs index 11f43f2c4..6d9a2d150 100644 --- a/meilisearch-http/tests/documents/get_documents.rs +++ b/meilisearch-http/tests/documents/get_documents.rs @@ -20,7 +20,7 @@ async fn error_get_unexisting_document() { let (response, code) = index.get_document(1, None).await; let expected_response = json!({ - "message": "Document 1 not found.", + "message": "Document `1` not found.", "code": "document_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#document_not_found" @@ -64,7 +64,7 @@ async fn error_get_unexisting_index_all_documents() { .await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/index/create_index.rs b/meilisearch-http/tests/index/create_index.rs index e5cfa2fa5..2d001517f 100644 --- a/meilisearch-http/tests/index/create_index.rs +++ b/meilisearch-http/tests/index/create_index.rs @@ -78,7 +78,7 @@ async fn error_create_existing_index() { let (response, code) = index.create(Some("primary")).await; let expected_response = json!({ - "message": "Index primary already exists.", + "message": "Index `test` already exists.", "code": "index_already_exists", "type": "invalid_request", "link":"https://docs.meilisearch.com/errors#index_already_exists" @@ -95,7 +95,7 @@ async fn error_create_with_invalid_index_uid() { let (response, code) = index.create(None).await; let expected_response = json!({ - "message": "test test#! is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", + "message": "`test test#!` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", "code": "invalid_index_uid", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_index_uid" diff --git a/meilisearch-http/tests/index/delete_index.rs b/meilisearch-http/tests/index/delete_index.rs index 84c780650..c4366af35 100644 --- a/meilisearch-http/tests/index/delete_index.rs +++ b/meilisearch-http/tests/index/delete_index.rs @@ -24,7 +24,7 @@ async fn error_delete_unexisting_index() { let (response, code) = index.delete().await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/index/get_index.rs b/meilisearch-http/tests/index/get_index.rs index ec35ea283..4a1fa6692 100644 --- a/meilisearch-http/tests/index/get_index.rs +++ b/meilisearch-http/tests/index/get_index.rs @@ -30,7 +30,7 @@ async fn error_get_unexisting_index() { let (response, code) = index.get().await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/index/stats.rs b/meilisearch-http/tests/index/stats.rs index 7abbd0cd4..755366fed 100644 --- a/meilisearch-http/tests/index/stats.rs +++ b/meilisearch-http/tests/index/stats.rs @@ -53,7 +53,7 @@ async fn error_get_stats_unexisting_index() { let (response, code) = server.index("test").stats().await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/index/update_index.rs b/meilisearch-http/tests/index/update_index.rs index 5d92a0a16..a22def409 100644 --- a/meilisearch-http/tests/index/update_index.rs +++ b/meilisearch-http/tests/index/update_index.rs @@ -44,14 +44,23 @@ async fn update_nothing() { async fn error_update_existing_primary_key() { let server = Server::new().await; let index = server.index("test"); - let (_response, code) = index.create(Some("primary")).await; + let (_response, code) = index.create(Some("id")).await; assert_eq!(code, 201); - let (response, code) = index.update(Some("primary2")).await; + let documents = json!([ + { + "id": "11", + "content": "foobar" + } + ]); + index.add_documents(documents, None).await; + index.wait_update_id(0).await; + + let (response, code) = index.update(Some("primary")).await; let expected_response = json!({ - "message": "Index test already has a primary key.", + "message": "Index already has a primary key: `id`.", "code": "index_primary_key_already_exists", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_primary_key_already_exists" @@ -67,7 +76,7 @@ async fn error_update_unexisting_index() { let (response, code) = server.index("test").update(None).await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/search/errors.rs b/meilisearch-http/tests/search/errors.rs index d7e4b342e..e4dc12f40 100644 --- a/meilisearch-http/tests/search/errors.rs +++ b/meilisearch-http/tests/search/errors.rs @@ -8,10 +8,17 @@ async fn search_unexisting_index() { let server = Server::new().await; let index = server.index("test"); + let expected_response = json!({ + "message": "Index `test` not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + index .search(json!({"q": "hello"}), |response, code| { - assert_eq!(code, 404, "{}", response); - assert_eq!(response["errorCode"], "index_not_found"); + assert_eq!(code, 404); + assert_eq!(response, expected_response); }) .await; } @@ -24,7 +31,7 @@ async fn search_unexisting_parameter() { index .search(json!({"marin": "hello"}), |response, code| { assert_eq!(code, 400, "{}", response); - assert_eq!(response["errorCode"], "bad_request"); + assert_eq!(response["code"], "bad_request"); }) .await; } @@ -43,13 +50,13 @@ async fn filter_invalid_syntax_object() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Invalid syntax for the filter parameter: :syntaxErrorHelper:REPLACE_ME.", + "message": "Invalid syntax for the filter parameter: ` --> 1:7\n |\n1 | title & Glass\n | ^---\n |\n = expected word`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" }); index - .search(json!({"filter": {"title": "Glass"}}), |response, code| { + .search(json!({"filter": "title & Glass"}), |response, code| { assert_eq!(response, expected_response); assert_eq!(code, 400); }) @@ -70,19 +77,16 @@ async fn filter_invalid_syntax_array() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Invalid syntax for the filter parameter: :syntaxErrorHelper:REPLACE_ME.", + "message": "Invalid syntax for the filter parameter: ` --> 1:7\n |\n1 | title & Glass\n | ^---\n |\n = expected word`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" }); index - .search( - json!({"filter": [[["title = Glass"]]]}), - |response, code| { - assert_eq!(response, expected_response); - assert_eq!(code, 400); - }, - ) + .search(json!({"filter": [["title & Glass"]]}), |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }) .await; } @@ -100,7 +104,7 @@ async fn filter_invalid_syntax_string() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Invalid syntax for the filter parameter: :syntaxErrorHelper:REPLACE_ME.", + "message": "Invalid syntax for the filter parameter: ` --> 1:15\n |\n1 | title = Glass XOR title = Glass\n | ^---\n |\n = expected EOI, and, or or`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -130,7 +134,7 @@ async fn filter_invalid_attribute_array() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Attribute many is not filterable. Available filterable attributes are: title.", + "message": "Attribute `many` is not filterable. Available filterable attributes are: `title`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -157,7 +161,7 @@ async fn filter_invalid_attribute_string() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Attribute many is not filterable. Available filterable attributes are: title.", + "message": "Attribute `many` is not filterable. Available filterable attributes are: `title`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -184,7 +188,7 @@ async fn filter_reserved_geo_attribute_array() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geo is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", + "message": "`_geo` is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -211,7 +215,7 @@ async fn filter_reserved_geo_attribute_string() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geo is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", + "message": "`_geo` is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -238,7 +242,7 @@ async fn filter_reserved_attribute_array() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geoDistance is a reserved keyword and thus can't be used as a filter expression.", + "message": "`_geoDistance` is a reserved keyword and thus can't be used as a filter expression.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -268,7 +272,7 @@ async fn filter_reserved_attribute_string() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geoDistance is a reserved keyword and thus can't be used as a filter expression.", + "message": "`_geoDistance` is a reserved keyword and thus can't be used as a filter expression.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -298,7 +302,7 @@ async fn sort_geo_reserved_attribute() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geo is a reserved keyword and thus can't be used as a filter expression. Use the _geoPoint(latitude, longitude) built-in rule to sort on _geo field coordinates.", + "message": "`_geo` is a reserved keyword and thus can't be used as a sort expression. Use the _geoPoint(latitude, longitude) built-in rule to sort on _geo field coordinates.", "code": "invalid_sort", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_sort" @@ -330,7 +334,7 @@ async fn sort_reserved_attribute() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geoDistance is a reserved keyword and thus can't be used as a filter expression.", + "message": "`_geoDistance` is a reserved keyword and thus can't be used as a sort expression.", "code": "invalid_sort", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_sort" @@ -362,7 +366,7 @@ async fn sort_unsortable_attribute() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Attribute title is not sortable. Available sortable attributes are: id.", + "message": "Attribute `title` is not sortable. Available sortable attributes are: `id`.", "code": "invalid_sort", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_sort" @@ -394,7 +398,7 @@ async fn sort_invalid_syntax() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Invalid syntax for the sort parameter: :syntaxErrorHelper:REPLACE_ME.", + "message": "Invalid syntax for the sort parameter: expected expression ending by `:asc` or `:desc`, found `title`.", "code": "invalid_sort", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_sort" @@ -419,7 +423,7 @@ async fn sort_unset_ranking_rule() { index .update_settings( - json!({"sortableAttributes": ["id"], "rankingRules": ["proximity", "exactness"]}), + json!({"sortableAttributes": ["title"], "rankingRules": ["proximity", "exactness"]}), ) .await; @@ -436,7 +440,7 @@ async fn sort_unset_ranking_rule() { index .search( json!({ - "sort": ["title"] + "sort": ["title:asc"] }), |response, code| { assert_eq!(response, expected_response); diff --git a/meilisearch-http/tests/updates/mod.rs b/meilisearch-http/tests/updates/mod.rs index c4ab1a275..f7bf9450a 100644 --- a/meilisearch-http/tests/updates/mod.rs +++ b/meilisearch-http/tests/updates/mod.rs @@ -7,7 +7,7 @@ async fn error_get_update_unexisting_index() { let (response, code) = server.index("test").get_update(0).await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" @@ -25,7 +25,7 @@ async fn error_get_unexisting_update_status() { let (response, code) = index.get_update(0).await; let expected_response = json!({ - "message": "Task 0 not found.", + "message": "Task `0` not found.", "code": "task_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#task_not_found" @@ -60,7 +60,7 @@ async fn error_list_updates_unexisting_index() { let (response, code) = server.index("test").list_updates().await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-lib/Cargo.toml b/meilisearch-lib/Cargo.toml index 429481539..74592419b 100644 --- a/meilisearch-lib/Cargo.toml +++ b/meilisearch-lib/Cargo.toml @@ -30,7 +30,7 @@ lazy_static = "1.4.0" log = "0.4.14" meilisearch-error = { path = "../meilisearch-error" } meilisearch-tokenizer = { git = "https://github.com/meilisearch/tokenizer.git", tag = "v0.2.5" } -milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.19.0" } +milli = { git = "https://github.com/meilisearch/milli.git", branch = "update-error-format" } mime = "0.3.16" num_cpus = "1.13.0" once_cell = "1.8.0" diff --git a/meilisearch-lib/src/document_formats.rs b/meilisearch-lib/src/document_formats.rs index 1d965d260..9ddbdf09d 100644 --- a/meilisearch-lib/src/document_formats.rs +++ b/meilisearch-lib/src/document_formats.rs @@ -25,9 +25,9 @@ impl fmt::Display for PayloadType { #[derive(thiserror::Error, Debug)] pub enum DocumentFormatError { - #[error("Internal error: {0}")] + #[error("Internal error!: {0}")] Internal(Box), - #[error("The {1} payload provided is malformed. {0}.")] + #[error("The `{1}` payload provided is malformed. `{0}`.")] MalformedPayload( Box, PayloadType, @@ -55,18 +55,18 @@ impl ErrorCode for DocumentFormatError { internal_error!(DocumentFormatError: io::Error); /// reads csv from input and write an obkv batch to writer. -pub fn read_csv(input: impl Read, writer: impl Write + Seek) -> Result<()> { +pub fn read_csv(input: impl Read, writer: impl Write + Seek) -> Result { let writer = BufWriter::new(writer); - DocumentBatchBuilder::from_csv(input, writer) - .map_err(|e| (PayloadType::Csv, e))? - .finish() - .map_err(|e| (PayloadType::Csv, e))?; + let builder = + DocumentBatchBuilder::from_csv(input, writer).map_err(|e| (PayloadType::Csv, e))?; + let document_count = builder.len(); + builder.finish().map_err(|e| (PayloadType::Csv, e))?; - Ok(()) + Ok(document_count) } /// reads jsonl from input and write an obkv batch to writer. -pub fn read_ndjson(input: impl Read, writer: impl Write + Seek) -> Result<()> { +pub fn read_ndjson(input: impl Read, writer: impl Write + Seek) -> Result { let mut reader = BufReader::new(input); let writer = BufWriter::new(writer); @@ -80,19 +80,22 @@ pub fn read_ndjson(input: impl Read, writer: impl Write + Seek) -> Result<()> { buf.clear(); } + let document_count = builder.len(); + builder.finish().map_err(|e| (PayloadType::Ndjson, e))?; - Ok(()) + Ok(document_count) } /// reads json from input and write an obkv batch to writer. -pub fn read_json(input: impl Read, writer: impl Write + Seek) -> Result<()> { +pub fn read_json(input: impl Read, writer: impl Write + Seek) -> Result { let writer = BufWriter::new(writer); let mut builder = DocumentBatchBuilder::new(writer).map_err(|e| (PayloadType::Json, e))?; builder .extend_from_json(input) .map_err(|e| (PayloadType::Json, e))?; + let document_count = builder.len(); builder.finish().map_err(|e| (PayloadType::Json, e))?; - Ok(()) + Ok(document_count) } diff --git a/meilisearch-lib/src/error.rs b/meilisearch-lib/src/error.rs index d29c18d25..689a8ddae 100644 --- a/meilisearch-lib/src/error.rs +++ b/meilisearch-lib/src/error.rs @@ -37,19 +37,17 @@ impl ErrorCode for MilliError<'_> { // TODO: wait for spec for new error codes. UserError::SerdeJson(_) | UserError::MaxDatabaseSizeReached - | UserError::InvalidDocumentId { .. } | UserError::InvalidStoreFile | UserError::NoSpaceLeftOnDevice - | UserError::DocumentLimitReached => Code::Internal, + | UserError::DocumentLimitReached + | UserError::UnknownInternalDocumentId { .. } => Code::Internal, UserError::AttributeLimitReached => Code::MaxFieldsLimitExceeded, UserError::InvalidFilter(_) => Code::Filter, - UserError::InvalidFilterAttribute(_) => Code::Filter, UserError::MissingDocumentId { .. } => Code::MissingDocumentId, + UserError::InvalidDocumentId { .. } => Code::InvalidDocumentId, UserError::MissingPrimaryKey => Code::MissingPrimaryKey, - UserError::PrimaryKeyCannotBeChanged => Code::PrimaryKeyAlreadyPresent, - UserError::PrimaryKeyCannotBeReset => Code::PrimaryKeyAlreadyPresent, + UserError::PrimaryKeyCannotBeChanged(_) => Code::PrimaryKeyAlreadyPresent, UserError::SortRankingRuleMissing => Code::Sort, - UserError::UnknownInternalDocumentId { .. } => Code::DocumentNotFound, UserError::InvalidFacetsDistribution { .. } => Code::BadRequest, UserError::InvalidSortableAttribute { .. } => Code::Sort, UserError::CriterionError(_) => Code::InvalidRankingRule, diff --git a/meilisearch-lib/src/index/error.rs b/meilisearch-lib/src/index/error.rs index 5899b9356..92691cb14 100644 --- a/meilisearch-lib/src/index/error.rs +++ b/meilisearch-lib/src/index/error.rs @@ -11,14 +11,12 @@ pub type Result = std::result::Result; pub enum IndexError { #[error("Internal error: {0}")] Internal(Box), - #[error("Document with id {0} not found.")] + #[error("Document `{0}` not found.")] DocumentNotFound(String), #[error("{0}")] Facet(#[from] FacetError), #[error("{0}")] Milli(#[from] milli::Error), - #[error("A primary key is already present. It's impossible to update it")] - ExistingPrimaryKey, } internal_error!( @@ -35,21 +33,20 @@ impl ErrorCode for IndexError { IndexError::DocumentNotFound(_) => Code::DocumentNotFound, IndexError::Facet(e) => e.error_code(), IndexError::Milli(e) => MilliError(e).error_code(), - IndexError::ExistingPrimaryKey => Code::PrimaryKeyAlreadyPresent, } } } #[derive(Debug, thiserror::Error)] pub enum FacetError { - #[error("Invalid facet expression, expected {}, found: {1}", .0.join(", "))] + #[error("Invalid syntax for the filter parameter: `expected {}, found: {1}`.", .0.join(", "))] InvalidExpression(&'static [&'static str], Value), } impl ErrorCode for FacetError { fn error_code(&self) -> Code { match self { - FacetError::InvalidExpression(_, _) => Code::Facet, + FacetError::InvalidExpression(_, _) => Code::Filter, } } } diff --git a/meilisearch-lib/src/index/updates.rs b/meilisearch-lib/src/index/updates.rs index 772d27d76..1d19b7dd8 100644 --- a/meilisearch-lib/src/index/updates.rs +++ b/meilisearch-lib/src/index/updates.rs @@ -11,7 +11,7 @@ use uuid::Uuid; use crate::index_controller::updates::status::{Failed, Processed, Processing, UpdateResult}; use crate::Update; -use super::error::{IndexError, Result}; +use super::error::Result; use super::index::{Index, IndexMeta}; fn serialize_with_wildcard( @@ -222,9 +222,6 @@ impl Index { match primary_key { Some(primary_key) => { let mut txn = self.write_txn()?; - if self.primary_key(&txn)?.is_some() { - return Err(IndexError::ExistingPrimaryKey); - } let mut builder = UpdateBuilder::new(0).settings(&mut txn, self); builder.set_primary_key(primary_key); builder.execute(|_, _| ())?; diff --git a/meilisearch-lib/src/index_controller/dump_actor/error.rs b/meilisearch-lib/src/index_controller/dump_actor/error.rs index 9831f3931..49da8c90f 100644 --- a/meilisearch-lib/src/index_controller/dump_actor/error.rs +++ b/meilisearch-lib/src/index_controller/dump_actor/error.rs @@ -9,7 +9,7 @@ pub type Result = std::result::Result; pub enum DumpActorError { #[error("Another dump is already in progress")] DumpAlreadyRunning, - #[error("Dump `{0}` not found")] + #[error("Dump `{0}` not found.")] DumpDoesNotExist(String), #[error("Internal error: {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 661b9bde3..7e97b8ce5 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/error.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/error.rs @@ -3,6 +3,7 @@ use std::fmt; use meilisearch_error::{Code, ErrorCode}; use tokio::sync::mpsc::error::SendError as MpscSendError; use tokio::sync::oneshot::error::RecvError as OneshotRecvError; +use uuid::Uuid; use crate::{error::MilliError, index::error::IndexError}; @@ -12,17 +13,19 @@ pub type Result = std::result::Result; pub enum IndexResolverError { #[error("{0}")] IndexError(#[from] IndexError), - #[error("Index already exists")] - IndexAlreadyExists, - #[error("Index {0} not found")] + #[error("Index `{0}` already exists.")] + IndexAlreadyExists(String), + #[error("Index `{0}` not found.")] UnexistingIndex(String), #[error("A primary key is already present. It's impossible to update it")] ExistingPrimaryKey, - #[error("Internal Error: {0}")] + #[error("Internal Error: `{0}`")] Internal(Box), + #[error("Internal Error: Index uuid `{0}` is already assigned.")] + UUIdAlreadyExists(Uuid), #[error("{0}")] Milli(#[from] milli::Error), - #[error("Index must have a valid uid; Index uid can be of type integer or string only composed of alphanumeric characters, hyphens (-) and underscores (_).")] + #[error("`{0}` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).")] BadlyFormatted(String), } @@ -53,10 +56,11 @@ impl ErrorCode for IndexResolverError { fn error_code(&self) -> Code { match self { IndexResolverError::IndexError(e) => e.error_code(), - IndexResolverError::IndexAlreadyExists => Code::IndexAlreadyExists, + IndexResolverError::IndexAlreadyExists(_) => Code::IndexAlreadyExists, IndexResolverError::UnexistingIndex(_) => Code::IndexNotFound, IndexResolverError::ExistingPrimaryKey => Code::PrimaryKeyAlreadyPresent, IndexResolverError::Internal(_) => Code::Internal, + IndexResolverError::UUIdAlreadyExists(_) => Code::Internal, IndexResolverError::Milli(e) => MilliError(e).error_code(), IndexResolverError::BadlyFormatted(_) => Code::InvalidIndexUid, } diff --git a/meilisearch-lib/src/index_controller/index_resolver/index_store.rs b/meilisearch-lib/src/index_controller/index_resolver/index_store.rs index dcc024121..302b23c2f 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/index_store.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/index_store.rs @@ -64,7 +64,7 @@ impl IndexStore for MapIndexStore { } let path = self.path.join(format!("{}", uuid)); if path.exists() { - return Err(IndexResolverError::IndexAlreadyExists); + return Err(IndexResolverError::UUIdAlreadyExists(uuid)); } let index_size = self.index_size; diff --git a/meilisearch-lib/src/index_controller/index_resolver/uuid_store.rs b/meilisearch-lib/src/index_controller/index_resolver/uuid_store.rs index f10bad757..94c3ddbb5 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/uuid_store.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/uuid_store.rs @@ -100,7 +100,7 @@ impl HeedUuidStore { let mut txn = env.write_txn()?; if db.get(&txn, &name)?.is_some() { - return Err(IndexResolverError::IndexAlreadyExists); + return Err(IndexResolverError::IndexAlreadyExists(name)); } db.put(&mut txn, &name, uuid.as_bytes())?; diff --git a/meilisearch-lib/src/index_controller/snapshot.rs b/meilisearch-lib/src/index_controller/snapshot.rs index c0d409990..07bf75199 100644 --- a/meilisearch-lib/src/index_controller/snapshot.rs +++ b/meilisearch-lib/src/index_controller/snapshot.rs @@ -222,10 +222,11 @@ mod test { setup(); let mut uuid_store = MockUuidStore::new(); - uuid_store - .expect_snapshot() - .once() - .returning(move |_| Box::pin(err(IndexResolverError::IndexAlreadyExists))); + uuid_store.expect_snapshot().once().returning(move |_| { + Box::pin(err(IndexResolverError::IndexAlreadyExists( + "test".to_string(), + ))) + }); let mut index_store = MockIndexStore::new(); index_store.expect_get().never(); @@ -264,9 +265,9 @@ mod test { let mut indexes = uuids.clone().into_iter().map(|uuid| { let mocker = Mocker::default(); // index returns random error - mocker - .when("snapshot") - .then(|_: &Path| -> IndexResult<()> { Err(IndexError::ExistingPrimaryKey) }); + mocker.when("snapshot").then(|_: &Path| -> IndexResult<()> { + Err(IndexError::DocumentNotFound("1".to_string())) + }); mocker.when("uuid").then(move |_: ()| uuid); Index::faux(mocker) }); diff --git a/meilisearch-lib/src/index_controller/updates/error.rs b/meilisearch-lib/src/index_controller/updates/error.rs index 39a73c7c4..27260079e 100644 --- a/meilisearch-lib/src/index_controller/updates/error.rs +++ b/meilisearch-lib/src/index_controller/updates/error.rs @@ -14,7 +14,7 @@ pub type Result = std::result::Result; #[derive(Debug, thiserror::Error)] #[allow(clippy::large_enum_variant)] pub enum UpdateLoopError { - #[error("Update {0} not found.")] + #[error("Task `{0}` not found.")] UnexistingUpdate(u64), #[error("Internal error: {0}")] Internal(Box), @@ -24,9 +24,8 @@ pub enum UpdateLoopError { FatalUpdateStoreError, #[error("{0}")] DocumentFormatError(#[from] DocumentFormatError), - // TODO: The reference to actix has to go. - #[error("{0}")] - PayloadError(#[from] actix_web::error::PayloadError), + #[error("The provided payload reached the size limit.")] + PayloadTooLarge, #[error("A {0} payload is missing.")] MissingPayload(DocumentAdditionFormat), #[error("{0}")] @@ -48,6 +47,15 @@ impl From for UpdateLoopError { } } +impl From for UpdateLoopError { + fn from(other: actix_web::error::PayloadError) -> Self { + match other { + actix_web::error::PayloadError::Overflow => Self::PayloadTooLarge, + _ => Self::Internal(Box::new(other)), + } + } +} + internal_error!( UpdateLoopError: heed::Error, std::io::Error, @@ -59,14 +67,11 @@ internal_error!( impl ErrorCode for UpdateLoopError { fn error_code(&self) -> Code { match self { - Self::UnexistingUpdate(_) => Code::NotFound, + Self::UnexistingUpdate(_) => Code::TaskNotFound, Self::Internal(_) => Code::Internal, Self::FatalUpdateStoreError => Code::Internal, Self::DocumentFormatError(error) => error.error_code(), - Self::PayloadError(error) => match error { - actix_web::error::PayloadError::Overflow => Code::PayloadTooLarge, - _ => Code::Internal, - }, + Self::PayloadTooLarge => Code::PayloadTooLarge, Self::MissingPayload(_) => Code::MissingPayload, Self::IndexError(e) => e.error_code(), } diff --git a/meilisearch-lib/src/index_controller/updates/mod.rs b/meilisearch-lib/src/index_controller/updates/mod.rs index 07ceed92b..3f2a2ef32 100644 --- a/meilisearch-lib/src/index_controller/updates/mod.rs +++ b/meilisearch-lib/src/index_controller/updates/mod.rs @@ -174,15 +174,18 @@ impl UpdateLoop { } let reader = Cursor::new(buffer); - match format { + let document_count = match format { DocumentAdditionFormat::Json => read_json(reader, &mut *update_file)?, DocumentAdditionFormat::Csv => read_csv(reader, &mut *update_file)?, DocumentAdditionFormat::Ndjson => read_ndjson(reader, &mut *update_file)?, + }; + + if document_count > 0 { + update_file.persist()?; + Ok(()) + } else { + Err(UpdateLoopError::MissingPayload(format)) } - - update_file.persist()?; - - Ok(()) }) .await??; diff --git a/meilisearch-lib/src/index_controller/updates/store/mod.rs b/meilisearch-lib/src/index_controller/updates/store/mod.rs index 81525c3fd..336d648a0 100644 --- a/meilisearch-lib/src/index_controller/updates/store/mod.rs +++ b/meilisearch-lib/src/index_controller/updates/store/mod.rs @@ -731,7 +731,7 @@ mod test { mocker .when::>("handle_update") .once() - .then(|update| Err(update.fail(IndexError::ExistingPrimaryKey))); + .then(|update| Err(update.fail(IndexError::DocumentNotFound("1".to_string())))); Box::pin(ok(Some(Index::faux(mocker)))) });