feat: clean-up sentry error reporting and reduce spurious reporting

Log out full user-agent information as tags for easier filtering.
Ignore excessive ping errors in reporting.

Closes #71

Cargo.toml

@@ -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"
@@ -74,4 +75,5 @@ rev = "e8fa9fee96185ddd18ebcef8a925c75459111edb"
 [dependencies.sentry]
 features = ["with_error_chain"]
 git = "https://github.com/getsentry/sentry-rust/"
-rev = "9656b8b9bd1fd65f3d8be744ede1c9089019e087"
+# Sentry 0.9.0 is broken, this is the last commit that works before 0.9.0
+rev = "0e99cdd6bc5377ce151084e432a66af91e4b5d17"

src/client.rs

@@ -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),
             }
         };
@@ -504,6 +505,15 @@ 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());
             sentry::capture_event(event);
             err.display_chain().to_string()
         } else {
@@ -780,6 +790,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();
@@ -906,7 +919,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 {
@@ -915,7 +928,7 @@ where
                     })
                 } else {
                     debug!("Got a ping too quickly, disconnecting");
-                    Err("code=4774".into())
+                    Err(ErrorKind::ExcessivePing.into())
                 }
             }
             Either::B(ServerNotification::Notification(notif)) => {
@@ -938,7 +951,6 @@ where
                 debug!("Got told to disconnect, connecting client has our uaid");
                 Err(ErrorKind::RepeatUaidDisconnect.into())
             }
-            _ => Err("Invalid message".into()),
         }
     }
 

src/errors.rs

@@ -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;
@@ -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)
+        }
     }
 }
 

src/lib.rs

@@ -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;

tests/test_integration.py

@@ -306,11 +306,14 @@ def test_sentry_output(self):
         client = Client(self._ws_url)
         yield client.connect()
         yield client.hello()
-        # Send unsolicited ack and drop
-        yield client.ack("garbage", "data")
+        # Send a duplicate hello
+        try:
+            yield client.hello()
+        except ValueError:
+            pass
         yield self.shut_down(client)
         data = MOCK_SENTRY_QUEUE.get(timeout=1)
-        assert data["exception"]["values"][0]["value"] == "invalid json text"
+        assert data["exception"]["values"][0]["value"].startswith("invalid")
 
     @inlineCallbacks
     def test_hello_echo(self):