Skip to content

Commit

Permalink
fix: capture sentry events for the unidentified state (#484)
Browse files Browse the repository at this point in the history
also kill db settings defaults and move the chatty error! log to trace

SYNC-3975
SYNC-3976
  • Loading branch information
pjenvey committed Oct 26, 2023
1 parent cd597d3 commit 09db55f
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,12 @@ impl WebPushClient {
}

/// Add User information and tags for this Client to a Sentry Event
pub fn add_sentry_info(&self, event: &mut sentry::protocol::Event) {
pub fn add_sentry_info(self, event: &mut sentry::protocol::Event) {
event.user = Some(sentry::User {
id: Some(self.uaid.as_simple().to_string()),
..Default::default()
});
let ua_info = self.ua_info.clone();
let ua_info = self.ua_info;
event
.tags
.insert("ua_name".to_owned(), ua_info.browser_name);
Expand Down
16 changes: 14 additions & 2 deletions autoconnect/autoconnect-ws/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use std::fmt;
use actix_ws::CloseCode;
use backtrace::Backtrace;

use autoconnect_ws_sm::SMError;
use autopush_common::errors::ReportableError;
use autoconnect_ws_sm::{SMError, WebPushClient};
use autopush_common::{errors::ReportableError, sentry::event_from_error};

/// WebPush WebSocket Handler Errors
#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -51,6 +51,18 @@ impl WSError {
pub fn close_description(&self) -> &str {
self.kind.as_ref()
}

/// Emit an event for this Error to Sentry
pub fn capture_sentry_event(&self, client: Option<WebPushClient>) {
if !self.is_sentry_event() {
return;
}
let mut event = event_from_error(self);
if let Some(client) = client {
client.add_sentry_info(&mut event);
}
sentry::capture_event(event);
}
}

impl ReportableError for WSError {
Expand Down
17 changes: 9 additions & 8 deletions autoconnect/autoconnect-ws/src/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use tokio::{select, time::timeout};
use autoconnect_common::protocol::{ServerMessage, ServerNotification};
use autoconnect_settings::AppState;
use autoconnect_ws_sm::{UnidentifiedClient, WebPushClient};
use autopush_common::{errors::ReportableError, sentry::event_from_error};

use crate::{
error::{WSError, WSErrorKind},
Expand All @@ -30,7 +29,7 @@ pub fn spawn_webpush_ws(
let close_reason = webpush_ws(client, &mut session, msg_stream)
.await
.unwrap_or_else(|e| {
error!("spawn_webpush_ws: Error: {}", e);
trace!("spawn_webpush_ws: Error: {}", e);
Some(CloseReason {
code: e.close_code(),
description: Some(e.close_description().to_owned()),
Expand Down Expand Up @@ -61,7 +60,13 @@ pub(crate) async fn webpush_ws(
// NOTE: UnidentifiedClient doesn't require shutdown/cleanup, so its
// Error's propagated. We don't propagate Errors afterwards to handle
// shutdown/cleanup of WebPushClient
let (mut client, smsgs) = unidentified_ws(client, &mut msg_stream).await?;
let (mut client, smsgs) = match unidentified_ws(client, &mut msg_stream).await {
Ok(t) => t,
Err(e) => {
e.capture_sentry_event(None);
return Err(e);
}
};

// Client now identified: add them to the registry to recieve ServerNotifications
let mut snotif_stream = client.registry_connect().await;
Expand All @@ -75,11 +80,7 @@ pub(crate) async fn webpush_ws(
client.shutdown(result.as_ref().err().map(|e| e.to_string()));

if let Err(ref e) = result {
if e.is_sentry_event() {
let mut event = event_from_error(e);
client.add_sentry_info(&mut event);
sentry::capture_event(event);
}
e.capture_sentry_event(Some(client));
}
result
}
Expand Down
20 changes: 0 additions & 20 deletions autopush-common/src/db/bigtable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ use crate::db::error::DbError;

/// The settings for accessing the BigTable contents.
#[derive(Clone, Debug, Deserialize)]
#[serde(default)]
pub struct BigTableDbSettings {
/// The Table name matches the GRPC template for table paths.
/// e.g. `projects/{projectid}/instances/{instanceid}/tables/{tablename}`
Expand All @@ -48,25 +47,6 @@ pub struct BigTableDbSettings {
pub message_topic_family: String,
}

/// NOTE: autopush will not autogenerate these families. They should
/// be created when the table is first provisioned. See
/// [BigTable schema](https://cloud.google.com/bigtable/docs/schema-design)
///
/// BE SURE TO CONFIRM the names of the families. These are not checked on
/// initialization, but will throw errors if not present or incorrectly
/// spelled.
///
impl Default for BigTableDbSettings {
fn default() -> Self {
Self {
table_name: "autopush".to_owned(),
router_family: "router".to_owned(),
message_family: "message".to_owned(),
message_topic_family: "message_topic".to_owned(),
}
}
}

impl TryFrom<&str> for BigTableDbSettings {
type Error = DbError;
fn try_from(setting_string: &str) -> Result<Self, Self::Error> {
Expand Down
9 changes: 0 additions & 9 deletions autopush-common/src/db/dynamodb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,6 @@ pub struct DynamoDbSettings {
pub message_table: String,
}

impl Default for DynamoDbSettings {
fn default() -> Self {
Self {
router_table: "router".to_string(),
message_table: "message".to_string(),
}
}
}

impl TryFrom<&str> for DynamoDbSettings {
type Error = DbError;
fn try_from(setting_string: &str) -> Result<Self, Self::Error> {
Expand Down

0 comments on commit 09db55f

Please sign in to comment.