mirror of
https://github.com/storytold/artcraft.git
synced 2026-11-05 01:38:44 +00:00
refactor web errors with cause + notification details
This commit is contained in:
1 parent
c7943bce59
commit
bf0703f0c1
10 files changed
+61
-41
No files matched your search
@@ -1,7 +1,6 @@
|
||||
use chrono::{DateTime, Utc};
|
||||
|
||||
use crate::notification::generate_deduplication_key::generate_deduplication_key;
|
||||
use crate::notification::notification_details_builder::NotificationDetailsBuilder;
|
||||
use crate::notification::notification_urgency::NotificationUrgency;
|
||||
|
||||
/// Details for a pager notification.
|
||||
@@ -20,7 +19,7 @@ pub struct NotificationDetails {
|
||||
pub(crate) event_time: DateTime<Utc>,
|
||||
|
||||
/// The error that triggered this notification, if any.
|
||||
pub(crate) maybe_error: Option<Box<dyn std::error::Error + Send + Sync + 'static>>,
|
||||
pub(crate) maybe_error: Option<std::sync::Arc<dyn std::error::Error + Send + Sync + 'static>>,
|
||||
|
||||
/// Whether this notification originated from an error.
|
||||
pub(crate) is_from_error: bool,
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use chrono::{DateTime, Utc};
|
||||
use std::fmt::{Debug, Display};
|
||||
use std::sync::Arc;
|
||||
|
||||
use chrono::{DateTime, Utc};
|
||||
|
||||
use crate::notification::notification_details::NotificationDetails;
|
||||
use crate::notification::notification_urgency::NotificationUrgency;
|
||||
@@ -10,7 +12,7 @@ pub struct NotificationDetailsBuilder {
|
||||
pub(crate) urgency: Option<NotificationUrgency>,
|
||||
pub(crate) event_time: DateTime<Utc>,
|
||||
|
||||
pub(crate) maybe_error: Option<Box<dyn std::error::Error + Send + Sync + 'static>>,
|
||||
pub(crate) maybe_error: Option<Arc<dyn std::error::Error + Send + Sync + 'static>>,
|
||||
pub(crate) is_from_error: bool,
|
||||
|
||||
pub(crate) http_method: Option<String>,
|
||||
@@ -47,7 +49,13 @@ impl NotificationDetailsBuilder {
|
||||
///
|
||||
/// Sets the title to a placeholder and attaches the error object.
|
||||
/// Callers should chain `.set_title()` to provide a meaningful title.
|
||||
pub fn from_error(error: Box<dyn std::error::Error + Send + Sync + 'static>) -> Self {
|
||||
/// Create a builder from a boxed error. Converts to `Arc` internally for shareability.
|
||||
pub fn from_boxed_error(error: Box<dyn std::error::Error + Send + Sync + 'static>) -> Self {
|
||||
Self::from_error(Arc::from(error))
|
||||
}
|
||||
|
||||
/// Create a builder from an `Arc`'d error.
|
||||
pub fn from_error(error: Arc<dyn std::error::Error + Send + Sync + 'static>) -> Self {
|
||||
Self {
|
||||
title: "Notification from Error".to_string(),
|
||||
description: None,
|
||||
@@ -80,7 +88,7 @@ impl NotificationDetailsBuilder {
|
||||
self
|
||||
}
|
||||
|
||||
pub fn set_error(mut self, error: Option<Box<dyn std::error::Error + Send + Sync + 'static>>) -> Self {
|
||||
pub fn set_error(mut self, error: Option<Arc<dyn std::error::Error + Send + Sync + 'static>>) -> Self {
|
||||
self.maybe_error = error;
|
||||
self
|
||||
}
|
||||
|
||||
@@ -13,7 +13,7 @@ pub fn alert_pager_and_return_err<T>(
|
||||
) -> anyhow::Result<T> {
|
||||
let err_message = format!("{:#}", err);
|
||||
|
||||
let mut builder = NotificationDetailsBuilder::from_error(err.into())
|
||||
let mut builder = NotificationDetailsBuilder::from_boxed_error(err.into())
|
||||
.set_title(title.to_string())
|
||||
.set_urgency(Some(NotificationUrgency::Medium));
|
||||
|
||||
|
||||
@@ -85,7 +85,7 @@ pub async fn process_failed_job(
|
||||
err,
|
||||
);
|
||||
|
||||
let notification = NotificationDetailsBuilder::from_error(err.into())
|
||||
let notification = NotificationDetailsBuilder::from_boxed_error(err.into())
|
||||
.set_title("Seedance2Pro refund failed".to_string())
|
||||
.set_inference_job_token(Some(job.job_token.to_string()))
|
||||
.set_third_party_id(Some(job.order_id.to_string()))
|
||||
@@ -140,7 +140,7 @@ pub async fn process_failed_job(
|
||||
err
|
||||
);
|
||||
|
||||
let notification = NotificationDetailsBuilder::from_error(err.into())
|
||||
let notification = NotificationDetailsBuilder::from_boxed_error(err.into())
|
||||
.set_title("Seedance2Pro mark job failed error".to_string())
|
||||
.set_urgency(Some(NotificationUrgency::Medium))
|
||||
.build();
|
||||
|
||||
@@ -11,7 +11,7 @@ pub fn alert_pager_and_return_err<T>(
|
||||
) -> anyhow::Result<T> {
|
||||
let err_message = format!("{:#}", err);
|
||||
|
||||
let notification = NotificationDetailsBuilder::from_error(err.into())
|
||||
let notification = NotificationDetailsBuilder::from_boxed_error(err.into())
|
||||
.set_title(title.to_string())
|
||||
.set_urgency(Some(NotificationUrgency::Medium))
|
||||
.build();
|
||||
|
||||
+29
-10
@@ -1,6 +1,8 @@
|
||||
use std::fmt::{Display, Formatter};
|
||||
use std::sync::Arc;
|
||||
|
||||
use actix_http::StatusCode;
|
||||
use actix_web::{HttpResponse, HttpResponseBuilder, ResponseError};
|
||||
use std::fmt::{Display, Formatter};
|
||||
|
||||
use crate::http_server::web_utils::user_session::require_user_session::RequireUserSessionError;
|
||||
|
||||
@@ -39,12 +41,20 @@ pub enum AdvancedCommonWebError {
|
||||
/// Uncaught errors are always 500 Internal Server Error.
|
||||
/// The user will never see the error cause or message, but our
|
||||
/// middleware will handle alerting, logging, etc.
|
||||
UncaughtServerError(Box<dyn std::error::Error + Send + Sync + 'static>),
|
||||
///
|
||||
/// Stored in `Arc` so the error alerting middleware can clone it for paging
|
||||
/// without consuming the original.
|
||||
UncaughtServerError(Arc<dyn std::error::Error + Send + Sync + 'static>),
|
||||
}
|
||||
|
||||
// =============== Public accessors ===============
|
||||
|
||||
impl AdvancedCommonWebError {
|
||||
/// Wrap any error as an `UncaughtServerError`.
|
||||
pub fn from_error(error: impl std::error::Error + Send + Sync + 'static) -> Self {
|
||||
Self::UncaughtServerError(Arc::new(error))
|
||||
}
|
||||
|
||||
/// Extract the wrapped causal error (only present for `UncaughtServerError`).
|
||||
pub fn cause(&self) -> Option<&(dyn std::error::Error + Send + Sync + 'static)> {
|
||||
match self {
|
||||
@@ -53,6 +63,15 @@ impl AdvancedCommonWebError {
|
||||
}
|
||||
}
|
||||
|
||||
/// Clone the wrapped causal error as an `Arc` (only present for `UncaughtServerError`).
|
||||
/// Useful for passing the error to the pager system without consuming the original.
|
||||
pub fn clone_cause_arc(&self) -> Option<Arc<dyn std::error::Error + Send + Sync + 'static>> {
|
||||
match self {
|
||||
Self::UncaughtServerError(err) => Some(Arc::clone(err)),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether this is a server error (500).
|
||||
pub fn is_server_error(&self) -> bool {
|
||||
matches!(self, Self::UncaughtServerError(_))
|
||||
@@ -136,19 +155,21 @@ impl ResponseError for AdvancedCommonWebError {
|
||||
|
||||
impl From<sqlx::Error> for AdvancedCommonWebError {
|
||||
fn from(err: sqlx::Error) -> Self {
|
||||
Self::UncaughtServerError(Box::new(err))
|
||||
Self::from_error(err)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<anyhow::Error> for AdvancedCommonWebError {
|
||||
fn from(err: anyhow::Error) -> Self {
|
||||
Self::UncaughtServerError(err.into())
|
||||
// anyhow::Error doesn't impl std::error::Error, so we go through Box -> Arc.
|
||||
let boxed: Box<dyn std::error::Error + Send + Sync> = err.into();
|
||||
Self::UncaughtServerError(Arc::from(boxed))
|
||||
}
|
||||
}
|
||||
|
||||
impl From<serde_json::Error> for AdvancedCommonWebError {
|
||||
fn from(err: serde_json::Error) -> Self {
|
||||
Self::UncaughtServerError(Box::new(err))
|
||||
Self::from_error(err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -156,9 +177,7 @@ impl From<RequireUserSessionError> for AdvancedCommonWebError {
|
||||
fn from(value: RequireUserSessionError) -> Self {
|
||||
match value {
|
||||
RequireUserSessionError::NotAuthorized => Self::NotAuthorized,
|
||||
RequireUserSessionError::ServerError => {
|
||||
Self::UncaughtServerError(Box::new(value))
|
||||
}
|
||||
RequireUserSessionError::ServerError => Self::from_error(value),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -223,7 +242,7 @@ mod tests {
|
||||
#[test]
|
||||
fn uncaught_io_error_returns_500_and_hides_cause() {
|
||||
let io_err = std::io::Error::new(std::io::ErrorKind::Other, "disk exploded");
|
||||
let error = AdvancedCommonWebError::UncaughtServerError(Box::new(io_err));
|
||||
let error = AdvancedCommonWebError::from_error(io_err);
|
||||
|
||||
assert_eq!(error.status_code(), StatusCode::INTERNAL_SERVER_ERROR);
|
||||
assert!(error.is_server_error());
|
||||
@@ -297,7 +316,7 @@ mod tests {
|
||||
#[test]
|
||||
fn error_source_returns_wrapped_error() {
|
||||
let io_err = std::io::Error::new(std::io::ErrorKind::Other, "root cause");
|
||||
let error = AdvancedCommonWebError::UncaughtServerError(Box::new(io_err));
|
||||
let error = AdvancedCommonWebError::from_error(io_err);
|
||||
|
||||
let source = std::error::Error::source(&error);
|
||||
assert!(source.is_some());
|
||||
|
||||
+2
-2
@@ -222,7 +222,7 @@ pub async fn seedance_2p0_multi_function_video_gen_handler(
|
||||
Err(err) => {
|
||||
warn!("Regular session fallback also failed for user {:?}: {:?}", user_token, err);
|
||||
if let Err(page_err) = server_state.pager.enqueue_page(
|
||||
NotificationDetailsBuilder::from_error(err.into())
|
||||
NotificationDetailsBuilder::from_boxed_error(err.into())
|
||||
.set_title("Seedance 2.0 generation failed (whitelist + fallback)".to_string())
|
||||
.set_urgency(Some(NotificationUrgency::High))
|
||||
.build()
|
||||
@@ -256,7 +256,7 @@ pub async fn seedance_2p0_multi_function_video_gen_handler(
|
||||
Err(err) => {
|
||||
warn!("Error calling seedance2pro generate_video: {:?}", err);
|
||||
if let Err(page_err) = server_state.pager.enqueue_page(
|
||||
NotificationDetailsBuilder::from_error(err.into())
|
||||
NotificationDetailsBuilder::from_boxed_error(err.into())
|
||||
.set_title("Seedance 2.0 generation failed".to_string())
|
||||
.set_urgency(Some(NotificationUrgency::High))
|
||||
.build()
|
||||
|
||||
+1
-1
@@ -129,7 +129,7 @@ pub async fn fal_webhook_handler(
|
||||
request_id, err, raw_body,
|
||||
);
|
||||
|
||||
let notification = NotificationDetailsBuilder::from_error(err.into())
|
||||
let notification = NotificationDetailsBuilder::from_boxed_error(err.into())
|
||||
.set_title("FAL webhook processing failed".to_string())
|
||||
.set_description(Some(description))
|
||||
.set_third_party_id(Some(request_id.to_string()))
|
||||
|
||||
+11
-17
@@ -21,29 +21,23 @@ pub(super) fn check_advanced_common_web_error(
|
||||
|
||||
let title = format!("UncaughtServerError on {} {}", method, path);
|
||||
|
||||
let description = if let Some(cause) = error.cause() {
|
||||
format!(
|
||||
"An UncaughtServerError was returned with a wrapped cause.\n\n\
|
||||
Endpoint: {} {}\n\
|
||||
Cause: {}\n\
|
||||
Cause (debug): {:?}",
|
||||
method, path, cause, cause,
|
||||
)
|
||||
// Clone the Arc'd causal error so the notification owns a reference to it.
|
||||
let cause_arc = error.clone_cause_arc();
|
||||
|
||||
let mut builder = if let Some(arc_err) = cause_arc {
|
||||
NotificationDetailsBuilder::from_error(arc_err)
|
||||
.set_title(title)
|
||||
} else {
|
||||
format!(
|
||||
"An UncaughtServerError was returned (no wrapped cause).\n\n\
|
||||
Endpoint: {} {}",
|
||||
method, path,
|
||||
)
|
||||
NotificationDetailsBuilder::from_title(title)
|
||||
};
|
||||
|
||||
let notification = NotificationDetailsBuilder::from_title(title)
|
||||
.set_description(Some(description))
|
||||
builder = builder
|
||||
.set_http_method(Some(method.to_string()))
|
||||
.set_http_path(Some(path.to_string()))
|
||||
.set_http_status_code(Some(500))
|
||||
.set_urgency(Some(NotificationUrgency::Medium))
|
||||
.build();
|
||||
.set_urgency(Some(NotificationUrgency::Medium));
|
||||
|
||||
let notification = builder.build();
|
||||
|
||||
if let Err(err) = pager.enqueue_page(notification) {
|
||||
warn!("Error alerting middleware: failed to enqueue page: {:?}", err);
|
||||
|
||||
+1
-1
@@ -35,7 +35,7 @@ pub async fn db_health_checker_thread(
|
||||
Ok(_) => {},
|
||||
}
|
||||
|
||||
let notification = NotificationDetailsBuilder::from_error(database_error.into())
|
||||
let notification = NotificationDetailsBuilder::from_boxed_error(database_error.into())
|
||||
.set_title("DB health check thread failed check".to_string())
|
||||
.set_urgency(Some(NotificationUrgency::High))
|
||||
.build();
|
||||
|
||||
Reference in new issue
Block a user