Fix some errors

This commit is contained in:
many 2021-10-26 19:36:48 +02:00
parent 61c15b69fb
commit 7464720426
No known key found for this signature in database
GPG key ID: 2CEF23B75189EACA
27 changed files with 347 additions and 234 deletions

View file

@ -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<dyn std::error::Error + Send + Sync + 'static>),
#[error("The {1} payload provided is malformed. {0}.")]
#[error("The `{1}` payload provided is malformed. `{0}`.")]
MalformedPayload(
Box<dyn std::error::Error + Send + Sync + 'static>,
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<usize> {
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<usize> {
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<usize> {
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)
}

View file

@ -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,

View file

@ -11,14 +11,12 @@ pub type Result<T> = std::result::Result<T, IndexError>;
pub enum IndexError {
#[error("Internal error: {0}")]
Internal(Box<dyn Error + Send + Sync + 'static>),
#[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,
}
}
}

View file

@ -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<S>(
@ -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(|_, _| ())?;

View file

@ -9,7 +9,7 @@ pub type Result<T> = std::result::Result<T, DumpActorError>;
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<dyn std::error::Error + Send + Sync + 'static>),

View file

@ -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<T> = std::result::Result<T, IndexResolverError>;
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<dyn std::error::Error + Send + Sync + 'static>),
#[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,
}

View file

@ -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;

View file

@ -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())?;

View file

@ -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)
});

View file

@ -14,7 +14,7 @@ pub type Result<T> = std::result::Result<T, UpdateLoopError>;
#[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<dyn Error + Send + Sync + 'static>),
@ -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<tokio::sync::oneshot::error::RecvError> for UpdateLoopError {
}
}
impl From<actix_web::error::PayloadError> 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(),
}

View file

@ -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??;

View file

@ -731,7 +731,7 @@ mod test {
mocker
.when::<Processing, std::result::Result<Processed, Failed>>("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))))
});