rust: don't swallow valuable error information stable
authorRaphaël Gomès <rgomes@octobus.net>
Wed, 18 May 2022 15:53:28 +0100
branchstable
changeset 49374 455fce57e89e
parent 49373 f8ec7b16c98f
child 49375 044e42ae45d9
rust: don't swallow valuable error information This helps when diagnosing corruption, and is in general good practice. The information is here, valuable and can be used easily.
rust/hg-core/src/revlog/revlog.rs
rust/hg-cpython/src/revlog.rs
rust/rhg/src/commands/status.rs
--- a/rust/hg-core/src/revlog/revlog.rs	Wed May 18 09:50:39 2022 +0100
+++ b/rust/hg-core/src/revlog/revlog.rs	Wed May 18 15:53:28 2022 +0100
@@ -49,18 +49,20 @@
     fn from(error: NodeMapError) -> Self {
         match error {
             NodeMapError::MultipleResults => RevlogError::AmbiguousPrefix,
-            NodeMapError::RevisionNotInIndex(_) => RevlogError::corrupted(),
+            NodeMapError::RevisionNotInIndex(rev) => RevlogError::corrupted(
+                format!("nodemap point to revision {} not in index", rev),
+            ),
         }
     }
 }
 
-fn corrupted() -> HgError {
-    HgError::corrupted("corrupted revlog")
+fn corrupted<S: AsRef<str>>(context: S) -> HgError {
+    HgError::corrupted(format!("corrupted revlog, {}", context.as_ref()))
 }
 
 impl RevlogError {
-    fn corrupted() -> Self {
-        RevlogError::Other(corrupted())
+    fn corrupted<S: AsRef<str>>(context: S) -> Self {
+        RevlogError::Other(corrupted(context))
     }
 }
 
@@ -329,7 +331,8 @@
         &self,
         rev: Revision,
     ) -> Result<RevlogEntry, HgError> {
-        return self.get_entry(rev).map_err(|_| corrupted());
+        self.get_entry(rev)
+            .map_err(|_| corrupted(format!("revision {} out of range", rev)))
     }
 }
 
@@ -449,7 +452,10 @@
         ) {
             Ok(data)
         } else {
-            Err(corrupted())
+            Err(corrupted(format!(
+                "hash check failed for revision {}",
+                self.rev
+            )))
         }
     }
 
@@ -478,7 +484,10 @@
             // zstd data.
             b'\x28' => Ok(Cow::Owned(self.uncompressed_zstd_data()?)),
             // A proper new format should have had a repo/store requirement.
-            _format_type => Err(corrupted()),
+            format_type => Err(corrupted(format!(
+                "unknown compression header '{}'",
+                format_type
+            ))),
         }
     }
 
@@ -486,12 +495,16 @@
         let mut decoder = ZlibDecoder::new(self.bytes);
         if self.is_delta() {
             let mut buf = Vec::with_capacity(self.compressed_len as usize);
-            decoder.read_to_end(&mut buf).map_err(|_| corrupted())?;
+            decoder
+                .read_to_end(&mut buf)
+                .map_err(|e| corrupted(e.to_string()))?;
             Ok(buf)
         } else {
             let cap = self.uncompressed_len.max(0) as usize;
             let mut buf = vec![0; cap];
-            decoder.read_exact(&mut buf).map_err(|_| corrupted())?;
+            decoder
+                .read_exact(&mut buf)
+                .map_err(|e| corrupted(e.to_string()))?;
             Ok(buf)
         }
     }
@@ -500,15 +513,15 @@
         if self.is_delta() {
             let mut buf = Vec::with_capacity(self.compressed_len as usize);
             zstd::stream::copy_decode(self.bytes, &mut buf)
-                .map_err(|_| corrupted())?;
+                .map_err(|e| corrupted(e.to_string()))?;
             Ok(buf)
         } else {
             let cap = self.uncompressed_len.max(0) as usize;
             let mut buf = vec![0; cap];
             let len = zstd::block::decompress_to_buffer(self.bytes, &mut buf)
-                .map_err(|_| corrupted())?;
+                .map_err(|e| corrupted(e.to_string()))?;
             if len != self.uncompressed_len as usize {
-                Err(corrupted())
+                Err(corrupted("uncompressed length does not match"))
             } else {
                 Ok(buf)
             }
--- a/rust/hg-cpython/src/revlog.rs	Wed May 18 09:50:39 2022 +0100
+++ b/rust/hg-cpython/src/revlog.rs	Wed May 18 15:53:28 2022 +0100
@@ -107,7 +107,10 @@
             String::from_utf8_lossy(node.data(py)).to_string()
         };
 
-        let prefix = NodePrefix::from_hex(&node_as_string).map_err(|_| PyErr::new::<ValueError, _>(py, "Invalid node or prefix"))?;
+        let prefix = NodePrefix::from_hex(&node_as_string)
+            .map_err(|_| PyErr::new::<ValueError, _>(
+                py, format!("Invalid node or prefix '{}'", node_as_string))
+            )?;
 
         nt.find_bin(idx, prefix)
             // TODO make an inner API returning the node directly
--- a/rust/rhg/src/commands/status.rs	Wed May 18 09:50:39 2022 +0100
+++ b/rust/rhg/src/commands/status.rs	Wed May 18 15:53:28 2022 +0100
@@ -517,10 +517,13 @@
     }
     let filelog = repo.filelog(hg_path)?;
     let fs_len = fs_metadata.len();
-    let filelog_entry =
-        filelog.entry_for_node(entry.node_id()?).map_err(|_| {
-            HgError::corrupted("filelog missing node from manifest")
-        })?;
+    let file_node = entry.node_id()?;
+    let filelog_entry = filelog.entry_for_node(file_node).map_err(|_| {
+        HgError::corrupted(format!(
+            "filelog missing node {:?} from manifest",
+            file_node
+        ))
+    })?;
     if filelog_entry.file_data_len_not_equal_to(fs_len) {
         // No need to read file contents:
         // it cannot be equal if it has a different length.