From bd2169d80bf2ffe6bc34900e4ba3a633719b7db4 Mon Sep 17 00:00:00 2001 From: Richard Hua Date: Wed, 12 Jun 2024 10:08:55 -0700 Subject: [PATCH] Add more context to error logs (#832) --- xmtp_mls/src/client.rs | 5 ++++- xmtp_mls/src/groups/mod.rs | 2 +- xmtp_mls/src/groups/sync.rs | 2 +- xmtp_mls/src/storage/encrypted_store/group.rs | 5 ++++- .../storage/encrypted_store/group_intent.rs | 20 +++++++++++++------ .../storage/encrypted_store/refresh_state.rs | 10 +++++++--- xmtp_mls/src/storage/errors.rs | 4 ++-- 7 files changed, 33 insertions(+), 15 deletions(-) diff --git a/xmtp_mls/src/client.rs b/xmtp_mls/src/client.rs index 486c12e90..e1e3b5ba3 100644 --- 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 index b04848df4..6ff781c84 100644 --- 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 index ba1096c33..2aa95f2e7 100644 --- 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 index 2aafd4105..8cca89419 100644 --- 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 index 859b045b3..e42eb41b8 100644 --- 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 index f5bc848b1..1c9e03478 100644 --- 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>>( + pub fn update_cursor( &self, - entity_id: IdType, + entity_id: &Vec, entity_kind: EntityKind, cursor: i64, ) -> Result { @@ -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 index b0d70432e..312b5db1e 100644 --- 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")]