Improve snafu error messages

Author: sbernauer

trino-lb/src/error_formatting.rs

@@ -0,0 +1,44 @@
+pub fn snafu_error_to_string<E: std::error::Error>(err: &E) -> String {
+    let mut result = format!("{err}");
+    let mut source = err.source();
+    while let Some(err) = source {
+        result.push_str(format!(": {err}").as_str());
+        source = err.source();
+    }
+
+    result
+}
+
+#[test]
+fn test_error_formatting() {
+    let err = trino_lb_persistence::Error::RedisError {
+        source: trino_lb_persistence::redis::Error::CreateClient {
+            source: redis::RedisError::from(std::io::Error::new(
+                std::io::ErrorKind::ConnectionRefused,
+                "Connection to Redis was refused",
+            )),
+        },
+    };
+
+    assert_eq!(format!("{err}"), "Redis persistence error");
+    assert_eq!(format!("{err:#}"), "Redis persistence error");
+    assert_eq!(
+        format!("{err:?}"),
+        "RedisError { source: CreateClient { source: Connection to Redis was refused } }"
+    );
+    assert_eq!(
+        snafu_error_to_string(&err),
+        "Redis persistence error: Failed to create redis client: Connection to Redis was refused: Connection to Redis was refused"
+    );
+    assert_eq!(
+        format!("{:#}", snafu::Report::from_error(err)),
+        "Redis persistence error
+
+Caused by these errors (recent errors listed first):
+  1: Failed to create redis client
+  2: Connection to Redis was refused
+
+NOTE: Some redundant information has been removed. Set SNAFU_RAW_ERROR_MESSAGES=1 to disable this behavior.
+"
+    );
+}

trino-lb/src/http_server/admin/mod.rs

@@ -21,6 +21,7 @@ use trino_lb_persistence::Persistence;
 
 use crate::{
     cluster_group_manager::{self, ClusterStats},
+    error_formatting::snafu_error_to_string,
     http_server::AppState,
 };
 
@@ -71,7 +72,7 @@ impl IntoResponse for Error {
             Error::GetQueryCounterForGroup { .. } => StatusCode::INTERNAL_SERVER_ERROR,
             Error::GetAllClusterStates { .. } => StatusCode::INTERNAL_SERVER_ERROR,
         };
-        (status_code, format!("{self}")).into_response()
+        (status_code, snafu_error_to_string(&self)).into_response()
     }
 }
 

trino-lb/src/http_server/metrics.rs

@@ -9,7 +9,7 @@ use prometheus::{Encoder, TextEncoder};
 use snafu::{ResultExt, Snafu};
 use tracing::{instrument, warn};
 
-use crate::http_server::AppState;
+use crate::{error_formatting::snafu_error_to_string, http_server::AppState};
 
 #[derive(Snafu, Debug)]
 pub enum Error {
@@ -23,7 +23,11 @@ pub enum Error {
 impl IntoResponse for Error {
     fn into_response(self) -> Response {
         warn!(error = ?self, "Error while processing metrics request");
-        (StatusCode::INTERNAL_SERVER_ERROR, format!("{self:?}")).into_response()
+        (
+            StatusCode::INTERNAL_SERVER_ERROR,
+            snafu_error_to_string(&self),
+        )
+            .into_response()
     }
 }
 

trino-lb/src/http_server/ui/index.rs

@@ -12,6 +12,7 @@ use tracing::{instrument, warn};
 
 use crate::{
     cluster_group_manager::{self, ClusterStats},
+    error_formatting::snafu_error_to_string,
     http_server::AppState,
 };
 
@@ -33,7 +34,7 @@ impl IntoResponse for Error {
             Error::GetAllClusterStates { .. } => StatusCode::INTERNAL_SERVER_ERROR,
             Error::RenderTemplate { .. } => StatusCode::INTERNAL_SERVER_ERROR,
         };
-        (status_code, format!("{self}")).into_response()
+        (status_code, snafu_error_to_string(&self)).into_response()
     }
 }
 

trino-lb/src/http_server/ui/query.rs

@@ -11,7 +11,7 @@ use tracing::{instrument, warn};
 use trino_lb_core::TrinoLbQueryId;
 use trino_lb_persistence::Persistence;
 
-use crate::http_server::AppState;
+use crate::{error_formatting::snafu_error_to_string, http_server::AppState};
 
 #[derive(Snafu, Debug)]
 pub enum Error {
@@ -36,7 +36,7 @@ impl IntoResponse for Error {
             Error::QueryIdMissing => StatusCode::BAD_REQUEST,
             Error::QueryIdNotFound { .. } => StatusCode::NOT_FOUND,
         };
-        (status_code, format!("{self}")).into_response()
+        (status_code, snafu_error_to_string(&self)).into_response()
     }
 }
 

trino-lb/src/http_server/v1/statement.rs

@@ -28,6 +28,7 @@ use url::Url;
 
 use crate::{
     cluster_group_manager::{self, SendToTrinoResponse},
+    error_formatting::snafu_error_to_string,
     http_server::AppState,
     maintenance::leftover_queries::UPDATE_QUEUED_QUERY_LAST_ACCESSED_INTERVAL,
 };
@@ -131,7 +132,11 @@ pub enum Error {
 impl IntoResponse for Error {
     fn into_response(self) -> Response {
         warn!(error = ?self, "Error while processing request");
-        (StatusCode::INTERNAL_SERVER_ERROR, format!("{self:?}")).into_response()
+        (
+            StatusCode::INTERNAL_SERVER_ERROR,
+            snafu_error_to_string(&self),
+        )
+            .into_response()
     }
 }
 

trino-lb/src/main.rs

@@ -23,6 +23,7 @@ use crate::{args::Args, http_server::start_http_server};
 
 mod args;
 mod cluster_group_manager;
+mod error_formatting;
 mod http_server;
 mod maintenance;
 mod metrics;