From a9a1796316b22c3d349b49e855624168c95aef1c Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Mon, 30 Oct 2023 10:50:28 -0700 Subject: [PATCH] fix: defer Error::source methods to inner kind's (#486) and kill ReportableError's redundant inheritance of fmt::Display SYNC-3977 --- .../autoconnect-ws-sm/src/error.rs | 18 +++++++++--------- autoconnect/autoconnect-ws/src/error.rs | 10 ++++++++-- autopush-common/src/errors.rs | 2 +- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/error.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/error.rs index 364803036..77989a2f0 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/error.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/error.rs @@ -1,21 +1,15 @@ -use std::fmt; +use std::{error::Error, fmt}; use actix_ws::CloseCode; use backtrace::Backtrace; use autopush_common::{db::error::DbError, errors::ReportableError}; -pub type NonStdBacktrace = backtrace::Backtrace; - /// WebSocket state machine errors -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub struct SMError { pub kind: SMErrorKind, - /// Avoid thiserror's automatic `std::backtrace::Backtrace` integration by - /// not using the type name "Backtrace". The older `backtrace::Backtrace` - /// is still preferred for Sentry integration: - /// https://github.com/getsentry/sentry-rust/issues/600 - backtrace: NonStdBacktrace, + backtrace: Backtrace, } impl fmt::Display for SMError { @@ -24,6 +18,12 @@ impl fmt::Display for SMError { } } +impl Error for SMError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + self.kind.source() + } +} + // Forward From impls to SMError from SMErrorKind. Because From is reflexive, // this impl also takes care of From. impl From for SMError diff --git a/autoconnect/autoconnect-ws/src/error.rs b/autoconnect/autoconnect-ws/src/error.rs index 5d656c444..9575f692d 100644 --- a/autoconnect/autoconnect-ws/src/error.rs +++ b/autoconnect/autoconnect-ws/src/error.rs @@ -1,4 +1,4 @@ -use std::fmt; +use std::{error::Error, fmt}; use actix_ws::CloseCode; use backtrace::Backtrace; @@ -7,7 +7,7 @@ use autoconnect_ws_sm::{SMError, WebPushClient}; use autopush_common::{errors::ReportableError, sentry::event_from_error}; /// WebPush WebSocket Handler Errors -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub struct WSError { pub kind: WSErrorKind, backtrace: Option, @@ -19,6 +19,12 @@ impl fmt::Display for WSError { } } +impl Error for WSError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + self.kind.source() + } +} + // Forward From impls to WSError from WSErrorKind. Because From is reflexive, // this impl also takes care of From. impl From for WSError diff --git a/autopush-common/src/errors.rs b/autopush-common/src/errors.rs index 6d96c5a2a..cdd8609fd 100644 --- a/autopush-common/src/errors.rs +++ b/autopush-common/src/errors.rs @@ -180,7 +180,7 @@ impl ApcErrorKind { } /// Interface for reporting our Error types to Sentry or as metrics -pub trait ReportableError: std::error::Error + fmt::Display { +pub trait ReportableError: std::error::Error { /// Return a `Backtrace` for this Error if one was captured fn backtrace(&self) -> Option<&Backtrace>;