From 14ef9cb2b86830e3efe148cc766d62caba16afb6 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Wed, 25 Mar 2026 22:31:02 -0400 Subject: [PATCH] break apart into easier to manage pieces --- .../middleware/error_alerting_middleware.rs | 233 ------------------ .../check_common_web_error.rs | 36 +++ .../check_status_code_fallback.rs | 30 +++ .../enqueue_alert.rs | 14 ++ .../error_alerting_middleware.rs | 141 +++++++++++ .../error_alerting_middleware/mod.rs | 5 + .../service/web/storyteller_web/src/main.rs | 2 +- 7 files changed, 227 insertions(+), 234 deletions(-) delete mode 100644 crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware.rs create mode 100644 crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/check_common_web_error.rs create mode 100644 crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/check_status_code_fallback.rs create mode 100644 crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/enqueue_alert.rs create mode 100644 crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/error_alerting_middleware.rs create mode 100644 crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/mod.rs diff --git a/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware.rs b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware.rs deleted file mode 100644 index 2e1ed8ef8f..0000000000 --- a/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware.rs +++ /dev/null @@ -1,233 +0,0 @@ -use std::future::{Future, Ready, ready}; -use std::pin::Pin; - -use actix_web::dev::{Service, ServiceRequest, ServiceResponse, Transform}; -use actix_web::Error; -use log::{debug, warn}; - -use pager::client::pager::Pager; -use pager::notification::notification_details::NotificationDetails; - -use crate::http_server::common_responses::common_web_error::CommonWebError; - -// ======================== Transform (factory) ======================== - -/// Middleware that intercepts error responses and enqueues pager alerts. -/// -/// Inspects errors in two ways: -/// 1. **Typed error matching** via downcast (e.g. `CommonWebError::ServerError`) -/// 2. **Status code fallback** for untyped 500s that slip through -/// -/// To add new error types, add a new branch to `check_common_web_error()` -/// or add a new `if let Some(...)` downcast block in the matcher functions. -/// To add new status-based rules, add to `check_status_fallback()`. -#[derive(Clone)] -pub struct ErrorAlertingMiddleware { - pager: Pager, -} - -impl ErrorAlertingMiddleware { - pub fn new(pager: Pager) -> Self { - Self { pager } - } -} - -impl Transform for ErrorAlertingMiddleware - where - S: Service, Error = Error> + 'static, - S::Future: 'static, - B: 'static, -{ - type Response = ServiceResponse; - type Error = Error; - type InitError = (); - type Transform = ErrorAlertingService; - type Future = Ready>; - - fn new_transform(&self, service: S) -> Self::Future { - ready(Ok(ErrorAlertingService { - service, - pager: self.pager.clone(), - })) - } -} - -// ======================== Service (per-request) ======================== - -pub struct ErrorAlertingService { - service: S, - pager: Pager, -} - -impl Service for ErrorAlertingService - where - S: Service, Error = Error> + 'static, - S::Future: 'static, - B: 'static, -{ - type Response = ServiceResponse; - type Error = Error; - type Future = Pin>>>; - - actix_service::forward_ready!(service); - - fn call(&self, req: ServiceRequest) -> Self::Future { - let method = req.method().to_string(); - let path = req.path().to_string(); - let pager = self.pager.clone(); - let fut = self.service.call(req); - - Box::pin(async move { - match fut.await { - Ok(res) => { - // The handler returned a response (possibly an error response via ResponseError). - // Check for typed errors stashed in the response, then fall back to status code. - check_ok_response_for_alerts(&pager, &method, &path, &res); - Ok(res) - } - Err(err) => { - // The handler (or inner middleware) returned an Err(actix_web::Error). - // Try typed downcast first, then fall back to status code. - check_err_for_alerts(&pager, &method, &path, &err); - Err(err) - } - } - }) - } -} - -// ======================== Ok(response) path ======================== - -/// Inspect a successful `ServiceResponse` that may contain an error response. -/// -/// Actix-web converts `ResponseError` types into HTTP responses and stashes -/// the original error in `response.error()`. We can downcast from there. -fn check_ok_response_for_alerts( - pager: &Pager, - method: &str, - path: &str, - response: &ServiceResponse, -) { - let status = response.status(); - - // Try to get the original typed error from the response (if it came from ResponseError). - if let Some(err) = response.response().error() { - // --- Typed matchers (add new error types here) --- - - if let Some(common_err) = err.as_error::() { - if check_common_web_error(pager, method, path, common_err) { - return; - } - } - - // Add more typed matchers here: - // if let Some(my_err) = err.as_error::() { - // if check_my_other_error(pager, method, path, my_err) { return; } - // } - } - - // --- Status code fallback --- - check_status_fallback(pager, method, path, status.as_u16()); -} - -// ======================== Err(error) path ======================== - -/// Inspect an `actix_web::Error` returned from the handler or inner middleware. -fn check_err_for_alerts( - pager: &Pager, - method: &str, - path: &str, - err: &Error, -) { - // --- Typed matchers (add new error types here) --- - - if let Some(common_err) = err.as_error::() { - if check_common_web_error(pager, method, path, common_err) { - return; - } - } - - // Add more typed matchers here: - // if let Some(my_err) = err.as_error::() { - // if check_my_other_error(pager, method, path, my_err) { return; } - // } - - // --- Status code fallback --- - let status = err.as_response_error().status_code(); - check_status_fallback(pager, method, path, status.as_u16()); -} - -// ======================== Typed error matchers ======================== - -/// Check `CommonWebError` and alert on server errors. -/// Returns `true` if the error was handled (alerted or intentionally skipped). -fn check_common_web_error( - pager: &Pager, - method: &str, - path: &str, - error: &CommonWebError, -) -> bool { - match error { - CommonWebError::ServerError => { - enqueue_alert( - pager, - format!("CommonWebError::ServerError on {} {}", method, path), - format!( - "A CommonWebError::ServerError was returned.\n\n\ - Endpoint: {} {}\n\ - Error: {:?}\n\ - Time: {}", - method, path, error, - chrono::Utc::now().format("%Y-%m-%d %H:%M:%S UTC"), - ), - ); - true - } - // Don't alert on client errors (400, 401, 404, 402). - CommonWebError::BadInputWithSimpleMessage(_) => true, - CommonWebError::NotFound => true, - CommonWebError::NotAuthorized => true, - CommonWebError::PaymentRequired => true, - } -} - -// ======================== Status code fallback ======================== - -/// Fallback alerting based on HTTP status code when no typed error matched. -fn check_status_fallback( - pager: &Pager, - method: &str, - path: &str, - status_code: u16, -) { - match status_code { - 500 => { - enqueue_alert( - pager, - format!("HTTP 500: {} {}", method, path), - format!( - "An untyped HTTP 500 response was returned (no typed error matched).\n\n\ - Endpoint: {} {}\n\ - Time: {}", - method, path, - chrono::Utc::now().format("%Y-%m-%d %H:%M:%S UTC"), - ), - ); - } - // Add more status-based rules here: - // 503 => { enqueue_alert(...); } - _ => {} - } -} - -// ======================== Helpers ======================== - -fn enqueue_alert(pager: &Pager, summary: String, description: String) { - let notification = NotificationDetails::with_summary_and_description(summary, description); - - if let Err(err) = pager.enqueue_page(notification) { - warn!("Error alerting middleware: failed to enqueue page: {:?}", err); - } else { - debug!("Error alerting middleware: enqueued alert"); - } -} diff --git a/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/check_common_web_error.rs b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/check_common_web_error.rs new file mode 100644 index 0000000000..5a3e9d4a3d --- /dev/null +++ b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/check_common_web_error.rs @@ -0,0 +1,36 @@ +use pager::client::pager::Pager; + +use crate::http_server::common_responses::common_web_error::CommonWebError; +use super::enqueue_alert::enqueue_alert; + +/// Check `CommonWebError` and alert on server errors. +/// Returns `true` if the error was handled (alerted or intentionally skipped). +pub(super) fn check_common_web_error( + pager: &Pager, + method: &str, + path: &str, + error: &CommonWebError, +) -> bool { + match error { + CommonWebError::ServerError => { + enqueue_alert( + pager, + format!("CommonWebError::ServerError on {} {}", method, path), + format!( + "A CommonWebError::ServerError was returned.\n\n\ + Endpoint: {} {}\n\ + Error: {:?}\n\ + Time: {}", + method, path, error, + chrono::Utc::now().format("%Y-%m-%d %H:%M:%S UTC"), + ), + ); + true + } + // Don't alert on client errors (400, 401, 404, 402). + CommonWebError::BadInputWithSimpleMessage(_) => true, + CommonWebError::NotFound => true, + CommonWebError::NotAuthorized => true, + CommonWebError::PaymentRequired => true, + } +} diff --git a/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/check_status_code_fallback.rs b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/check_status_code_fallback.rs new file mode 100644 index 0000000000..e84014e469 --- /dev/null +++ b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/check_status_code_fallback.rs @@ -0,0 +1,30 @@ +use pager::client::pager::Pager; + +use super::enqueue_alert::enqueue_alert; + +/// Fallback alerting based on HTTP status code when no typed error matched. +pub(super) fn check_status_code_fallback( + pager: &Pager, + method: &str, + path: &str, + status_code: u16, +) { + match status_code { + 500 => { + enqueue_alert( + pager, + format!("HTTP 500: {} {}", method, path), + format!( + "An untyped HTTP 500 response was returned (no typed error matched).\n\n\ + Endpoint: {} {}\n\ + Time: {}", + method, path, + chrono::Utc::now().format("%Y-%m-%d %H:%M:%S UTC"), + ), + ); + } + // Add more status-based rules here: + // 503 => { enqueue_alert(...); } + _ => {} + } +} diff --git a/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/enqueue_alert.rs b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/enqueue_alert.rs new file mode 100644 index 0000000000..4bfb9766a3 --- /dev/null +++ b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/enqueue_alert.rs @@ -0,0 +1,14 @@ +use log::{debug, warn}; + +use pager::client::pager::Pager; +use pager::notification::notification_details::NotificationDetails; + +pub(super) fn enqueue_alert(pager: &Pager, summary: String, description: String) { + let notification = NotificationDetails::with_summary_and_description(summary, description); + + if let Err(err) = pager.enqueue_page(notification) { + warn!("Error alerting middleware: failed to enqueue page: {:?}", err); + } else { + debug!("Error alerting middleware: enqueued alert"); + } +} diff --git a/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/error_alerting_middleware.rs b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/error_alerting_middleware.rs new file mode 100644 index 0000000000..559f479818 --- /dev/null +++ b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/error_alerting_middleware.rs @@ -0,0 +1,141 @@ +use std::future::{Future, Ready, ready}; +use std::pin::Pin; + +use actix_web::dev::{Service, ServiceRequest, ServiceResponse, Transform}; +use actix_web::Error; + +use pager::client::pager::Pager; + +use crate::http_server::common_responses::common_web_error::CommonWebError; +use super::check_common_web_error::check_common_web_error; +use super::check_status_code_fallback::check_status_code_fallback; + +// ======================== Transform (factory) ======================== + +/// Middleware that intercepts error responses and enqueues pager alerts. +/// +/// Inspects errors in two ways: +/// 1. **Typed error matching** via downcast (e.g. `CommonWebError::ServerError`) +/// 2. **Status code fallback** for untyped 500s that slip through +/// +/// To add new error types, create a new `check_*.rs` matcher module and +/// add a downcast branch in `check_ok_response_for_alerts` / `check_err_for_alerts`. +#[derive(Clone)] +pub struct ErrorAlertingMiddleware { + pager: Pager, +} + +impl ErrorAlertingMiddleware { + pub fn new(pager: Pager) -> Self { + Self { pager } + } +} + +impl Transform for ErrorAlertingMiddleware + where + S: Service, Error = Error> + 'static, + S::Future: 'static, + B: 'static, +{ + type Response = ServiceResponse; + type Error = Error; + type InitError = (); + type Transform = ErrorAlertingService; + type Future = Ready>; + + fn new_transform(&self, service: S) -> Self::Future { + ready(Ok(ErrorAlertingService { + service, + pager: self.pager.clone(), + })) + } +} + +// ======================== Service (per-request) ======================== + +pub struct ErrorAlertingService { + service: S, + pager: Pager, +} + +impl Service for ErrorAlertingService + where + S: Service, Error = Error> + 'static, + S::Future: 'static, + B: 'static, +{ + type Response = ServiceResponse; + type Error = Error; + type Future = Pin>>>; + + actix_service::forward_ready!(service); + + fn call(&self, req: ServiceRequest) -> Self::Future { + let method = req.method().to_string(); + let path = req.path().to_string(); + let pager = self.pager.clone(); + let fut = self.service.call(req); + + Box::pin(async move { + match fut.await { + Ok(res) => { + check_ok_response_for_alerts(&pager, &method, &path, &res); + Ok(res) + } + Err(err) => { + check_err_for_alerts(&pager, &method, &path, &err); + Err(err) + } + } + }) + } +} + +// ======================== Response inspection ======================== + +/// Inspect a successful `ServiceResponse` that may contain an error response. +/// +/// Actix-web converts `ResponseError` types into HTTP responses and stashes +/// the original error in `response.error()`. We can downcast from there. +fn check_ok_response_for_alerts( + pager: &Pager, + method: &str, + path: &str, + response: &ServiceResponse, +) { + let status = response.status(); + + // Try to get the original typed error from the response (if it came from ResponseError). + if let Some(err) = response.response().error() { + // --- Typed matchers (add new check_*.rs modules and downcast branches here) --- + + if let Some(common_err) = err.as_error::() { + if check_common_web_error(pager, method, path, common_err) { + return; + } + } + } + + // --- Status code fallback --- + check_status_code_fallback(pager, method, path, status.as_u16()); +} + +/// Inspect an `actix_web::Error` returned from the handler or inner middleware. +fn check_err_for_alerts( + pager: &Pager, + method: &str, + path: &str, + err: &Error, +) { + // --- Typed matchers (add new check_*.rs modules and downcast branches here) --- + + if let Some(common_err) = err.as_error::() { + if check_common_web_error(pager, method, path, common_err) { + return; + } + } + + // --- Status code fallback --- + let status = err.as_response_error().status_code(); + check_status_code_fallback(pager, method, path, status.as_u16()); +} diff --git a/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/mod.rs b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/mod.rs new file mode 100644 index 0000000000..36d4c5b69f --- /dev/null +++ b/crates/service/web/storyteller_web/src/http_server/middleware/error_alerting_middleware/mod.rs @@ -0,0 +1,5 @@ +mod check_common_web_error; +mod check_status_code_fallback; +mod enqueue_alert; + +pub mod error_alerting_middleware; diff --git a/crates/service/web/storyteller_web/src/main.rs b/crates/service/web/storyteller_web/src/main.rs index af75b76d1d..3ead8f9828 100644 --- a/crates/service/web/storyteller_web/src/main.rs +++ b/crates/service/web/storyteller_web/src/main.rs @@ -72,7 +72,7 @@ use crate::configs::app_startup::redis_rate_limiters::configure_redis_rate_limit use crate::configs::connect_to_database::connect_to_database; use crate::configs::static_api_tokens::StaticApiTokenSet; use crate::http_server::cookies::anonymous_visitor_tracking::avt_cookie_manager::AvtCookieManager; -use crate::http_server::middleware::error_alerting_middleware::ErrorAlertingMiddleware; +use crate::http_server::middleware::error_alerting_middleware::error_alerting_middleware::ErrorAlertingMiddleware; use crate::http_server::middleware::pushback_filter_middleware::PushbackFilter; use crate::http_server::routes::add_routes::add_routes; use crate::http_server::session::session_checker::SessionChecker;