diff --git a/src/servers/src/error.rs b/src/servers/src/error.rs index 80c93444158f..b9a859cc8c6f 100644 --- a/src/servers/src/error.rs +++ b/src/servers/src/error.rs @@ -25,7 +25,7 @@ use common_error::define_into_tonic_status; use common_error::ext::{BoxedError, ErrorExt}; use common_error::status_code::StatusCode; use common_macro::stack_trace_debug; -use common_telemetry::{debug, error}; +use common_telemetry::{error, warn}; use datatypes::prelude::ConcreteDataType; use headers::ContentType; use query::parser::PromQuery; @@ -737,8 +737,17 @@ impl From for Error { } } +fn log_error_if_necessary(error: &Error) { + if error.status_code().should_log_error() { + error!(error; "Failed to handle HTTP request "); + } else { + warn!(error; "Failed to handle HTTP request "); + } +} + impl IntoResponse for Error { fn into_response(self) -> Response { + use pipeline::error::Error as PipelineError; let error_msg = self.output_msg(); let status = match self { Error::InfluxdbLineProtocol { .. } @@ -752,16 +761,17 @@ impl IntoResponse for Error { | Error::InvalidPromRemoteRequest { .. } | Error::InvalidQuery { .. } | Error::TimePrecision { .. } => HttpStatusCode::BAD_REQUEST, - _ => { - if self.status_code().should_log_error() { - error!(self; "Failed to handle HTTP request: "); - } else { - debug!("Failed to handle HTTP request: {self:?}"); - } - - HttpStatusCode::INTERNAL_SERVER_ERROR - } + Error::Pipeline { ref source, .. } => match source { + PipelineError::CompilePipeline { .. } + | PipelineError::InvalidPipelineVersion { .. } + | PipelineError::PipelineNotFound { .. } => HttpStatusCode::BAD_REQUEST, + _ => HttpStatusCode::INTERNAL_SERVER_ERROR, + }, + _ => HttpStatusCode::INTERNAL_SERVER_ERROR, }; + + log_error_if_necessary(&self); + let body = Json(json!({ "error": error_msg, })); diff --git a/src/servers/src/http.rs b/src/servers/src/http.rs index 6204908c03a7..d4180f5478e7 100644 --- a/src/servers/src/http.rs +++ b/src/servers/src/http.rs @@ -697,7 +697,9 @@ impl HttpServer { .layer( ServiceBuilder::new() .layer(HandleErrorLayer::new(handle_error)) - .layer(TraceLayer::new_for_http()) + // disable on failure tracing. because printing out isn't very helpful, + // and we have impl IntoResponse for Error. It will print out more detailed error messages + .layer(TraceLayer::new_for_http().on_failure(())) .option_layer(timeout_layer) .option_layer(body_limit_layer) // auth layer