diff --git a/meilisearch/src/error.rs b/meilisearch/src/error.rs index ee54cf831..6c5f76a72 100644 --- a/meilisearch/src/error.rs +++ b/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, diff --git a/meilisearch/src/routes/logs.rs b/meilisearch/src/routes/logs.rs index b327acab1..96228f9aa 100644 --- a/meilisearch/src/routes/logs.rs +++ b/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,36 +40,59 @@ pub enum LogMode { Profile, } -#[derive(Debug, Deserr)] -#[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] -pub struct GetLogs { - #[deserr(default, error = DeserrJsonError)] - pub target: String, +/// Simple wrapper around the `Targets` from `tracing_subscriber` to implement `MergeWithError` on it. +#[derive(Clone, Debug)] +struct MyTargets(Targets); - #[deserr(default, error = DeserrJsonError)] - pub mode: LogMode, +/// 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 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"), +impl FromStr for MyTargets { + type Err = MyParseError; + + fn from_str(s: &str) -> Result { + if s.is_empty() { + Err(MyParseError::Example) + } else { + Ok(MyTargets(Targets::from_str(s).map_err(MyParseError::ParseError)?)) } } } -struct LogWriter { - sender: mpsc::UnboundedSender>, +impl MergeWithError for DeserrJsonError { + fn merge( + _self_: Option, + other: MyParseError, + merge_location: ValuePointerRef, + ) -> ControlFlow { + Self::error::( + None, + ErrorKind::Unexpected { msg: other.to_string() }, + merge_location, + ) + } } -impl Drop for LogWriter { - fn drop(&mut self) { - println!("hello"); - } +#[derive(Debug, Deserr)] +#[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] +pub struct GetLogs { + #[deserr(default = "info".parse().unwrap(), try_from(&String) = MyTargets::from_str -> DeserrJsonError)] + target: MyTargets, + + #[deserr(default, error = DeserrJsonError)] + mode: LogMode, +} + +struct LogWriter { + sender: mpsc::UnboundedSender>, } impl Write for LogWriter { @@ -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, Data>, logs: Data, body: AwebJson, - _req: HttpRequest, ) -> Result { 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, Data>, logs: Data, - _req: HttpRequest, ) -> Result { if let Err(e) = logs.modify(|layer| *layer.inner_mut() = None) { tracing::error!("Could not free the logs route: {e}"); diff --git a/meilisearch/tests/logs/error.rs b/meilisearch/tests/logs/error.rs new file mode 100644 index 000000000..965b68d17 --- /dev/null +++ b/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" + } + "###); +} diff --git a/meilisearch/tests/logs/mod.rs b/meilisearch/tests/logs/mod.rs new file mode 100644 index 000000000..4aba0a4cd --- /dev/null +++ b/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}"); +}