fix: don't eat WebpushSocket poll_complete errors

instead check specifically for the harmless connection closed error and don't
log it

also remove the stacktrace from ApcError::to_string, include it in the sentry
event exception instead

Issue SYNC-3444

autopush-common/src/errors.rs

@@ -31,19 +31,9 @@ pub struct ApcError {
     pub backtrace: Box<Backtrace>,
 }
 
-// Print out the error and backtrace, including source errors
 impl Display for ApcError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "Error: {}\nBacktrace: \n{:?}", self.kind, self.backtrace)?;
-
-        // Go down the chain of errors
-        let mut error: &dyn std::error::Error = &self.kind;
-        while let Some(source) = error.source() {
-            write!(f, "\n\nCaused by: {source}")?;
-            error = source;
-        }
-
-        Ok(())
+        self.kind.fmt(f)
     }
 }
 

autopush/src/client.rs

@@ -638,6 +638,9 @@ where
         let error = if let Some(ref err) = error {
             let ua_info = ua_info.clone();
             let mut event = sentry::event_from_error(err);
+            event.exception.last_mut().unwrap().stacktrace =
+                sentry::integrations::backtrace::backtrace_to_stacktrace(&err.backtrace);
+
             event.user = Some(sentry::User {
                 id: Some(webpush.uaid.as_simple().to_string()),
                 ..Default::default()

autopush/src/server/mod.rs

@@ -406,12 +406,14 @@ impl Server {
                     handle.spawn(client.then(move |res| {
                         srv.open_connections.set(srv.open_connections.get() - 1);
                         if let Err(e) = res {
-                            let mut errstr: &str = &e.to_string();
-                            if errstr.contains("Connection closed normally") {
-                                // we don't need the stack for a Success Error.
-                                errstr = errstr.split('\n').collect::<Vec<&str>>()[0];
+                            // No need to log ConnectionClosed (denotes the
+                            // connection closed normally)
+                            if !matches!(
+                                e.kind,
+                                ApcErrorKind::Ws(tungstenite::error::Error::ConnectionClosed)
+                            ) {
+                                debug!("🤫 {}: {}", addr, e.to_string());
                             }
-                            debug!("🤫 {}: {}", addr, errstr);
                         }
                         Ok(())
                     }));
@@ -909,14 +911,9 @@ where
     }
 
     /// Handle the poll completion.
-    /// Note, that this eats the errors of poll_complete(), because one of the known states
-    /// is "Error: Connection closed normally" which can raise a false error condition.
     fn poll_complete(&mut self) -> Poll<(), ApcError> {
         try_ready!(self.send_ws_ping());
-        if self.inner.poll_complete().is_err() {
-            warn!("Error encountered with poll_complete");
-        }
-        Ok(Async::Ready(()))
+        Ok(self.inner.poll_complete()?)
     }
 
     fn close(&mut self) -> Poll<(), ApcError> {