feat: quiet remove_node_id's conditional failure case errors (#517)

SYNC-3984

autoendpoint/src/routers/webpush.rs

@@ -147,7 +147,7 @@ impl Router for WebPushRouter {
                 }
             }
             Err(error) => {
-                // Can't communicate with the node, so we should stop using it
+                // Can't communicate with the node, attempt to stop using it
                 debug!("✉ Error while triggering notification check: {}", error);
                 self.remove_node_id(&user, node_id).await?;
                 Ok(self.make_stored_response(notification))
@@ -195,9 +195,13 @@ impl WebPushRouter {
     /// connected to the node.
     async fn remove_node_id(&self, user: &User, node_id: &str) -> ApiResult<()> {
         self.metrics.incr("updates.client.host_gone").ok();
-        self.db
+        let removed = self
+            .db
             .remove_node_id(&user.uaid, node_id, user.connected_at)
             .await?;
+        if !removed {
+            debug!("✉ The node id was not removed");
+        }
         Ok(())
     }
 

autopush-common/src/db/bigtable/bigtable_client/mod.rs

@@ -707,7 +707,12 @@ impl DbClient for BigTableClientImpl {
     }
 
     /// Remove the node_id. Can't really "surgically strike" this
-    async fn remove_node_id(&self, uaid: &Uuid, _node_id: &str, connected_at: u64) -> DbResult<()> {
+    async fn remove_node_id(
+        &self,
+        uaid: &Uuid,
+        _node_id: &str,
+        connected_at: u64,
+    ) -> DbResult<bool> {
         trace!(
             "🉑 Removing node_ids for {} up to {:?} ",
             &uaid.simple().to_string(),
@@ -723,8 +728,9 @@ impl DbClient for BigTableClientImpl {
             &["node_id"].to_vec(),
             Some(&time_range),
         )
-        .await
-        .map_err(|e| e.into())
+        .await?;
+        // TODO: is a conditional check possible?
+        Ok(true)
     }
 
     /// Write the notification to storage.

autopush-common/src/db/client.rs

@@ -46,10 +46,11 @@ pub trait DbClient: Send + Sync {
     /// Remove a channel from a user. Returns if the removed channel did exist.
     async fn remove_channel(&self, uaid: &Uuid, channel_id: &Uuid) -> DbResult<bool>;
 
-    /// Remove the node ID from a user in the router table.
-    /// The node ID will only be cleared if `connected_at` matches up with the
-    /// item's `connected_at`.
-    async fn remove_node_id(&self, uaid: &Uuid, node_id: &str, connected_at: u64) -> DbResult<()>;
+    /// Remove the node ID from a user in the router table. Returns whether the
+    /// removal occurred. The node ID will only be removed if `connected_at`
+    /// matches up with the item's `connected_at`.
+    async fn remove_node_id(&self, uaid: &Uuid, node_id: &str, connected_at: u64)
+        -> DbResult<bool>;
 
     /// Save a message to the message table
     async fn save_message(&self, uaid: &Uuid, message: Notification) -> DbResult<()>;

autopush-common/src/db/dynamodb/mod.rs

@@ -337,7 +337,12 @@ impl DbClient for DdbClientImpl {
             .unwrap_or(false))
     }
 
-    async fn remove_node_id(&self, uaid: &Uuid, node_id: &str, connected_at: u64) -> DbResult<()> {
+    async fn remove_node_id(
+        &self,
+        uaid: &Uuid,
+        node_id: &str,
+        connected_at: u64,
+    ) -> DbResult<bool> {
         let input = UpdateItemInput {
             key: ddb_item! { uaid: s => uaid.simple().to_string() },
             update_expression: Some("REMOVE node_id".to_string()),
@@ -350,14 +355,17 @@ impl DbClient for DdbClientImpl {
             ..Default::default()
         };
 
-        retry_policy()
+        let result = retry_policy()
             .retry_if(
                 || self.db_client.update_item(input.clone()),
                 retryable_updateitem_error(self.metrics.clone()),
             )
-            .await?;
-
-        Ok(())
+            .await;
+        match result {
+            Ok(_) => Ok(true),
+            Err(RusotoError::Service(UpdateItemError::ConditionalCheckFailed(_))) => Ok(false),
+            Err(e) => Err(e.into()),
+        }
     }
 
     async fn fetch_topic_messages(