review and fix all error codes

crates/meilisearch-types/src/error.rs

@@ -5,6 +5,7 @@ use actix_web::{self as aweb, HttpResponseBuilder};
 use aweb::http::header;
 use aweb::rt::task::JoinError;
 use convert_case::Casing;
+use milli::cellulite;
 use milli::heed::{Error as HeedError, MdbError};
 use serde::{Deserialize, Serialize};
 use utoipa::ToSchema;
@@ -239,6 +240,7 @@ InconsistentDocumentChangeHeaders              , InvalidRequest       , BAD_REQU
 InvalidDocumentFilter                          , InvalidRequest       , BAD_REQUEST ;
 InvalidDocumentSort                            , InvalidRequest       , BAD_REQUEST ;
 InvalidDocumentGeoField                        , InvalidRequest       , BAD_REQUEST ;
+InvalidDocumentGeojsonField                    , InvalidRequest       , BAD_REQUEST ;
 InvalidHeaderValue                             , InvalidRequest       , BAD_REQUEST ;
 InvalidVectorDimensions                        , InvalidRequest       , BAD_REQUEST ;
 InvalidVectorsType                             , InvalidRequest       , BAD_REQUEST ;
@@ -527,7 +529,16 @@ impl ErrorCode for milli::Error {
                     | UserError::DocumentEditionCompilationError(_) => {
                         Code::EditDocumentsByFunctionError
                     }
-                    UserError::CelluliteError(_) => Code::Internal,
+                    UserError::CelluliteError(err) => match err {
+                        cellulite::Error::BuildCanceled
+                        | cellulite::Error::VersionMismatchOnBuild(_)
+                        | cellulite::Error::DatabaseDoesntExists
+                        | cellulite::Error::Heed(_)
+                        | cellulite::Error::InvalidGeometry(_)
+                        | cellulite::Error::InternalDocIdMissing(_, _)
+                        | cellulite::Error::CannotConvertLineToCell(_, _, _) => Code::Internal,
+                        cellulite::Error::InvalidGeoJson(_) => Code::InvalidDocumentGeojsonField,
+                    },
                 }
             }
         }

crates/milli/src/error.rs

@@ -121,6 +121,8 @@ pub enum FieldIdMapMissingEntry {
 pub enum UserError {
     #[error(transparent)]
     CelluliteError(#[from] cellulite::Error),
+    #[error("Malformed geojson: {0}")]
+    MalformedGeojson(serde_json::Error),
     #[error("A document cannot contain more than 65,535 fields.")]
     AttributeLimitReached,
     #[error(transparent)]

crates/milli/src/lib.rs

@@ -54,7 +54,7 @@ pub use search::new::{
 };
 use serde_json::Value;
 pub use thread_pool_no_abort::{PanicCatched, ThreadPoolNoAbort, ThreadPoolNoAbortBuilder};
-pub use {arroy, charabia as tokenizer, hannoy, heed, rhai};
+pub use {arroy, cellulite, charabia as tokenizer, hannoy, heed, rhai};
 
 pub use self::asc_desc::{AscDesc, AscDescError, Member, SortError};
 pub use self::attribute_patterns::{AttributePatterns, PatternMatch};

crates/milli/src/search/facet/filter.rs

@@ -870,10 +870,7 @@ impl<'a> Filter<'a> {
                         Vec::new(),
                     );
 
-                    let result = index
-                        .cellulite
-                        .in_shape(rtxn, &polygon)
-                        .map_err(InternalError::CelluliteError)?; // TODO: error code in invalid
+                    let result = index.cellulite.in_shape(rtxn, &polygon)?;
 
                     r2 = Some(RoaringBitmap::from_iter(result)); // TODO: Remove once we update roaring in meilisearch
                 }
@@ -922,10 +919,7 @@ impl<'a> Filter<'a> {
                 }
 
                 let polygon = geo_types::Polygon::new(geo_types::LineString(coords), Vec::new());
-                let result = index
-                    .cellulite
-                    .in_shape(rtxn, &polygon)
-                    .map_err(InternalError::CelluliteError)?; // TODO: update error code
+                let result = index.cellulite.in_shape(rtxn, &polygon)?;
 
                 let result = roaring::RoaringBitmap::from_iter(result); // TODO: Remove once we update roaring in meilisearch
 

crates/milli/src/update/index_documents/extract/extract_geo_points.rs

@@ -10,7 +10,7 @@ use crate::error::GeoError;
 use crate::update::del_add::{DelAdd, KvReaderDelAdd, KvWriterDelAdd};
 use crate::update::index_documents::extract_finite_float_from_value;
 use crate::update::settings::{InnerIndexSettings, InnerIndexSettingsDiff};
-use crate::{DocumentId, FieldId, InternalError, Result};
+use crate::{DocumentId, FieldId, InternalError, Result, UserError};
 
 /// Extracts the geographical coordinates contained in each document under the `_geo` field.
 ///
@@ -174,9 +174,8 @@ fn extract_geojson_field(
     match settings.geojson_fid {
         Some(fid) if settings.filterable_attributes_rules.iter().any(|rule| rule.has_geojson()) => {
             let value = obkv.get(fid).map(KvReaderDelAdd::from_slice).and_then(|r| r.get(deladd));
-            // TODO: That's a user error, not an internal error
             Ok(value
-                .map(|v| serde_json::from_slice(v).map_err(InternalError::SerdeJson))
+                .map(|v| GeoJson::from_reader(v).map_err(UserError::MalformedGeojson))
                 .transpose()?)
         }
         _ => Ok(None),

crates/milli/src/update/index_documents/extract/mod.rs

@@ -244,9 +244,7 @@ fn send_original_documents_data(
     let original_documents_chunk =
         original_documents_chunk.and_then(|c| unsafe { as_cloneable_grenad(&c) })?;
 
-    tracing::warn!("Do we have a geojson");
     if settings_diff.reindex_geojson() {
-        tracing::warn!("Yes we do");
         let documents_chunk_cloned = original_documents_chunk.clone();
         let lmdb_writer_sx_cloned = lmdb_writer_sx.clone();
         let settings_diff = settings_diff.clone();