From 9881abf08358a27b90763092ed21a336cce5fc1e Mon Sep 17 00:00:00 2001 From: waelwindows Date: Mon, 6 Feb 2023 18:59:23 +0300 Subject: [PATCH] feat: Expose better errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous errors didn't allow API consumers to inspect the underlying cause for their error. Whether it was to handle the error manually or to display a proper "error-chain" for the errors (à la `eyre`, `anyhow`, etc). --- .envrc | 1 + .gitignore | 4 ++ Cargo.toml | 1 + flake.lock | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++ flake.nix | 44 ++++++++++++++ src/lib.rs | 109 ++++++++++++++++++++------------- 6 files changed, 290 insertions(+), 42 deletions(-) create mode 100644 .envrc create mode 100644 flake.lock create mode 100644 flake.nix diff --git a/.envrc b/.envrc new file mode 100644 index 00000000..c4b17d79 --- /dev/null +++ b/.envrc @@ -0,0 +1 @@ +use_flake diff --git a/.gitignore b/.gitignore index e0ba890b..4da33048 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,7 @@ *~ target Cargo.lock + +# Nix +.direnv/ +result* diff --git a/Cargo.toml b/Cargo.toml index d421e424..c27aa88a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,3 +14,4 @@ edition = "2018" encoding = "0.2.33" lazy_static = "1.4.0" regex = "1.5.5" +thiserror = "1.0.38" diff --git a/flake.lock b/flake.lock new file mode 100644 index 00000000..6624d881 --- /dev/null +++ b/flake.lock @@ -0,0 +1,173 @@ +{ + "nodes": { + "crane": { + "inputs": { + "flake-compat": "flake-compat", + "flake-utils": "flake-utils", + "nixpkgs": [ + "nixpkgs" + ], + "rust-overlay": "rust-overlay" + }, + "locked": { + "lastModified": 1675475924, + "narHash": "sha256-KWdfV9a6+zG6Ij/7PZYLnomjBZZUu8gdRy+hfjGrrJQ=", + "owner": "ipetkov", + "repo": "crane", + "rev": "1bde9c762ebf26de9f8ecf502357c92105bc4577", + "type": "github" + }, + "original": { + "owner": "ipetkov", + "repo": "crane", + "type": "github" + } + }, + "flake-compat": { + "flake": false, + "locked": { + "lastModified": 1673956053, + "narHash": "sha256-4gtG9iQuiKITOjNQQeQIpoIB6b16fm+504Ch3sNKLd8=", + "owner": "edolstra", + "repo": "flake-compat", + "rev": "35bb57c0c8d8b62bbfd284272c928ceb64ddbde9", + "type": "github" + }, + "original": { + "owner": "edolstra", + "repo": "flake-compat", + "type": "github" + } + }, + "flake-utils": { + "locked": { + "lastModified": 1667395993, + "narHash": "sha256-nuEHfE/LcWyuSWnS8t12N1wc105Qtau+/OdUAjtQ0rA=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "5aed5285a952e0b949eb3ba02c12fa4fcfef535f", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "flake-utils", + "type": "github" + } + }, + "flake-utils_2": { + "locked": { + "lastModified": 1667395993, + "narHash": "sha256-nuEHfE/LcWyuSWnS8t12N1wc105Qtau+/OdUAjtQ0rA=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "5aed5285a952e0b949eb3ba02c12fa4fcfef535f", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "flake-utils", + "type": "github" + } + }, + "flake-utils_3": { + "locked": { + "lastModified": 1659877975, + "narHash": "sha256-zllb8aq3YO3h8B/U0/J1WBgAL8EX5yWf5pMj3G0NAmc=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "c0e246b9b83f637f4681389ecabcb2681b4f3af0", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "flake-utils", + "type": "github" + } + }, + "nixpkgs": { + "locked": { + "lastModified": 1675698968, + "narHash": "sha256-tc1o1AalJOCaoFOVPc5ZwBi/Qs7217yp/Jp7o7xSn40=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "0406afa8c713de794aa2fb5d42834ec9b2b1e8e0", + "type": "github" + }, + "original": { + "owner": "NixOS", + "repo": "nixpkgs", + "type": "github" + } + }, + "nixpkgs_2": { + "locked": { + "lastModified": 1665296151, + "narHash": "sha256-uOB0oxqxN9K7XGF1hcnY+PQnlQJ+3bP2vCn/+Ru/bbc=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "14ccaaedd95a488dd7ae142757884d8e125b3363", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixpkgs-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, + "root": { + "inputs": { + "crane": "crane", + "flake-utils": "flake-utils_2", + "nixpkgs": "nixpkgs", + "rust-overlay": "rust-overlay_2" + } + }, + "rust-overlay": { + "inputs": { + "flake-utils": [ + "crane", + "flake-utils" + ], + "nixpkgs": [ + "crane", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1675391458, + "narHash": "sha256-ukDKZw922BnK5ohL9LhwtaDAdCsJL7L6ScNEyF1lO9w=", + "owner": "oxalica", + "repo": "rust-overlay", + "rev": "383a4acfd11d778d5c2efcf28376cbd845eeaedf", + "type": "github" + }, + "original": { + "owner": "oxalica", + "repo": "rust-overlay", + "type": "github" + } + }, + "rust-overlay_2": { + "inputs": { + "flake-utils": "flake-utils_3", + "nixpkgs": "nixpkgs_2" + }, + "locked": { + "lastModified": 1675650300, + "narHash": "sha256-GWnDfrKbQPb+skMMesm1WVu7a3y3GTDpYpfOv3yF4gc=", + "owner": "oxalica", + "repo": "rust-overlay", + "rev": "cccf15dce049eb0be24060ed461c251552b2fb4d", + "type": "github" + }, + "original": { + "owner": "oxalica", + "repo": "rust-overlay", + "type": "github" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/flake.nix b/flake.nix new file mode 100644 index 00000000..cc506704 --- /dev/null +++ b/flake.nix @@ -0,0 +1,44 @@ +{ + description = "serde_divatree"; + + inputs = { + nixpkgs.url = "github:NixOS/nixpkgs"; + flake-utils.url = "github:numtide/flake-utils"; + rust-overlay.url = "github:oxalica/rust-overlay"; + crane = { + url = "github:ipetkov/crane"; + inputs.nixpkgs.follows = "nixpkgs"; + }; + }; + + outputs = { + self, + nixpkgs, + flake-utils, + rust-overlay, + crane, + }: let + in + flake-utils.lib.eachDefaultSystem + (system: let + pkgs = import nixpkgs { + inherit system; + overlays = [ (import rust-overlay) ]; + }; + rustToolchain = pkgs.rust-bin.selectLatestNightlyWith (toolchain: toolchain.default.override { + extensions = [ "rust-src" "rust-analyzer" ]; + }); + craneLib = crane.lib.${system}; + in rec { + devShells.default = with pkgs; pkgs.mkShell rec { + nativeBuildInputs = [ + pkg-config + ]; + buildInputs = [ + rustToolchain + cargo-semver-checks + ]; + }; + } + ); +} diff --git a/src/lib.rs b/src/lib.rs index 82f2d9fc..1d0f3731 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,6 +62,7 @@ use encoding::RawEncoder; use encoding::all::ISO_8859_1; use lazy_static::lazy_static; use regex::Regex; +use std::borrow::Cow; use std::collections::HashMap; use std::convert::From; use std::error::Error; @@ -73,22 +74,67 @@ use std::io::Bytes; use std::io::Read; use std::io::Write; use std::iter::Peekable; +use std::num::ParseIntError; use std::ops::Deref; +use thiserror::Error; ///////////////////// /// The error type for reading and writing properties files. -#[derive(Debug)] +#[derive(Debug, Error)] +#[error("{cause} (line_number = {})", self.display_line_number())] pub struct PropertiesError { - description: String, - cause: Option>, + #[source] + cause: PropertiesErrorCause, line_number: Option, } +/// Underlying errors for [`PropertiesError`] +#[non_exhaustive] +#[derive(Debug, Error)] +pub enum PropertiesErrorCause { + #[error("I/O error")] + IOError(#[from] io::Error), + #[error(transparent)] + EncodingError(#[from] EncodingError), + #[error("Malformed \\uxxxx encoding")] + UnicodeEscapeError(#[from] UnicodeEscapeError), + #[error("Bad key/value separator: {0:?}")] + BadKeyValueSeparator(String), + #[error("Bad comment prefix: {0:?}")] + BadCommentPrefix(String), +} + +/// Errors for escaping unicode code point sequences. +#[non_exhaustive] +#[derive(Debug, Error)] +pub enum UnicodeEscapeError { + #[error("Not enough digits")] + NotEnoughDigits, + #[error("Not hex")] + NotHex(#[from] ParseIntError), + #[error("Invalid character code ({0:X?})")] + InvalidCharacter(u16), +} + +/// Opaque wrapper for encoding errors +#[non_exhaustive] +#[derive(Debug, Error)] +#[error("Error reading {name} encoding: {err}")] +pub struct EncodingError { + name: &'static str, + err: Cow<'static, str>, +} + +impl EncodingError { + fn new(encoding: &dyn Encoding, err: Cow<'static, str>) -> Self { + Self { name: encoding.name(), err } + } +} + impl PropertiesError { - fn new(description: &str, cause: Option>, line_number: Option) -> Self { + fn new(cause: PropertiesErrorCause, line_number: Option) -> Self { PropertiesError { - description: description.to_string(), cause, line_number, } @@ -98,36 +144,15 @@ impl PropertiesError { pub fn line_number(&self) -> Option { self.line_number } -} -impl Error for PropertiesError { - fn description(&self) -> &str { - &self.description - } - - // The "readable" version is less readable, especially since it requires manual type assertions. - #[allow(clippy::manual_map)] - fn source(&self) -> Option<&(dyn Error + 'static)> { - match self.cause { - Some(ref c) => Some(c.deref()), - None => None, - } + fn display_line_number(&self) -> String { + self.line_number.map(|x| x.to_string()).unwrap_or("unknown".to_string()) } } impl From for PropertiesError { fn from(e: io::Error) -> Self { - PropertiesError::new("I/O error", Some(Box::new(e)), None) - } -} - -impl Display for PropertiesError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}", &self.description)?; - match self.line_number { - Some(n) => write!(f, " (line_number = {})", n), - None => write!(f, " (line_number = unknown)"), - } + PropertiesError::new(e.into(), None) } } @@ -157,7 +182,7 @@ impl NaturalLines { fn decode(&self, buf: &[u8]) -> Result { match self.encoding.decode(buf, DecoderTrap::Strict) { Ok(s) => Ok(NaturalLine(self.line_count, s)), - Err(e) => Err(PropertiesError::new(&format!("Error reading {} encoding: {}", self.encoding.name(), e), None, Some(self.line_count))), + Err(e) => Err(PropertiesError::new(EncodingError::new(self.encoding, e).into(), Some(self.line_count))), } } } @@ -187,7 +212,7 @@ impl Iterator for NaturalLines { return Some(self.decode(&buf)); }, Some(Ok(b)) => buf.push(b), - Some(Err(e)) => return Some(Err(PropertiesError::new("I/O error", Some(Box::new(e)), Some(self.line_count + 1)))), + Some(Err(e)) => return Some(Err(PropertiesError::new(e.into(), Some(self.line_count + 1)))), None => { self.eof = true; self.line_count += 1; @@ -379,16 +404,16 @@ fn unescape(s: &str, line_number: usize) -> Result { for _ in 0..4 { match iter.next() { Some(c) => tmp.push(c), - None => return Err(PropertiesError::new("Malformed \\uxxxx encoding: not enough digits.", None, Some(line_number))), + None => return Err(PropertiesError::new(UnicodeEscapeError::NotEnoughDigits.into(), Some(line_number))), } } let val = match u16::from_str_radix(&tmp, 16) { Ok(x) => x, - Err(e) => return Err(PropertiesError::new("Malformed \\uxxxx encoding: not hex.", Some(Box::new(e)), Some(line_number))), + Err(e) => return Err(PropertiesError::new(UnicodeEscapeError::from(e).into(), Some(line_number))), }; match std::char::from_u32(val as u32) { Some(c) => buf.push(c), - None => return Err(PropertiesError::new("Malformed \\uxxxx encoding: invalid character.", None, Some(line_number))), + None => return Err(PropertiesError::new(UnicodeEscapeError::InvalidCharacter(val).into(), Some(line_number))), } }, _ => buf.push(c), @@ -615,7 +640,7 @@ impl PropertiesWriter { self.writer.write_all(&self.comment_prefix)?; match self.encoding.encode(comment, UNICODE_ESCAPE) { Ok(d) => self.writer.write_all(&d)?, - Err(e) => return Err(PropertiesError::new(&format!("Encoding error: {}", e), None, Some(self.lines_written))), + Err(e) => return Err(PropertiesError::new(EncodingError::new(self.encoding, e).into(), Some(self.lines_written))), }; self.write_eol()?; Ok(()) @@ -642,7 +667,7 @@ impl PropertiesWriter { } match self.encoding.encode(&escaped, UNICODE_ESCAPE) { Ok(d) => self.writer.write_all(&d)?, - Err(e) => return Err(PropertiesError::new(&format!("Encoding error: {}", e), None, Some(self.lines_written))), + Err(e) => return Err(PropertiesError::new(EncodingError::new(self.encoding, e).into(), Some(self.lines_written))), }; Ok(()) } @@ -671,11 +696,11 @@ impl PropertiesWriter { static ref RE: Regex = Regex::new(r"^[ \t\x0c]*[#!][^\r\n]*$").unwrap(); } if !RE.is_match(prefix) { - return Err(PropertiesError::new(&format!("Bad comment prefix: {:?}", prefix), None, None)); + return Err(PropertiesError::new(PropertiesErrorCause::BadCommentPrefix(prefix.into()), None)); } match self.encoding.encode(prefix, UNICODE_ESCAPE) { Ok(bytes) => self.comment_prefix = bytes, - Err(e) => return Err(PropertiesError::new(&format!("Encoding error: {}", e), None, None)), + Err(e) => return Err(PropertiesError::new(EncodingError::new(self.encoding, e).into(), None)), }; Ok(()) } @@ -689,11 +714,11 @@ impl PropertiesWriter { static ref RE: Regex = Regex::new(r"^([ \t\x0c]*[:=][ \t\x0c]*|[ \t\x0c]+)$").unwrap(); } if !RE.is_match(separator) { - return Err(PropertiesError::new(&format!("Bad key/value separator: {:?}", separator), None, None)); + return Err(PropertiesError::new(PropertiesErrorCause::BadKeyValueSeparator(separator.into()), None)); } match self.encoding.encode(separator, UNICODE_ESCAPE) { Ok(bytes) => self.kv_separator = bytes, - Err(e) => return Err(PropertiesError::new(&format!("Encoding error: {}", e), None, None)), + Err(e) => return Err(PropertiesError::new(EncodingError::new(self.encoding, e).into(), None)), }; Ok(()) } @@ -1217,8 +1242,8 @@ mod tests { #[test] fn properties_error_display() { - assert_eq!(format!("{}", PropertiesError::new("foo", None, None)), "foo (line_number = unknown)"); - assert_eq!(format!("{}", PropertiesError::new("foo", None, Some(1))), "foo (line_number = 1)"); + assert_eq!(PropertiesError::new(crate::PropertiesErrorCause::BadCommentPrefix("".into()), None).to_string(), "Bad comment prefix: \"\" (line_number = unknown)"); + assert_eq!(PropertiesError::new(crate::PropertiesErrorCause::BadCommentPrefix("".into()), Some(1)).to_string(), "Bad comment prefix: \"\" (line_number = 1)"); } #[test]