Add more context to error logs (#832)

xmtp · Jun 12, 2024 · bd2169d · bd2169d
1 parent 8f8b2ac
commit bd2169d
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 15 deletions.
diff --git a/xmtp_mls/src/client.rs b/xmtp_mls/src/client.rs
@@ -314,7 +314,10 @@ where
                 group.id,
                 group.created_at_ns,
             )),
-            None => Err(ClientError::Storage(StorageError::NotFound)),
+            None => Err(ClientError::Storage(StorageError::NotFound(format!(
+                "group {}",
+                hex::encode(group_id)
+            )))),
         }
     }
 

diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs
@@ -431,7 +431,7 @@ impl MlsGroup {
 
         // Skipping a full sync here and instead just firing and forgetting
         if let Err(err) = self.publish_intents(conn, client).await {
-            println!("error publishing intents: {:?}", err);
+            log::error!("Send: error publishing intents: {:?}", err);
         }
         Ok(message_id)
     }

diff --git a/xmtp_mls/src/groups/sync.rs b/xmtp_mls/src/groups/sync.rs
@@ -97,7 +97,7 @@ impl MlsGroup {
 
         // Even if publish fails, continue to receiving
         if let Err(publish_error) = self.publish_intents(conn.clone(), client).await {
-            log::error!("error publishing intents {:?}", publish_error);
+            log::error!("Sync: error publishing intents {:?}", publish_error);
             errors.push(publish_error);
         }
 

diff --git a/xmtp_mls/src/storage/encrypted_store/group.rs b/xmtp_mls/src/storage/encrypted_store/group.rs
@@ -154,7 +154,10 @@ impl DbConnection {
             Ok(ts)
         })?;
 
-        last_ts.ok_or(StorageError::NotFound)
+        last_ts.ok_or(StorageError::NotFound(format!(
+            "installation time for group {}",
+            hex::encode(group_id)
+        )))
     }
 
     /// Updates the 'last time checked' we checked for new installations.

diff --git a/xmtp_mls/src/storage/encrypted_store/group_intent.rs b/xmtp_mls/src/storage/encrypted_store/group_intent.rs
@@ -144,7 +144,9 @@ impl DbConnection {
 
         match res {
             // If nothing matched the query, return an error. Either ID or state was wrong
-            0 => Err(StorageError::NotFound),
+            0 => Err(StorageError::NotFound(format!(
+                "ToPublish intent {intent_id} for publish"
+            ))),
             _ => Ok(()),
         }
     }
@@ -163,7 +165,9 @@ impl DbConnection {
 
         match res {
             // If nothing matched the query, return an error. Either ID or state was wrong
-            0 => Err(StorageError::NotFound),
+            0 => Err(StorageError::NotFound(format!(
+                "Published intent {intent_id} for commit"
+            ))),
             _ => Ok(()),
         }
     }
@@ -188,7 +192,9 @@ impl DbConnection {
 
         match res {
             // If nothing matched the query, return an error. Either ID or state was wrong
-            0 => Err(StorageError::NotFound),
+            0 => Err(StorageError::NotFound(format!(
+                "Published intent {intent_id} for ToPublish"
+            ))),
             _ => Ok(()),
         }
     }
@@ -204,7 +210,9 @@ impl DbConnection {
 
         match res {
             // If nothing matched the query, return an error. Either ID or state was wrong
-            0 => Err(StorageError::NotFound),
+            0 => Err(StorageError::NotFound(format!(
+                "state for intent {intent_id}"
+            ))),
             _ => Ok(()),
         }
     }
@@ -593,14 +601,14 @@ mod tests {
             assert!(commit_result.is_err());
             assert!(matches!(
                 commit_result.err().unwrap(),
-                StorageError::NotFound
+                StorageError::NotFound(_)
             ));
 
             let to_publish_result = conn.set_group_intent_to_publish(intent.id);
             assert!(to_publish_result.is_err());
             assert!(matches!(
                 to_publish_result.err().unwrap(),
-                StorageError::NotFound
+                StorageError::NotFound(_)
             ));
         })
     }

diff --git a/xmtp_mls/src/storage/encrypted_store/refresh_state.rs b/xmtp_mls/src/storage/encrypted_store/refresh_state.rs
@@ -89,9 +89,9 @@ impl DbConnection {
         }
     }
 
-    pub fn update_cursor<IdType: AsRef<Vec<u8>>>(
+    pub fn update_cursor(
         &self,
-        entity_id: IdType,
+        entity_id: &Vec<u8>,
         entity_kind: EntityKind,
         cursor: i64,
     ) -> Result<bool, StorageError> {
@@ -107,7 +107,11 @@ impl DbConnection {
                 })?;
                 Ok(num_updated == 1)
             }
-            None => Err(StorageError::NotFound),
+            None => Err(StorageError::NotFound(format!(
+                "state for entity ID {} with kind {:?}",
+                hex::encode(entity_id),
+                entity_kind
+            ))),
         }
     }
 }

diff --git a/xmtp_mls/src/storage/errors.rs b/xmtp_mls/src/storage/errors.rs
@@ -22,8 +22,8 @@ pub enum StorageError {
     Serialization(String),
     #[error("deserialization error")]
     Deserialization(String),
-    #[error("not found")]
-    NotFound,
+    #[error("{0} not found")]
+    NotFound(String),
     #[error("lock")]
     Lock(String),
     #[error("Pool needs to  reconnect before use")]