feat: add an autoconnect-web Error type (#432)

and don't report NoWebsocketUpgrades to sentry

SYNC-3889

autoconnect/autoconnect-web/Cargo.toml

@@ -6,15 +6,18 @@ version.workspace = true
 
 [dependencies]
 actix-web.workspace = true
+actix-http.workspace = true
 actix-rt.workspace = true
 actix-ws.workspace = true
+backtrace.workspace = true
 bytes.workspace = true
 bytestring.workspace = true
 cadence.workspace = true
 futures-util.workspace = true
 reqwest.workspace = true
 serde_json.workspace = true
 slog-scope.workspace = true
+thiserror.workspace = true
 uuid.workspace = true
 
 

autoconnect/autoconnect-web/src/dockerflow.rs

@@ -1,13 +1,16 @@
 //! Health and Dockerflow routes
 use std::thread;
 
-use actix_web::web::{Data, Json};
-use actix_web::HttpResponse;
-use reqwest::StatusCode;
+use actix_web::{
+    web::{Data, Json},
+    HttpResponse, ResponseError,
+};
 use serde_json::json;
 
 use autoconnect_settings::AppState;
 
+use crate::error::ApiError;
+
 /// Handle the `/health` and `/__heartbeat__` routes
 pub async fn health_route(state: Data<AppState>) -> Json<serde_json::Value> {
     let status = if state.db.health_check().await.is_ok() {
@@ -55,16 +58,17 @@ pub async fn version_route() -> HttpResponse {
 }
 
 /// Handle the `/v1/err` route
-pub async fn log_check() -> HttpResponse {
+pub async fn log_check() -> Result<HttpResponse, ApiError> {
+    let err = ApiError::LogCheck;
     error!(
         "Test Critical Message";
-        "status_code" => StatusCode::IM_A_TEAPOT.as_u16(),
-        "errno" => 999,
+        "status_code" => err.status_code().as_u16(),
+        "errno" => err.errno(),
     );
 
     thread::spawn(|| {
         panic!("LogCheck");
     });
 
-    HttpResponse::new(StatusCode::IM_A_TEAPOT)
+    Err(err)
 }

autoconnect/autoconnect-web/src/error.rs

@@ -0,0 +1,61 @@
+use actix_http::ws::HandshakeError;
+use actix_web::{error::ResponseError, http::StatusCode, HttpResponse};
+use backtrace::Backtrace;
+use serde_json::json;
+
+use autopush_common::errors::ReportableError;
+
+/// The main error type
+#[derive(thiserror::Error, Debug)]
+pub enum ApiError {
+    #[error("Actix Web error: {0}")]
+    Actix(#[from] actix_web::error::Error),
+
+    #[error("LogCheck")]
+    LogCheck,
+}
+
+impl ResponseError for ApiError {
+    fn status_code(&self) -> StatusCode {
+        match self {
+            ApiError::Actix(e) => e.as_response_error().status_code(),
+            ApiError::LogCheck => StatusCode::IM_A_TEAPOT,
+        }
+    }
+
+    fn error_response(&self) -> HttpResponse {
+        HttpResponse::build(self.status_code()).json(json!({
+            "code": self.status_code().as_u16(),
+            "errno": self.errno(),
+            "error": self.to_string(),
+        }))
+    }
+}
+
+impl ReportableError for ApiError {
+    fn backtrace(&self) -> Option<&Backtrace> {
+        None
+    }
+
+    fn is_sentry_event(&self) -> bool {
+        match self {
+            // Ignore failing upgrade to WebSocket
+            ApiError::Actix(e) => e.as_error() != Some(&HandshakeError::NoWebsocketUpgrade),
+            _ => true,
+        }
+    }
+
+    fn metric_label(&self) -> Option<&'static str> {
+        None
+    }
+}
+
+impl ApiError {
+    /// Return a unique errno code per variant
+    pub fn errno(&self) -> i32 {
+        match self {
+            ApiError::Actix(_) => 500,
+            ApiError::LogCheck => 999,
+        }
+    }
+}

autoconnect/autoconnect-web/src/lib.rs

@@ -6,14 +6,14 @@
 extern crate slog_scope;
 
 pub mod dockerflow;
+pub mod error;
 pub mod metrics;
 pub mod routes;
 #[cfg(test)]
 mod test;
 
 use actix_web::web;
 
-/// Requires import of the `config` function also in this module to use.
 #[macro_export]
 macro_rules! build_app {
     ($app_state: expr) => {
@@ -24,18 +24,18 @@ macro_rules! build_app {
                 autopush_common::errors::render_404,
             ))
             .wrap(autopush_common::middleware::sentry::SentryWrapper::<
-                autopush_common::errors::ApcError,
+                $crate::error::ApiError,
             >::new(
                 $app_state.metrics.clone(), "error".to_owned()
             ))
-            .configure(config)
+            .configure($crate::config)
     };
 }
 
 pub fn config(cfg: &mut web::ServiceConfig) {
     cfg
         // Websocket Handler
-        .route("/", web::get().to(autoconnect_ws::ws_handler))
+        .route("/", web::get().to(routes::ws_route))
         .service(web::resource("/push/{uaid}").route(web::put().to(routes::push_route)))
         .service(web::resource("/notif/{uaid}").route(web::put().to(routes::check_storage_route)))
         .service(web::resource("/status").route(web::get().to(dockerflow::status_route)))

autoconnect/autoconnect-web/src/routes.rs

@@ -1,10 +1,20 @@
-/// Handle incoming notifications from autoendpoint
-use actix_web::{web, HttpResponse};
+use actix_web::{web, HttpRequest, HttpResponse};
 use uuid::Uuid;
 
 use autoconnect_settings::AppState;
 use autopush_common::notification::Notification;
 
+use crate::error::ApiError;
+
+/// Handle WebSocket WebPush clients
+pub async fn ws_route(
+    req: HttpRequest,
+    body: web::Payload,
+    app_state: web::Data<AppState>,
+) -> Result<HttpResponse, ApiError> {
+    Ok(autoconnect_ws::ws_handler(req, body, app_state).await?)
+}
+
 /// Deliver a Push notification directly to a connected client
 pub async fn push_route(
     uaid: web::Path<Uuid>,

autoconnect/autoconnect-web/src/test.rs

@@ -10,7 +10,7 @@ use autoconnect_common::test_support::{hello_again_db, hello_db, DUMMY_UAID, HEL
 use autoconnect_settings::{AppState, Settings};
 use autopush_common::notification::Notification;
 
-use crate::{build_app, config};
+use crate::build_app;
 
 #[ctor::ctor]
 fn init_test_logging() {

autoconnect/src/main.rs

@@ -11,7 +11,7 @@ use docopt::Docopt;
 use serde::Deserialize;
 
 use autoconnect_settings::{AppState, Settings};
-use autoconnect_web::{build_app, config};
+use autoconnect_web::build_app;
 use autopush_common::{
     errors::{ApcErrorKind, Result},
     logging,

tests/test_integration_all_rust.py

@@ -869,6 +869,22 @@ def test_sentry_output(self):
             # This may resolve by updating tests to python3 (see #334)
         assert data["exception"]["values"][0]["value"] == "LogCheck"
 
+    @max_logs(conn=4)
+    def test_no_sentry_output(self):
+        if os.getenv("SKIP_SENTRY"):
+            SkipTest("Skipping sentry test")
+            return
+        ws_url = urlparse(self._ws_url)._replace(scheme="http").geturl()
+        try:
+            requests.get(ws_url)
+        except requests.exceptions.ConnectionError:
+            pass
+        try:
+            data = MOCK_SENTRY_QUEUE.get(timeout=1)
+            assert not data
+        except Empty:
+            pass
+
     @inlineCallbacks
     def test_hello_echo(self):
         client = Client(self._ws_url)