Skip to content

Commit

Permalink
feat: clean-up sentry error reporting and reduce spurious reporting
Browse files Browse the repository at this point in the history
Log out full user-agent information as tags for easier filtering.
Ignore excessive ping errors in reporting.

Closes #71
  • Loading branch information
bbangert committed Sep 5, 2018
1 parent cea8b8e commit 43bf889
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 26 deletions.
24 changes: 8 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ reqwest = { version = "0.8.6", features = ["unstable"] }
rusoto_core = "0.32.0"
rusoto_credential = "0.11.0"
rusoto_dynamodb = "0.32.0"
sentry = { version = "0.9.0", features = ["with_error_chain"] }
# XXX: pinned until server side's upgraded
serde = "1.0.70"
serde_derive = "1.0.70"
Expand Down Expand Up @@ -70,8 +71,3 @@ woothee = "0.7.3"
[dependencies.config]
git = "https://github.com/mehcode/config-rs"
rev = "e8fa9fee96185ddd18ebcef8a925c75459111edb"

[dependencies.sentry]
features = ["with_error_chain"]
git = "https://github.com/getsentry/sentry-rust/"
rev = "9656b8b9bd1fd65f3d8be744ede1c9089019e087"
21 changes: 17 additions & 4 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,8 @@ where
| Err(Error(ErrorKind::Ws(_), _))
| Err(Error(ErrorKind::Io(_), _))
| Err(Error(ErrorKind::PongTimeout, _))
| Err(Error(ErrorKind::RepeatUaidDisconnect, _)) => None,
| Err(Error(ErrorKind::RepeatUaidDisconnect, _))
| Err(Error(ErrorKind::ExcessivePing, _)) => None,
Err(e) => Some(e),
}
};
Expand Down Expand Up @@ -504,6 +505,16 @@ where
// Log out the sentry message if applicable and convert to error msg
let error = if let Some(ref err) = error {
let mut event = event_from_error_chain(err);
event.user = Some(sentry::User {
id: Some(webpush.uaid.simple().to_string()),
..Default::default()
});
event.tags.insert("ua_name".to_string(), ua_result.name.to_string());
event.tags.insert("ua_os_family".to_string(), metrics_os.to_string());
event.tags.insert("ua_os_ver".to_string(), ua_result.os_version.to_string());
event.tags.insert("ua_browser_family".to_string(), metrics_browser.to_string());
event.tags.insert("ua_browser_ver".to_string(), ua_result.version.to_string());
event.tags.insert("ua_category".to_string(), ua_result.category.to_string());
sentry::capture_event(event);
err.display_chain().to_string()
} else {
Expand Down Expand Up @@ -780,6 +791,9 @@ where
let webpush_rc = data.webpush.clone();
let mut webpush = webpush_rc.borrow_mut();
match input {
Either::A(ClientMessage::Hello { .. }) => {
Err(ErrorKind::InvalidStateTransition("AwaitInput".to_string(), "Hello".to_string()).into())
}
Either::A(ClientMessage::BroadcastSubscribe { broadcasts }) => {
let broadcast_delta = {
let mut broadcast_subs = data.broadcast_subs.borrow_mut();
Expand Down Expand Up @@ -906,7 +920,7 @@ where
Either::A(ClientMessage::Ping) => {
// Clients shouldn't ping > than once per minute or we
// disconnect them
if sec_since_epoch() - webpush.last_ping >= 55 {
if sec_since_epoch() - webpush.last_ping >= 45 {
debug!("Got a ping, sending pong");
webpush.last_ping = sec_since_epoch();
transition!(Send {
Expand All @@ -915,7 +929,7 @@ where
})
} else {
debug!("Got a ping too quickly, disconnecting");
Err("code=4774".into())
Err(ErrorKind::ExcessivePing.into())
}
}
Either::B(ServerNotification::Notification(notif)) => {
Expand All @@ -938,7 +952,6 @@ where
debug!("Got told to disconnect, connecting client has our uaid");
Err(ErrorKind::RepeatUaidDisconnect.into())
}
_ => Err("Invalid message".into()),
}
}

Expand Down
10 changes: 9 additions & 1 deletion src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
//!
//! You can find some more documentation about this in the `error-chain` crate
//! online.
use std::any::Any;
use std::error;
use std::io;
Expand Down Expand Up @@ -66,6 +65,15 @@ error_chain! {
RepeatUaidDisconnect {
description("repeat uaid disconnected")
}

ExcessivePing {
description("pings are not far enough apart")
}

InvalidStateTransition(from: String, to: String) {
description("invalid state transition")
display("invalid state transition, from: {}, to: {}", from, to)
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
//! aren't as relevant to the Web Push implementation.
//!
//! Otherwise be sure to check out each module for more documentation!
// `error_chain!` can recurse deeply
#![recursion_limit = "1024"]
extern crate base64;
extern crate bytes;
extern crate cadence;
Expand Down

0 comments on commit 43bf889

Please sign in to comment.