From d8c649b3cd1bdd6a0be8f2525ca8a9376aa143bd Mon Sep 17 00:00:00 2001 From: bwbonanno Date: Wed, 18 Oct 2023 08:28:24 -0700 Subject: [PATCH] Return recoverable error if we fail to retrieve metrics state --- index-scheduler/src/error.rs | 4 ++++ meilisearch/src/middleware.rs | 23 +++++++++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/index-scheduler/src/error.rs b/index-scheduler/src/error.rs index ddc6960f7..417a55be8 100644 --- a/index-scheduler/src/error.rs +++ b/index-scheduler/src/error.rs @@ -113,6 +113,8 @@ pub enum Error { Dump(#[from] dump::Error), #[error(transparent)] Heed(#[from] heed::Error), + #[error("Unable to record metrics for request.")] + CannotRecordMetrics, #[error(transparent)] Milli(#[from] milli::Error), #[error("An unexpected crash occurred when processing the task.")] @@ -177,6 +179,7 @@ impl Error { | Error::TaskCancelationWithEmptyQuery | Error::Dump(_) | Error::Heed(_) + | Error::CannotRecordMetrics | Error::Milli(_) | Error::ProcessBatchPanicked | Error::FileStore(_) @@ -223,6 +226,7 @@ impl ErrorCode for Error { Error::Milli(e) => e.error_code(), Error::ProcessBatchPanicked => Code::Internal, Error::Heed(e) => e.error_code(), + Error::CannotRecordMetrics => Code::Internal, Error::HeedTransaction(e) => e.error_code(), Error::FileStore(e) => e.error_code(), Error::IoError(e) => e.error_code(), diff --git a/meilisearch/src/middleware.rs b/meilisearch/src/middleware.rs index de74b3a37..7f7429186 100644 --- a/meilisearch/src/middleware.rs +++ b/meilisearch/src/middleware.rs @@ -6,7 +6,9 @@ use actix_web::dev::{self, Service, ServiceRequest, ServiceResponse, Transform}; use actix_web::web::Data; use actix_web::Error; use futures_util::future::LocalBoxFuture; +use index_scheduler::Error::CannotRecordMetrics; use index_scheduler::IndexScheduler; +use meilisearch_types::error::ResponseError; use prometheus::HistogramTimer; pub struct RouteMetrics; @@ -49,15 +51,20 @@ where fn call(&self, req: ServiceRequest) -> Self::Future { let mut histogram_timer: Option = None; - let data = req.app_data::>(); - let metrics_enabled = data - .ok_or("Could not get index scheduler") - .and_then(|index_scheduler| { - index_scheduler.features().map_err(|_| "Could not get features") - }) - .and_then(|features| features.check_metrics().map_err(|_| "Metrics not enabled")); - if let Ok(()) = metrics_enabled { + // calling unwrap here is safe because index scheduler is added to app data while creating actix app. + // also, the tests will fail if this is not present. + let index_scheduler = req.app_data::>().unwrap(); + let features = match index_scheduler.features() { + Ok(features) => features, + Err(_e) => { + return Box::pin( + async move { Err(ResponseError::from(CannotRecordMetrics).into()) }, + ); + } + }; + + if features.check_metrics().is_ok() { let request_path = req.path(); let is_registered_resource = req.resource_map().has_resource(request_path); if is_registered_resource {