Skip to content

Commit

Permalink
feat: consolidate the sentry middlwares into autopush_common
Browse files Browse the repository at this point in the history
via the ReportableError trait

SYNC-3825
  • Loading branch information
pjenvey committed Jul 24, 2023
1 parent ccb85c5 commit e65486b
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 276 deletions.
9 changes: 4 additions & 5 deletions autoconnect/autoconnect-web/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,11 @@ macro_rules! build_app {
actix_http::StatusCode::NOT_FOUND,
autopush_common::errors::render_404,
))
/*
.wrap(crate::middleware::sentry::SentryWrapper::new(
$app_state.metrics.clone(),
"error".to_owned(),
.wrap(autopush_common::middleware::sentry::SentryWrapper::<
autopush_common::errors::ApcError,
>::new(
$app_state.metrics.clone(), "error".to_owned()
))
*/
.configure(config)
};
}
Expand Down
31 changes: 10 additions & 21 deletions autoconnect/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ use autopush_common::{
logging,
};

mod middleware;

pub type LocalError = autopush_common::errors::ApcError;

const USAGE: &str = "
Usage: autopush_rs [options]
Expand Down Expand Up @@ -79,21 +75,14 @@ async fn main() -> Result<()> {
"Starting autoconnect on port {} (router_port: {})",
port, router_port
);
HttpServer::new(move || {
let app = build_app!(app_state);
// TODO: should live in build_app!
app.wrap(crate::middleware::sentry::SentryWrapper::new(
app_state.metrics.clone(),
"error".to_owned(),
))
})
.bind(("0.0.0.0", port))?
.bind(("0.0.0.0", router_port))?
.run()
.await
.map_err(|e| e.into())
.map(|v| {
info!("Shutting down autoconnect");
v
})
HttpServer::new(move || build_app!(app_state))
.bind(("0.0.0.0", port))?
.bind(("0.0.0.0", router_port))?
.run()
.await
.map_err(|e| e.into())
.map(|v| {
info!("Shutting down autoconnect");
v
})
}
3 changes: 0 additions & 3 deletions autoconnect/src/middleware/mod.rs

This file was deleted.

201 changes: 0 additions & 201 deletions autoconnect/src/middleware/sentry.rs

This file was deleted.

16 changes: 15 additions & 1 deletion autoendpoint/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::fmt::{self, Display};
use thiserror::Error;
use validator::{ValidationErrors, ValidationErrorsKind};

use autopush_common::db::error::DbError;
use autopush_common::{db::error::DbError, errors::ReportableError};

/// Common `Result` type.
pub type ApiResult<T> = Result<T, ApiError>;
Expand Down Expand Up @@ -331,6 +331,20 @@ impl Serialize for ApiError {
}
}

impl ReportableError for ApiError {
fn backtrace(&self) -> Option<&Backtrace> {
Some(&self.backtrace)
}

fn is_sentry_event(&self) -> bool {
self.kind.is_sentry_event()
}

fn metric_label(&self) -> Option<&'static str> {
self.kind.metric_label()
}
}

/// Get the error number from validation errors. If multiple errors are present,
/// the first one with a valid error code is used.
fn errno_from_validation_errors(e: &ValidationErrors) -> Option<usize> {
Expand Down
3 changes: 0 additions & 3 deletions autoendpoint/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ mod error;
mod extractors;
mod headers;
mod metrics;
mod middleware;
mod routers;
mod routes;
mod server;
Expand All @@ -21,8 +20,6 @@ use std::error::Error;

use autopush_common::logging;

pub type LocalError = crate::error::ApiError;

const USAGE: &str = "
Usage: autoendpoint [options]
Expand Down
3 changes: 0 additions & 3 deletions autoendpoint/src/middleware/mod.rs

This file was deleted.

7 changes: 5 additions & 2 deletions autoendpoint/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ use cadence::StatsdClient;
use fernet::MultiFernet;
use serde_json::json;

use autopush_common::db::{client::DbClient, dynamodb::DdbClientImpl, DbSettings, StorageType};
use autopush_common::{
db::{client::DbClient, dynamodb::DdbClientImpl, DbSettings, StorageType},
middleware::sentry::SentryWrapper,
};

use crate::error::{ApiError, ApiErrorKind, ApiResult};
use crate::metrics;
Expand Down Expand Up @@ -126,7 +129,7 @@ impl Server {
// Middleware
.wrap(ErrorHandlers::new().handler(StatusCode::NOT_FOUND, ApiError::render_404))
// Our modified Sentry wrapper which does some blocking of non-reportable errors.
.wrap(crate::middleware::sentry::SentryWrapper::new(
.wrap(SentryWrapper::<ApiError>::new(
metrics.clone(),
"api_error".to_owned(),
))
Expand Down
18 changes: 16 additions & 2 deletions autopush-common/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,15 @@ impl ApcErrorKind {
}
}

pub fn metric_label(&self) -> Option<String> {
pub fn metric_label(&self) -> Option<&'static str> {
// TODO: add labels for skipped stuff
let label = match self {
Self::PongTimeout => "pong_timeout",
Self::ExcessivePing => "excessive_ping",
Self::PayloadError(_) => "payload",
_ => return None,
};
Some(label.to_owned())
Some(label)
}
}

Expand All @@ -195,3 +195,17 @@ pub trait ReportableError: std::error::Error + fmt::Display {
/// increment metric instead with this label
fn metric_label(&self) -> Option<&'static str>;
}

impl ReportableError for ApcError {
fn backtrace(&self) -> Option<&Backtrace> {
Some(&self.backtrace)
}

fn is_sentry_event(&self) -> bool {
self.kind.is_sentry_event()
}

fn metric_label(&self) -> Option<&'static str> {
self.kind.metric_label()
}
}
1 change: 1 addition & 0 deletions autopush-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub mod endpoint;
pub mod errors;
pub mod logging;
pub mod metrics;
pub mod middleware;
pub mod notification;
pub mod sentry;
pub mod tags;
Expand Down
1 change: 1 addition & 0 deletions autopush-common/src/middleware/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod sentry;
Loading

0 comments on commit e65486b

Please sign in to comment.