Mercurial > hg
changeset 49374:455fce57e89e stable
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.
author | Raphaël Gomès <rgomes@octobus.net> |
---|---|
date | Wed, 18 May 2022 15:53:28 +0100 |
parents | f8ec7b16c98f |
children | 044e42ae45d9 |
files | rust/hg-core/src/revlog/revlog.rs rust/hg-cpython/src/revlog.rs rust/rhg/src/commands/status.rs |
diffstat | 3 files changed, 37 insertions(+), 18 deletions(-) [+] |
line wrap: on
line diff
--- 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.