integrate milli errors

meilisearch-http/Cargo.toml

@@ -51,7 +51,7 @@ main_error = "0.1.0"
 meilisearch-error = { path = "../meilisearch-error" }
 meilisearch-tokenizer = { git = "https://github.com/meilisearch/Tokenizer.git", tag = "v0.2.2" }
 memmap = "0.7.0"
-milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.4.0" }
+milli = { git = "https://github.com/meilisearch/milli.git", rev = "70bee7d405711d5e6d24b62710e92671be5ac67a" }
 mime = "0.3.16"
 once_cell = "1.5.2"
 oxidized-json-checker = "0.3.2"

meilisearch-http/src/error.rs

@@ -7,6 +7,7 @@ use actix_web::body::Body;
 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;
@@ -139,3 +140,44 @@ macro_rules! internal_error {
         )*
     }
 }
+
+#[derive(Debug)]
+pub struct MilliError<'a>(pub &'a milli::Error);
+
+impl Error for MilliError<'_> {}
+
+impl fmt::Display for MilliError<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
+impl ErrorCode for MilliError<'_> {
+    fn error_code(&self) -> Code {
+        match self.0 {
+            milli::Error::InternalError(_) => Code::Internal,
+            milli::Error::IoError(_) => Code::Internal,
+            milli::Error::UserError(ref error) => {
+                match error {
+                    // TODO: wait for spec for new error codes.
+                    UserError::AttributeLimitReached
+                    | UserError::Csv(_)
+                    | UserError::SerdeJson(_)
+                    | UserError::MaxDatabaseSizeReached
+                    | UserError::InvalidCriterionName { .. }
+                    | UserError::InvalidDocumentId { .. }
+                    | UserError::InvalidStoreFile
+                    | UserError::NoSpaceLeftOnDevice
+                    | UserError::DocumentLimitReached => todo!(),
+                    UserError::InvalidFilter(_) => Code::Filter,
+                    UserError::InvalidFilterAttribute(_) => Code::Filter,
+                    UserError::MissingDocumentId { .. } => Code::MissingDocumentId,
+                    UserError::MissingPrimaryKey => Code::MissingPrimaryKey,
+                    UserError::PrimaryKeyCannotBeChanged => Code::PrimaryKeyAlreadyPresent,
+                    UserError::PrimaryKeyCannotBeReset => Code::PrimaryKeyAlreadyPresent,
+                    UserError::UnknownInternalDocumentId { .. } => Code::DocumentNotFound,
+                }
+            },
+        }
+    }
+}

meilisearch-http/src/index/dump.rs

@@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize};
 
 use crate::option::IndexerOpts;
 
-use super::error::{IndexError, Result};
+use super::error::Result;
 use super::{update_handler::UpdateHandler, Index, Settings, Unchecked};
 
 #[derive(Serialize, Deserialize)]
@@ -38,9 +38,7 @@ impl Index {
         let document_file_path = path.as_ref().join(DATA_FILE_NAME);
         let mut document_file = File::create(&document_file_path)?;
 
-        let documents = self
-            .all_documents(txn)
-            .map_err(|e| IndexError::Internal(e.into()))?;
+        let documents = self.all_documents(txn)?;
         let fields_ids_map = self.fields_ids_map(txn)?;
 
         // dump documents

meilisearch-http/src/index/error.rs

@@ -3,6 +3,8 @@ use std::error::Error;
 use meilisearch_error::{Code, ErrorCode};
 use serde_json::Value;
 
+use crate::error::MilliError;
+
 pub type Result<T> = std::result::Result<T, IndexError>;
 
 #[derive(Debug, thiserror::Error)]
@@ -13,6 +15,8 @@ pub enum IndexError {
     DocumentNotFound(String),
     #[error("error with facet: {0}")]
     Facet(#[from] FacetError),
+    #[error("{0}")]
+    Milli(#[from] milli::Error),
 }
 
 internal_error!(
@@ -29,6 +33,7 @@ impl ErrorCode for IndexError {
             IndexError::Internal(_) => Code::Internal,
             IndexError::DocumentNotFound(_) => Code::DocumentNotFound,
             IndexError::Facet(e) => e.error_code(),
+            IndexError::Milli(e) => MilliError(e).error_code(),
         }
     }
 }

meilisearch-http/src/index/mod.rs

@@ -51,8 +51,7 @@ impl Index {
         create_dir_all(&path)?;
         let mut options = EnvOpenOptions::new();
         options.map_size(size);
-        let index =
-            milli::Index::new(options, &path).map_err(|e| IndexError::Internal(e.into()))?;
+        let index = milli::Index::new(options, &path)?;
         Ok(Index(Arc::new(index)))
     }
 
@@ -70,11 +69,7 @@ impl Index {
             .searchable_fields(&txn)?
             .map(|fields| fields.into_iter().map(String::from).collect());
 
-        let faceted_attributes = self
-            .faceted_fields(&txn)
-            .map_err(|e| IndexError::Internal(Box::new(e)))?
-            .into_iter()
-            .collect();
+        let faceted_attributes = self.faceted_fields(&txn)?.into_iter().collect();
 
         let criteria = self
             .criteria(&txn)?
@@ -83,8 +78,7 @@ impl Index {
             .collect();
 
         let stop_words = self
-            .stop_words(&txn)
-            .map_err(|e| IndexError::Internal(e.into()))?
+            .stop_words(&txn)?
             .map(|stop_words| -> Result<BTreeSet<_>> {
                 Ok(stop_words.stream().into_strs()?.into_iter().collect())
             })
@@ -126,18 +120,16 @@ impl Index {
         let txn = self.read_txn()?;
 
         let fields_ids_map = self.fields_ids_map(&txn)?;
-        let fields_to_display = self
-            .fields_to_display(&txn, &attributes_to_retrieve, &fields_ids_map)
-            .map_err(|e| IndexError::Internal(e.into()))?;
+        let fields_to_display =
+            self.fields_to_display(&txn, &attributes_to_retrieve, &fields_ids_map)?;
 
         let iter = self.documents.range(&txn, &(..))?.skip(offset).take(limit);
 
         let mut documents = Vec::new();
 
         for entry in iter {
             let (_id, obkv) = entry?;
-            let object = obkv_to_json(&fields_to_display, &fields_ids_map, obkv)
-                .map_err(|e| IndexError::Internal(e.into()))?;
+            let object = obkv_to_json(&fields_to_display, &fields_ids_map, obkv)?;
             documents.push(object);
         }
 
@@ -153,31 +145,24 @@ impl Index {
 
         let fields_ids_map = self.fields_ids_map(&txn)?;
 
-        let fields_to_display = self
-            .fields_to_display(&txn, &attributes_to_retrieve, &fields_ids_map)
-            .map_err(|e| IndexError::Internal(e.into()))?;
+        let fields_to_display =
+            self.fields_to_display(&txn, &attributes_to_retrieve, &fields_ids_map)?;
 
         let internal_id = self
-            .external_documents_ids(&txn)
-            .map_err(|e| IndexError::Internal(e.into()))?
+            .external_documents_ids(&txn)?
             .get(doc_id.as_bytes())
             .ok_or_else(|| IndexError::DocumentNotFound(doc_id.clone()))?;
 
         let document = self
-            .documents(&txn, std::iter::once(internal_id))
-            .map_err(|e| IndexError::Internal(e.into()))?
+            .documents(&txn, std::iter::once(internal_id))?
             .into_iter()
             .next()
-            .map(|(_, d)| d);
-
-        match document {
-            Some(document) => {
-                let document = obkv_to_json(&fields_to_display, &fields_ids_map, document)
-                    .map_err(|e| IndexError::Internal(e.into()))?;
-                Ok(document)
-            }
-            None => Err(IndexError::DocumentNotFound(doc_id)),
-        }
+            .map(|(_, d)| d)
+            .ok_or(IndexError::DocumentNotFound(doc_id))?;
+
+        let document = obkv_to_json(&fields_to_display, &fields_ids_map, document)?;
+
+        Ok(document)
     }
 
     pub fn size(&self) -> u64 {
@@ -190,8 +175,7 @@ impl Index {
         attributes_to_retrieve: &Option<Vec<S>>,
         fields_ids_map: &milli::FieldsIdsMap,
     ) -> Result<Vec<u8>> {
-        let mut displayed_fields_ids = match self.displayed_fields_ids(&txn)
-            .map_err(|e| IndexError::Internal(Box::new(e)))? {
+        let mut displayed_fields_ids = match self.displayed_fields_ids(&txn)? {
             Some(ids) => ids.into_iter().collect::<Vec<_>>(),
             None => fields_ids_map.iter().map(|(id, _)| id).collect(),
         };

meilisearch-http/src/index/search.rs

@@ -13,7 +13,7 @@ use serde_json::Value;
 
 use crate::index::error::FacetError;
 
-use super::error::{IndexError, Result};
+use super::error::Result;
 use super::Index;
 
 pub type Document = IndexMap<String, Value>;
@@ -87,14 +87,12 @@ impl Index {
             matching_words,
             candidates,
             ..
-        } = search
-            .execute()
-            .map_err(|e| IndexError::Internal(e.into()))?;
+        } = search.execute()?;
+
         let fields_ids_map = self.fields_ids_map(&rtxn).unwrap();
 
         let displayed_ids = self
-            .displayed_fields_ids(&rtxn)
-            .map_err(|e| IndexError::Internal(Box::new(e)))?
+            .displayed_fields_ids(&rtxn)?
             .map(|fields| fields.into_iter().collect::<HashSet<_>>())
             .unwrap_or_else(|| fields_ids_map.iter().map(|(id, _)| id).collect());
 
@@ -164,9 +162,7 @@ impl Index {
             Highlighter::new(&stop_words, (String::from("<em>"), String::from("</em>")));
 
         let mut documents = Vec::new();
-        let documents_iter = self
-            .documents(&rtxn, documents_ids)
-            .map_err(|e| IndexError::Internal(e.into()))?;
+        let documents_iter = self.documents(&rtxn, documents_ids)?;
 
         for (_id, obkv) in documents_iter {
             let document = make_document(&all_attributes, &fields_ids_map, obkv)?;
@@ -195,8 +191,7 @@ impl Index {
                 }
                 let distribution = facet_distribution
                     .candidates(candidates)
-                    .execute()
-                    .map_err(|e| IndexError::Internal(e.into()))?;
+                    .execute()?;
 
                 Some(distribution)
             }
@@ -348,8 +343,7 @@ impl<'a, A: AsRef<[u8]>> Highlighter<'a, A> {
 fn parse_facets(facets: &Value, index: &Index, txn: &RoTxn) -> Result<Option<FilterCondition>> {
     match facets {
         Value::String(expr) => {
-            let condition = FilterCondition::from_str(txn, index, expr)
-                .map_err(|e| IndexError::Internal(e.into()))?;
+            let condition = FilterCondition::from_str(txn, index, expr)?;
             Ok(Some(condition))
         }
         Value::Array(arr) => parse_facets_array(txn, index, arr),
@@ -386,7 +380,7 @@ fn parse_facets_array(
         }
     }
 
-    FilterCondition::from_array(txn, &index.0, ands).map_err(|e| IndexError::Internal(Box::new(e)))
+    Ok(FilterCondition::from_array(txn, &index.0, ands)?)
 }
 
 #[cfg(test)]

meilisearch-http/src/index/updates.rs

@@ -8,7 +8,6 @@ use log::info;
 use milli::update::{IndexDocumentsMethod, UpdateBuilder, UpdateFormat};
 use serde::{Deserialize, Serialize, Serializer};
 
-use crate::index::error::IndexError;
 use crate::index_controller::UpdateResult;
 
 use super::error::Result;
@@ -206,11 +205,9 @@ impl Index {
 
         // Set the primary key if not set already, ignore if already set.
         if let (None, Some(primary_key)) = (self.primary_key(txn)?, primary_key) {
-            let mut builder = UpdateBuilder::new(0)
-                .settings(txn, &self);
+            let mut builder = UpdateBuilder::new(0).settings(txn, &self);
             builder.set_primary_key(primary_key.to_string());
-            builder.execute(|_, _| ())
-                .map_err(|e| IndexError::Internal(Box::new(e)))?;
+            builder.execute(|_, _| ())?;
         }
 
         let mut builder = update_builder.index_documents(txn, self);
@@ -222,15 +219,9 @@ impl Index {
 
         let gzipped = false;
         let addition = match content {
-            Some(content) if gzipped => builder
-                .execute(GzDecoder::new(content), indexing_callback)
-                .map_err(|e| IndexError::Internal(e.into()))?,
-            Some(content) => builder
-                .execute(content, indexing_callback)
-                .map_err(|e| IndexError::Internal(e.into()))?,
-            None => builder
-                .execute(std::io::empty(), indexing_callback)
-                .map_err(|e| IndexError::Internal(e.into()))?,
+            Some(content) if gzipped => builder.execute(GzDecoder::new(content), indexing_callback)?,
+            Some(content) => builder.execute(content, indexing_callback)?,
+            None => builder.execute(std::io::empty(), indexing_callback)?,
         };
 
         info!("document addition done: {:?}", addition);
@@ -243,13 +234,11 @@ impl Index {
         let mut wtxn = self.write_txn()?;
         let builder = update_builder.clear_documents(&mut wtxn, self);
 
-        match builder.execute() {
-            Ok(_count) => wtxn
-                .commit()
-                .and(Ok(UpdateResult::Other))
-                .map_err(Into::into),
-            Err(e) => Err(IndexError::Internal(Box::new(e))),
-        }
+        let _count = builder.execute()?;
+
+        wtxn.commit()
+            .and(Ok(UpdateResult::Other))
+            .map_err(Into::into)
     }
 
     pub fn update_settings_txn<'a, 'b>(
@@ -308,9 +297,7 @@ impl Index {
             }
         }
 
-        builder
-            .execute(|indexing_step, update_id| info!("update {}: {:?}", update_id, indexing_step))
-            .map_err(|e| IndexError::Internal(e.into()))?;
+        builder.execute(|indexing_step, update_id| info!("update {}: {:?}", update_id, indexing_step))?;
 
         Ok(UpdateResult::Other)
     }
@@ -332,22 +319,17 @@ impl Index {
         update_builder: UpdateBuilder,
     ) -> Result<UpdateResult> {
         let mut txn = self.write_txn()?;
-        let mut builder = update_builder
-            .delete_documents(&mut txn, self)
-            .map_err(|e| IndexError::Internal(e.into()))?;
+        let mut builder = update_builder.delete_documents(&mut txn, self)?;
 
         // We ignore unexisting document ids
         document_ids.iter().for_each(|id| {
             builder.delete_external_id(id);
         });
 
-        match builder.execute() {
-            Ok(deleted) => txn
-                .commit()
-                .and(Ok(UpdateResult::DocumentDeletion { deleted }))
-                .map_err(Into::into),
-            Err(e) => Err(IndexError::Internal(Box::new(e))),
-        }
+        let deleted = builder.execute()?;
+        txn.commit()
+            .and(Ok(UpdateResult::DocumentDeletion { deleted }))
+            .map_err(Into::into)
     }
 }