handle and tests errors

meilisearch/src/error.rs

@@ -61,7 +61,6 @@ impl ErrorCode for MeilisearchHttpError {
     fn error_code(&self) -> Code {
         match self {
             MeilisearchHttpError::MissingContentType(_) => Code::MissingContentType,
-            /// TODO: TAMO: create a new error code
             MeilisearchHttpError::AlreadyUsedLogRoute => Code::BadRequest,
             MeilisearchHttpError::CsvDelimiterWithWrongContentType(_) => Code::InvalidContentType,
             MeilisearchHttpError::MissingPayload(_) => Code::MissingPayload,

meilisearch/src/routes/logs.rs

@@ -1,19 +1,21 @@
-use std::fmt;
+use std::convert::Infallible;
 use std::io::Write;
+use std::ops::ControlFlow;
 use std::pin::Pin;
 use std::str::FromStr;
 use std::sync::Arc;
 
 use actix_web::web::{Bytes, Data};
-use actix_web::{web, HttpRequest, HttpResponse};
+use actix_web::{web, HttpResponse};
 use deserr::actix_web::AwebJson;
-use deserr::Deserr;
+use deserr::{DeserializeError, Deserr, ErrorKind, MergeWithError, ValuePointerRef};
 use futures_util::Stream;
 use meilisearch_auth::AuthController;
 use meilisearch_types::deserr::DeserrJsonError;
 use meilisearch_types::error::deserr_codes::*;
 use meilisearch_types::error::{Code, ResponseError};
 use tokio::sync::mpsc::{self};
+use tracing_subscriber::filter::Targets;
 use tracing_subscriber::Layer;
 
 use crate::error::MeilisearchHttpError;
@@ -30,17 +32,6 @@ pub fn configure(cfg: &mut web::ServiceConfig) {
     );
 }
 
-#[derive(Debug, Default, Clone, Copy, Deserr)]
-#[deserr(rename_all = lowercase)]
-pub enum LogLevel {
-    Error,
-    Warn,
-    #[default]
-    Info,
-    Debug,
-    Trace,
-}
-
 #[derive(Debug, Default, Clone, Copy, Deserr)]
 #[deserr(rename_all = lowercase)]
 pub enum LogMode {
@@ -49,38 +40,61 @@ pub enum LogMode {
     Profile,
 }
 
+/// Simple wrapper around the `Targets` from `tracing_subscriber` to implement `MergeWithError` on it.
+#[derive(Clone, Debug)]
+struct MyTargets(Targets);
+
+/// Simple wrapper around the `ParseError` from `tracing_subscriber` to implement `MergeWithError` on it.
+#[derive(Debug, thiserror::Error)]
+enum MyParseError {
+    #[error(transparent)]
+    ParseError(#[from] tracing_subscriber::filter::ParseError),
+    #[error(
+        "Empty string is not a valid target. If you want to get no logs use `OFF`. Usage: `info`, `info:meilisearch`, or you can write multiple filters in one target: `index_scheduler=info,milli=trace`"
+    )]
+    Example,
+}
+
+impl FromStr for MyTargets {
+    type Err = MyParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.is_empty() {
+            Err(MyParseError::Example)
+        } else {
+            Ok(MyTargets(Targets::from_str(s).map_err(MyParseError::ParseError)?))
+        }
+    }
+}
+
+impl MergeWithError<MyParseError> for DeserrJsonError<BadRequest> {
+    fn merge(
+        _self_: Option<Self>,
+        other: MyParseError,
+        merge_location: ValuePointerRef,
+    ) -> ControlFlow<Self, Self> {
+        Self::error::<Infallible>(
+            None,
+            ErrorKind::Unexpected { msg: other.to_string() },
+            merge_location,
+        )
+    }
+}
+
 #[derive(Debug, Deserr)]
 #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)]
 pub struct GetLogs {
-    #[deserr(default, error = DeserrJsonError<BadRequest>)]
-    pub target: String,
+    #[deserr(default = "info".parse().unwrap(), try_from(&String) = MyTargets::from_str -> DeserrJsonError<BadRequest>)]
+    target: MyTargets,
 
     #[deserr(default, error = DeserrJsonError<BadRequest>)]
-    pub mode: LogMode,
-}
-
-impl fmt::Display for LogLevel {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            LogLevel::Error => f.write_str("error"),
-            LogLevel::Warn => f.write_str("warn"),
-            LogLevel::Info => f.write_str("info"),
-            LogLevel::Debug => f.write_str("debug"),
-            LogLevel::Trace => f.write_str("trace"),
-        }
-    }
+    mode: LogMode,
 }
 
 struct LogWriter {
     sender: mpsc::UnboundedSender<Vec<u8>>,
 }
 
-impl Drop for LogWriter {
-    fn drop(&mut self) {
-        println!("hello");
-    }
-}
-
 impl Write for LogWriter {
     fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
         self.sender.send(buf.to_vec()).map_err(std::io::Error::other)?;
@@ -99,7 +113,6 @@ struct HandleGuard {
 
 impl Drop for HandleGuard {
     fn drop(&mut self) {
-        println!("log streamer being dropped");
         if let Err(e) = self.logs.modify(|layer| *layer.inner_mut() = None) {
             tracing::error!("Could not free the logs route: {e}");
         }
@@ -203,7 +216,6 @@ pub async fn get_logs(
     _auth_controller: GuardedData<ActionPolicy<{ actions::METRICS_ALL }>, Data<AuthController>>,
     logs: Data<LogRouteHandle>,
     body: AwebJson<GetLogs, DeserrJsonError>,
-    _req: HttpRequest,
 ) -> Result<HttpResponse, ResponseError> {
     let opt = body.into_inner();
 
@@ -212,8 +224,7 @@ pub async fn get_logs(
     logs.modify(|layer| match layer.inner_mut() {
         None => {
             // there is no one getting logs
-            *layer.filter_mut() =
-                tracing_subscriber::filter::Targets::from_str(&opt.target).unwrap();
+            *layer.filter_mut() = opt.target.0.clone();
             let (new_layer, new_stream) = make_layer(&opt, logs.clone());
 
             *layer.inner_mut() = Some(new_layer);
@@ -235,7 +246,6 @@ pub async fn get_logs(
 pub async fn cancel_logs(
     _auth_controller: GuardedData<ActionPolicy<{ actions::METRICS_ALL }>, Data<AuthController>>,
     logs: Data<LogRouteHandle>,
-    _req: HttpRequest,
 ) -> Result<HttpResponse, ResponseError> {
     if let Err(e) = logs.modify(|layer| *layer.inner_mut() = None) {
         tracing::error!("Could not free the logs route: {e}");

meilisearch/tests/logs/error.rs

@@ -0,0 +1,98 @@
+use meili_snap::*;
+
+use crate::common::Server;
+use crate::json;
+
+#[actix_rt::test]
+async fn logs_bad_target() {
+    let server = Server::new().await;
+
+    // Wrong type
+    let (response, code) = server.service.post("/logs", json!({ "target": true })).await;
+    snapshot!(code, @"400 Bad Request");
+    snapshot!(response, @r###"
+    {
+      "message": "Invalid value type at `.target`: expected a string, but found a boolean: `true`",
+      "code": "bad_request",
+      "type": "invalid_request",
+      "link": "https://docs.meilisearch.com/errors#bad_request"
+    }
+    "###);
+
+    // Wrong type
+    let (response, code) = server.service.post("/logs", json!({ "target": [] })).await;
+    snapshot!(code, @"400 Bad Request");
+    snapshot!(response, @r###"
+    {
+      "message": "Invalid value type at `.target`: expected a string, but found an array: `[]`",
+      "code": "bad_request",
+      "type": "invalid_request",
+      "link": "https://docs.meilisearch.com/errors#bad_request"
+    }
+    "###);
+
+    // Our help message
+    let (response, code) = server.service.post("/logs", json!({ "target": "" })).await;
+    snapshot!(code, @"400 Bad Request");
+    snapshot!(response, @r###"
+    {
+      "message": "Invalid value at `.target`: Empty string is not a valid target. If you want to get no logs use `OFF`. Usage: `info`, `info:meilisearch`, or you can write multiple filters in one target: `index_scheduler=info,milli=trace`",
+      "code": "bad_request",
+      "type": "invalid_request",
+      "link": "https://docs.meilisearch.com/errors#bad_request"
+    }
+    "###);
+
+    // An error from the target parser
+    let (response, code) = server.service.post("/logs", json!({ "target": "==" })).await;
+    snapshot!(code, @"400 Bad Request");
+    snapshot!(response, @r###"
+    {
+      "message": "Invalid value at `.target`: invalid filter directive: too many '=' in filter directive, expected 0 or 1",
+      "code": "bad_request",
+      "type": "invalid_request",
+      "link": "https://docs.meilisearch.com/errors#bad_request"
+    }
+    "###);
+}
+
+#[actix_rt::test]
+async fn logs_bad_mode() {
+    let server = Server::new().await;
+
+    // Wrong type
+    let (response, code) = server.service.post("/logs", json!({ "mode": true })).await;
+    snapshot!(code, @"400 Bad Request");
+    snapshot!(response, @r###"
+    {
+      "message": "Invalid value type at `.mode`: expected a string, but found a boolean: `true`",
+      "code": "bad_request",
+      "type": "invalid_request",
+      "link": "https://docs.meilisearch.com/errors#bad_request"
+    }
+    "###);
+
+    // Wrong type
+    let (response, code) = server.service.post("/logs", json!({ "mode": [] })).await;
+    snapshot!(code, @"400 Bad Request");
+    snapshot!(response, @r###"
+    {
+      "message": "Invalid value type at `.mode`: expected a string, but found an array: `[]`",
+      "code": "bad_request",
+      "type": "invalid_request",
+      "link": "https://docs.meilisearch.com/errors#bad_request"
+    }
+    "###);
+
+    // Wrong value
+    let (response, code) = server.service.post("/logs", json!({ "mode": "tamo" })).await;
+    snapshot!(code, @"400 Bad Request");
+    snapshot!(response, @r###"
+    {
+      "message": "Unknown value `tamo` at `.mode`: expected one of `fmt`, `profile`",
+      "code": "bad_request",
+      "type": "invalid_request",
+      "link": "https://docs.meilisearch.com/errors#bad_request"
+    }
+    "###);
+}

meilisearch/tests/logs/mod.rs

@@ -0,0 +1,89 @@
+mod error;
+
+use std::rc::Rc;
+use std::str::FromStr;
+
+use actix_web::http::header::ContentType;
+use meili_snap::snapshot;
+use meilisearch::{analytics, create_app, Opt};
+use tracing::level_filters::LevelFilter;
+use tracing_subscriber::layer::SubscriberExt;
+use tracing_subscriber::Layer;
+
+use crate::common::{default_settings, Server};
+use crate::json;
+
+#[actix_web::test]
+async fn basic_test_log_route() {
+    let db_path = tempfile::tempdir().unwrap();
+    let server =
+        Server::new_with_options(Opt { ..default_settings(db_path.path()) }).await.unwrap();
+
+    let (route_layer, route_layer_handle) =
+        tracing_subscriber::reload::Layer::new(None.with_filter(
+            tracing_subscriber::filter::Targets::new().with_target("", LevelFilter::OFF),
+        ));
+
+    let subscriber = tracing_subscriber::registry().with(route_layer).with(
+        tracing_subscriber::fmt::layer()
+            .with_line_number(true)
+            .with_span_events(tracing_subscriber::fmt::format::FmtSpan::ACTIVE)
+            .with_filter(tracing_subscriber::filter::LevelFilter::from_str("INFO").unwrap()),
+    );
+
+    let app = actix_web::test::init_service(create_app(
+        server.service.index_scheduler.clone().into(),
+        server.service.auth.clone().into(),
+        server.service.options.clone(),
+        route_layer_handle,
+        analytics::MockAnalytics::new(&server.service.options),
+        true,
+    ))
+    .await;
+
+    // set the subscriber as the default for the application
+    tracing::subscriber::set_global_default(subscriber).unwrap();
+
+    let app = Rc::new(app);
+
+    // First, we start listening on the `/logs` route
+    let handle_app = app.clone();
+    let handle = tokio::task::spawn_local(async move {
+        let req = actix_web::test::TestRequest::post()
+            .uri("/logs")
+            .insert_header(ContentType::json())
+            .set_payload(
+                serde_json::to_vec(&json!({
+                    "mode": "fmt",
+                    "target": "info",
+                }))
+                .unwrap(),
+            );
+        let req = req.to_request();
+        let ret = actix_web::test::call_service(&*handle_app, req).await;
+        actix_web::test::read_body(ret).await
+    });
+
+    // We're going to create an index to get at least one info log saying we processed a batch of task
+    let (ret, _code) = server.create_index(json!({ "uid": "tamo" })).await;
+    snapshot!(ret, @r###"
+    {
+      "taskUid": 0,
+      "indexUid": "tamo",
+      "status": "enqueued",
+      "type": "indexCreation",
+      "enqueuedAt": "[date]"
+    }
+    "###);
+    server.wait_task(ret.uid()).await;
+
+    let req = actix_web::test::TestRequest::delete().uri("/logs");
+    let req = req.to_request();
+    let ret = actix_web::test::call_service(&*app, req).await;
+    let code = ret.status();
+    snapshot!(code, @"204 No Content");
+
+    let logs = handle.await.unwrap();
+    let logs = String::from_utf8(logs.to_vec()).unwrap();
+    assert!(logs.contains("INFO"), "{logs}");
+}