Merge pull request #792 from MarinPostma/error-codes-in-updates

Error codes in updates

meilisearch-core/src/error.rs

@@ -124,7 +124,10 @@ impl From<BincodeError> for Error {
 
 impl From<SerializerError> for Error {
     fn from(error: SerializerError) -> Error {
-        Error::Serializer(error)
+        match error {
+            SerializerError::DocumentIdNotFound => Error::MissingDocumentId,
+            e => Error::Serializer(e),
+        }
     }
 }
 

meilisearch-core/src/update/mod.rs

@@ -24,6 +24,8 @@ use sdset::Set;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 
+use meilisearch_error::ErrorCode;
+
 use crate::{store, MResult};
 use crate::database::{MainT, UpdateT};
 use crate::settings::SettingsUpdate;
@@ -128,6 +130,12 @@ pub struct ProcessedUpdateResult {
     pub update_type: UpdateType,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub error: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub error_type: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub error_code: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub error_link: Option<String>,
     pub duration: f64, // in seconds
     pub enqueued_at: DateTime<Utc>,
     pub processed_at: DateTime<Utc>,
@@ -288,7 +296,10 @@ pub fn update_task<'a, 'b>(
     let status = ProcessedUpdateResult {
         update_id,
         update_type,
-        error: result.map_err(|e| e.to_string()).err(),
+        error: result.as_ref().map_err(|e| e.to_string()).err(),
+        error_code: result.as_ref().map_err(|e| e.error_name()).err(),
+        error_type: result.as_ref().map_err(|e| e.error_type()).err(),
+        error_link: result.as_ref().map_err(|e| e.error_url()).err(),
         duration: duration.as_secs_f64(),
         enqueued_at,
         processed_at: Utc::now(),

meilisearch-error/src/lib.rs

@@ -86,26 +86,32 @@ impl Code {
 
         match self {
             // index related errors
+            // create index is thrown on internal error while creating an index.
             CreateIndex => ErrCode::invalid("index_creation_failed", StatusCode::BAD_REQUEST),
             IndexAlreadyExists => ErrCode::invalid("index_already_exists", StatusCode::BAD_REQUEST),
+            // 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),
+            // thrown when no primary key has been set
             MissingPrimaryKey => ErrCode::internal("missing_primary_key", StatusCode::INTERNAL_SERVER_ERROR),
-            PrimaryKeyAlreadyPresent => ErrCode::internal("primary_key_already_present", StatusCode::INTERNAL_SERVER_ERROR),
+            // error thrown when trying to set an already existing primary key
+            PrimaryKeyAlreadyPresent => ErrCode::invalid("primary_key_already_present", StatusCode::BAD_REQUEST),
 
             // invalid document
             MaxFieldsLimitExceeded => ErrCode::invalid("max_field_limit_exceeded", StatusCode::BAD_REQUEST),
             MissingDocumentId => ErrCode::invalid("missing_document_id", StatusCode::BAD_REQUEST),
 
+            // error related to facets
             Facet => ErrCode::invalid("invalid_facet", StatusCode::BAD_REQUEST),
+            // error related to filters
             Filter => ErrCode::invalid("invalid_filter", StatusCode::BAD_REQUEST),
 
             BadParameter => ErrCode::invalid("bad_parameter", StatusCode::BAD_REQUEST),
             BadRequest => ErrCode::invalid("bad_request", StatusCode::BAD_REQUEST),
-            DocumentNotFound => ErrCode::internal("document_not_found", StatusCode::NOT_FOUND),
+            DocumentNotFound => ErrCode::invalid("document_not_found", StatusCode::NOT_FOUND),
             Internal => ErrCode::internal("internal", StatusCode::INTERNAL_SERVER_ERROR),
             InvalidToken => ErrCode::authentication("invalid_token", StatusCode::FORBIDDEN),
             Maintenance =>  ErrCode::internal("maintenance", StatusCode::SERVICE_UNAVAILABLE),

meilisearch-http/src/routes/index.rs

@@ -253,17 +253,8 @@ async fn update_index(
 
         if let Some(id) = body.primary_key.clone() {
             if let Some(mut schema) = index.main.schema(writer)? {
-                match schema.primary_key() {
+                schema.set_primary_key(&id)?;
-                    Some(_) => {
+                index.main.put_schema(writer, &schema)?;
-                        return Err(Error::bad_request(
-                                "The primary key cannot be updated",
-                        ).into());
-                    }
-                    None => {
-                        schema.set_primary_key(&id)?;
-                        index.main.put_schema(writer, &schema)?;
-                    }
-                }
             }
         }
         index.main.put_updated_at(writer)?;

meilisearch-http/tests/common.rs

@@ -111,17 +111,19 @@ impl Server {
 
 
     pub async fn wait_update_id(&mut self, update_id: u64) {
-        loop {
+        // try 10 times to get status, or panic to not wait forever
+        for _ in 0..10 {
             let (response, status_code) = self.get_update_status(update_id).await;
             assert_eq!(status_code, 200);
 
-            if response["status"] == "processed" || response["status"] == "error" {
+            if response["status"] == "processed" || response["status"] == "failed" {
                 eprintln!("{:#?}", response);
                 return;
             }
 
             delay_for(Duration::from_secs(1)).await;
         }
+        panic!("Timeout waiting for update id");
     }
 
     // Global Http request GET/POST/DELETE async or sync

meilisearch-http/tests/errors.rs

@@ -0,0 +1,182 @@
+mod common;
+
+use std::thread;
+use std::time::Duration;
+
+use actix_http::http::StatusCode;
+use serde_json::{json, Map, Value};
+
+macro_rules! assert_error {
+    ($code:literal, $type:literal, $status:path, $req:expr) => {
+        let (response, status_code) = $req;
+        assert_eq!(status_code, $status);
+        assert_eq!(response["errorCode"].as_str().unwrap(), $code);
+        assert_eq!(response["errorType"].as_str().unwrap(), $type);
+    };
+}
+
+macro_rules! assert_error_async {
+    ($code:literal, $type:literal, $server:expr, $req:expr) => {
+        let (response, _) = $req;
+        let update_id = response["updateId"].as_u64().unwrap();
+        for _ in 1..10 {
+            let (response, status_code) = $server.get_update_status(update_id).await;
+            assert_eq!(status_code, StatusCode::OK);
+            if response["status"] == "processed" || response["status"] == "failed" {
+                println!("response: {}", response);
+                assert_eq!(response["status"], "failed");
+                assert_eq!(response["errorCode"], $code);
+                assert_eq!(response["errorType"], $type);
+                return
+            }
+            thread::sleep(Duration::from_secs(1));
+        }
+    };
+}
+
+#[actix_rt::test]
+async fn index_already_exists_error() {
+    let mut server = common::Server::with_uid("test");
+    let body = json!({
+        "uid": "test"
+    });
+    let (response, status_code) = server.create_index(body.clone()).await;
+    println!("{}", response);
+    assert_eq!(status_code, StatusCode::CREATED);
+    assert_error!(
+        "index_already_exists",
+        "invalid_request_error",
+        StatusCode::BAD_REQUEST,
+        server.create_index(body).await);
+}
+
+#[actix_rt::test]
+async fn index_not_found_error() {
+    let mut server = common::Server::with_uid("test");
+    assert_error!(
+        "index_not_found",
+        "invalid_request_error",
+        StatusCode::NOT_FOUND,
+        server.get_index().await);
+}
+
+#[actix_rt::test]
+async fn primary_key_already_present_error() {
+    let mut server = common::Server::with_uid("test");
+    let body = json!({
+        "uid": "test",
+        "primaryKey": "test"
+    });
+    server.create_index(body.clone()).await;
+    let body = json!({
+        "primaryKey": "t"
+    });
+    assert_error!(
+        "primary_key_already_present",
+        "invalid_request_error",
+        StatusCode::BAD_REQUEST,
+        server.update_index(body).await);
+}
+
+#[actix_rt::test]
+async fn max_field_limit_exceeded_error() {
+    let mut server = common::Server::test_server().await;
+    let body = json!({
+        "uid": "test",
+    });
+    server.create_index(body).await;
+    let mut doc = Map::with_capacity(70_000);
+    doc.insert("id".into(), Value::String("foo".into()));
+    for i in 0..69_999 {
+        doc.insert(format!("field{}", i), Value::String("foo".into()));
+    }
+    let docs = json!([doc]);
+    assert_error_async!(
+        "max_field_limit_exceeded",
+        "invalid_request_error",
+        server,
+        server.add_or_replace_multiple_documents_sync(docs).await);
+}
+
+#[actix_rt::test]
+async fn missing_document_id() {
+    let mut server = common::Server::test_server().await;
+    let body = json!({
+        "uid": "test",
+        "primaryKey": "test"
+    });
+    server.create_index(body).await;
+    let docs = json!([
+        {
+            "foo": "bar",
+        }
+    ]);
+    assert_error_async!(
+        "missing_document_id",
+        "invalid_request_error",
+        server,
+        server.add_or_replace_multiple_documents_sync(docs).await);
+}
+
+#[actix_rt::test]
+async fn facet_error() {
+    let mut server = common::Server::test_server().await;
+    let search = json!({
+        "q": "foo",
+        "facetFilters": ["test:hello"]
+    });
+    assert_error!(
+        "invalid_facet",
+        "invalid_request_error",
+        StatusCode::BAD_REQUEST,
+        server.search_post(search).await);
+}
+
+#[actix_rt::test]
+async fn filters_error() {
+    let mut server = common::Server::test_server().await;
+    let search = json!({
+        "q": "foo",
+        "filters": "fo:12"
+    });
+    assert_error!(
+        "invalid_filter",
+        "invalid_request_error",
+        StatusCode::BAD_REQUEST,
+        server.search_post(search).await);
+}
+
+#[actix_rt::test]
+async fn bad_request_error() {
+    let mut server = common::Server::with_uid("test");
+    let body = json!({
+        "foo": "bar",
+    });
+    assert_error!(
+        "bad_request",
+        "invalid_request_error",
+        StatusCode::BAD_REQUEST,
+        server.search_post(body).await);
+}
+
+#[actix_rt::test]
+async fn document_not_found_error() {
+    let mut server = common::Server::with_uid("test");
+    server.create_index(json!({"uid": "test"})).await;
+    assert_error!(
+        "document_not_found",
+        "invalid_request_error",
+        StatusCode::NOT_FOUND,
+        server.get_document(100).await);
+}
+
+#[actix_rt::test]
+async fn payload_too_large_error() {
+    let mut server = common::Server::with_uid("test");
+    let bigvec = vec![0u64; 10_000_000]; // 80mb
+    assert_error!(
+        "payload_too_large",
+        "invalid_request_error",
+        StatusCode::PAYLOAD_TOO_LARGE,
+        server.create_index(json!(bigvec)).await);
+}