From 1d290d79472959bcbbc349c4b72c1197f47df7f2 Mon Sep 17 00:00:00 2001
From: Sammy Khamis <sbkhamis@gmail.com>
Date: Tue, 21 Jun 2022 17:51:02 -1000
Subject: [PATCH] Normalize guid errors to allow easier anonymizing/aggregation
 of data (#71)

* Normalize guid-related errors to allow easier anonymizing/aggregation of data
---
 src/error.rs | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/error.rs b/src/error.rs
index f40142e..b950062 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -57,35 +57,42 @@ impl From<Utf8Error> for Error {
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        // We format the guid-specific params with <guid: {}> to make it easier on the
+        // telemetry side to parse out the user-specific guid and normalize the errors
+        // to better aggregate the data
         match self.kind() {
             ErrorKind::MismatchedItemKind(local_item, remote_item) => write!(
                 f,
-                "Can't merge local {} {} and remote {} {}",
+                "Can't merge local {} <guid: {}> and remote {} <guid: {}>",
                 local_item.kind, local_item.guid, remote_item.kind, remote_item.guid,
             ),
-            ErrorKind::DuplicateItem(guid) => write!(f, "Item {} already exists in tree", guid),
-            ErrorKind::MissingItem(guid) => write!(f, "Item {} doesn't exist in tree", guid),
+            ErrorKind::DuplicateItem(guid) => {
+                write!(f, "Item <guid: {}> already exists in tree", guid)
+            }
+            ErrorKind::MissingItem(guid) => {
+                write!(f, "Item <guid: {}> doesn't exist in tree", guid)
+            }
             ErrorKind::InvalidParent(child, parent) => write!(
                 f,
-                "Can't insert {} {} into {} {}",
+                "Can't insert {} <guid: {}> into {} <guid: {}>",
                 child.kind, child.guid, parent.kind, parent.guid,
             ),
             ErrorKind::InvalidParentForUnknownChild(child_guid, parent) => write!(
                 f,
-                "Can't insert unknown child {} into {} {}",
+                "Can't insert unknown child <guid: {}> into {} <guid: {}>",
                 child_guid, parent.kind, parent.guid,
             ),
             ErrorKind::MissingParent(child, parent_guid) => write!(
                 f,
-                "Can't insert {} {} into nonexistent parent {}",
+                "Can't insert {} <guid: {}> into nonexistent parent <guid: {}>",
                 child.kind, child.guid, parent_guid,
             ),
             ErrorKind::MissingParentForUnknownChild(child_guid, parent_guid) => write!(
                 f,
-                "Can't insert unknown child {} into nonexistent parent {}",
+                "Can't insert unknown child <guid: {}> into nonexistent parent <guid: {}>",
                 child_guid, parent_guid,
             ),
-            ErrorKind::Cycle(guid) => write!(f, "Item {} can't contain itself", guid),
+            ErrorKind::Cycle(guid) => write!(f, "Item <guid: {}> can't contain itself", guid),
             ErrorKind::MergeConflict => write!(f, "Local tree changed during merge"),
             ErrorKind::UnmergedLocalItems => {
                 write!(f, "Merged tree doesn't mention all items from local tree")
@@ -94,9 +101,13 @@ impl fmt::Display for Error {
                 write!(f, "Merged tree doesn't mention all items from remote tree")
             }
             ErrorKind::InvalidGuid(invalid_guid) => {
-                write!(f, "Merged tree contains invalid GUID {}", invalid_guid)
+                write!(
+                    f,
+                    "Merged tree contains invalid GUID <guid: {}>",
+                    invalid_guid
+                )
             }
-            ErrorKind::InvalidByte(b) => write!(f, "Invalid byte {} in UTF-16 encoding", b),
+            ErrorKind::InvalidByte(b) => write!(f, "Invalid byte <byte: {}> in UTF-16 encoding", b),
             ErrorKind::MalformedString(err) => err.fmt(f),
             ErrorKind::Abort => write!(f, "Operation aborted"),
         }