From 9657d2c88dc4a14a479b09f7fb953a9e5951dd28 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Wed, 25 Oct 2023 22:22:21 -0700 Subject: [PATCH 1/2] fix: capture sentry events for the unidentified state SYNC-3975 --- .../autoconnect-ws-sm/src/identified/mod.rs | 4 ++-- autoconnect/autoconnect-ws/src/error.rs | 16 ++++++++++++++-- autoconnect/autoconnect-ws/src/handler.rs | 15 ++++++++------- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/mod.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/mod.rs index d7b0c76f0..7be27c58e 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/mod.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/mod.rs @@ -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); diff --git a/autoconnect/autoconnect-ws/src/error.rs b/autoconnect/autoconnect-ws/src/error.rs index 542b78dbc..2d8b6960c 100644 --- a/autoconnect/autoconnect-ws/src/error.rs +++ b/autoconnect/autoconnect-ws/src/error.rs @@ -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)] @@ -51,6 +51,18 @@ impl WSError { pub fn close_description(&self) -> &str { self.kind.as_ref() } + + /// Emit an event for this Error to Sentry if set to + pub fn capture_sentry_event(&self, client: Option) { + 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 { diff --git a/autoconnect/autoconnect-ws/src/handler.rs b/autoconnect/autoconnect-ws/src/handler.rs index fd8a2f24e..3b701b8b1 100644 --- a/autoconnect/autoconnect-ws/src/handler.rs +++ b/autoconnect/autoconnect-ws/src/handler.rs @@ -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}, @@ -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; @@ -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 } From 5c340dd71a5c4d692e696865516602cf0e935424 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Wed, 25 Oct 2023 22:24:56 -0700 Subject: [PATCH 2/2] fix: kill db settings defaults and move the chatty error! log to trace SYNC-3976 --- autoconnect/autoconnect-ws/src/error.rs | 2 +- autoconnect/autoconnect-ws/src/handler.rs | 2 +- autopush-common/src/db/bigtable/mod.rs | 20 -------------------- autopush-common/src/db/dynamodb/mod.rs | 9 --------- 4 files changed, 2 insertions(+), 31 deletions(-) diff --git a/autoconnect/autoconnect-ws/src/error.rs b/autoconnect/autoconnect-ws/src/error.rs index 2d8b6960c..5d656c444 100644 --- a/autoconnect/autoconnect-ws/src/error.rs +++ b/autoconnect/autoconnect-ws/src/error.rs @@ -52,7 +52,7 @@ impl WSError { self.kind.as_ref() } - /// Emit an event for this Error to Sentry if set to + /// Emit an event for this Error to Sentry pub fn capture_sentry_event(&self, client: Option) { if !self.is_sentry_event() { return; diff --git a/autoconnect/autoconnect-ws/src/handler.rs b/autoconnect/autoconnect-ws/src/handler.rs index 3b701b8b1..3bce5706d 100644 --- a/autoconnect/autoconnect-ws/src/handler.rs +++ b/autoconnect/autoconnect-ws/src/handler.rs @@ -29,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()), diff --git a/autopush-common/src/db/bigtable/mod.rs b/autopush-common/src/db/bigtable/mod.rs index c2ed5a288..35d28d393 100644 --- a/autopush-common/src/db/bigtable/mod.rs +++ b/autopush-common/src/db/bigtable/mod.rs @@ -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}` @@ -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 { diff --git a/autopush-common/src/db/dynamodb/mod.rs b/autopush-common/src/db/dynamodb/mod.rs index ac9a839ab..6965862bc 100644 --- a/autopush-common/src/db/dynamodb/mod.rs +++ b/autopush-common/src/db/dynamodb/mod.rs @@ -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 {