From 1fbb0aec79df9e001edf38835ff988331808b3b8 Mon Sep 17 00:00:00 2001 From: Nico Wagner Date: Fri, 7 Jul 2023 10:27:58 +0200 Subject: [PATCH] Print more helpful error message on `ParsePicaError` (#643) --- CHANGELOG.md | 4 ++++ pica-record/src/io/mod.rs | 21 +++++++++++++--- pica-record/src/io/reader.rs | 41 ++++++++++++++++++++++++++------ src/bin/pica/commands/invalid.rs | 7 +++--- src/bin/pica/util.rs | 9 +++---- 5 files changed, 65 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aee25389d..6955d5032 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * #641 Stabilize `sample` command * #642 Add `--squash` and `--merge` option +### Changed + +* #643 Print more helpful error message on `ParsePicaError` + ### Removed * #639 Remove `xml` command diff --git a/pica-record/src/io/mod.rs b/pica-record/src/io/mod.rs index 287d65433..d0ea18c56 100644 --- a/pica-record/src/io/mod.rs +++ b/pica-record/src/io/mod.rs @@ -23,8 +23,8 @@ pub use writer::{ /// [BufReader](std::io::BufReader). #[derive(Error, Debug)] pub enum ReadPicaError { - #[error("parse error")] - Parse(#[from] ParsePicaError), + #[error("parse error: {msg:?}")] + Parse { msg: String, err: ParsePicaError }, #[error("io error")] Io(#[from] io::Error), @@ -34,7 +34,22 @@ impl ReadPicaError { /// Returns true, if the underlying error was caused by parsing an /// invalid record. pub fn is_invalid_record(&self) -> bool { - matches!(self, Self::Parse(ParsePicaError::InvalidRecord(_))) + matches!( + self, + Self::Parse { + msg: _, + err: ParsePicaError::InvalidRecord(_) + } + ) + } +} + +impl From for ReadPicaError { + fn from(err: ParsePicaError) -> Self { + Self::Parse { + msg: "invalid record".into(), + err, + } } } diff --git a/pica-record/src/io/reader.rs b/pica-record/src/io/reader.rs index 1be9ef7f1..233058993 100644 --- a/pica-record/src/io/reader.rs +++ b/pica-record/src/io/reader.rs @@ -34,7 +34,7 @@ impl ReaderBuilder { /// let data = /// Cursor::new(b"003@ \x1f0abc\x1e\n003@ \x1f0def\x1e\n"); /// let mut reader = - /// ReaderBuilder::new().limit(1).from_reader(data); + /// ReaderBuilder::new().limit(1).from_reader(data, None); /// /// let mut count = 0; /// while let Some(result) = reader.next() { @@ -60,7 +60,7 @@ impl ReaderBuilder { /// fn example() -> anyhow::Result<()> { /// let data = /// Cursor::new(b"003@ \x1f0abc\x1e\n003@ \x1f0def\x1e\n"); - /// let mut reader = ReaderBuilder::new().from_reader(data); + /// let mut reader = ReaderBuilder::new().from_reader(data, None); /// /// let mut count = 0; /// while let Some(result) = reader.next() { @@ -72,8 +72,12 @@ impl ReaderBuilder { /// Ok(()) /// } /// ``` - pub fn from_reader(&self, reader: R) -> Reader { - Reader::new(self, reader) + pub fn from_reader( + &self, + reader: R, + source: Option, + ) -> Reader { + Reader::new(self, reader, source) } pub fn from_path>( @@ -81,6 +85,7 @@ impl ReaderBuilder { path: P, ) -> io::Result>> { let path = path.as_ref(); + let source = path.to_string_lossy().to_string(); let reader: Box = match path .extension() @@ -96,22 +101,28 @@ impl ReaderBuilder { } }; - Ok(self.from_reader(reader)) + Ok(self.from_reader(reader, Some(source))) } } pub struct Reader { inner: BufReader, + source: Option, limit: usize, count: usize, buf: Vec, } impl Reader { - pub fn new(builder: &ReaderBuilder, reader: R) -> Self { + pub fn new( + builder: &ReaderBuilder, + reader: R, + source: Option, + ) -> Self { Self { inner: BufReader::new(reader), limit: builder.limit, + source, buf: vec![], count: 0, } @@ -145,7 +156,23 @@ impl RecordsIterator for Reader { Ok(_) => { let result = ByteRecord::from_bytes(&self.buf); match result { - Err(e) => Some(Err(ReadPicaError::from(e))), + Err(err) => { + let msg = match &self.source { + Some(source) => { + if source == "-" { + format!("invalid record in line {} (stdin)", self.count) + } else { + format!("invalid record in line {} ({})", self.count, source) + } + } + None => format!( + "invalid record on line {}", + self.count + ), + }; + + Some(Err(ReadPicaError::Parse { msg, err })) + } Ok(record) => { self.count += 1; Some(Ok(record)) diff --git a/src/bin/pica/commands/invalid.rs b/src/bin/pica/commands/invalid.rs index 59d49db30..d92746adc 100644 --- a/src/bin/pica/commands/invalid.rs +++ b/src/bin/pica/commands/invalid.rs @@ -34,9 +34,10 @@ impl Invalid { while let Some(result) = reader.next() { match result { - Err(ReadPicaError::Parse( - ParsePicaError::InvalidRecord(data), - )) => { + Err(ReadPicaError::Parse { + msg: _, + err: ParsePicaError::InvalidRecord(data), + }) => { writer.write_all(&data)?; } Err(e) => return Err(e.into()), diff --git a/src/bin/pica/util.rs b/src/bin/pica/util.rs index 6eb312586..a709a82df 100644 --- a/src/bin/pica/util.rs +++ b/src/bin/pica/util.rs @@ -8,7 +8,7 @@ pub(crate) enum CliError { Io(io::Error), Csv(csv::Error), Pica(pica::Error), - ParsePica(pica_record::ParsePicaError), + ParsePica(String), ParsePath(pica_path::ParsePathError), ParseMatcher(pica_matcher::ParseMatcherError), ParseQuery(pica_select::ParseQueryError), @@ -52,9 +52,10 @@ impl From for CliError { fn from(err: pica_record::io::ReadPicaError) -> Self { match err { pica_record::io::ReadPicaError::Io(e) => CliError::Io(e), - pica_record::io::ReadPicaError::Parse(e) => { - CliError::ParsePica(e) - } + pica_record::io::ReadPicaError::Parse { + msg: m, + err: _, + } => CliError::ParsePica(m), } } }