From 93bd134bb72b04dc561c1e51585866b72d542c0a Mon Sep 17 00:00:00 2001 From: Stepan Henek Date: Sat, 23 May 2020 00:26:46 +0200 Subject: [PATCH] feature: improved error handling --- streamson-bin/src/main.rs | 2 +- streamson-lib/src/collector.rs | 6 +- streamson-lib/src/error.rs | 112 +++++++++++++++++++++++++-- streamson-lib/src/handler.rs | 2 +- streamson-lib/src/handler/buffer.rs | 2 +- streamson-lib/src/handler/file.rs | 19 +++-- streamson-lib/src/handler/println.rs | 4 +- streamson-lib/src/matcher/simple.rs | 75 +++++++++++++++++- streamson-lib/src/path.rs | 77 ++++++++++-------- 9 files changed, 237 insertions(+), 62 deletions(-) diff --git a/streamson-bin/src/main.rs b/streamson-bin/src/main.rs index 2b41548..e022e35 100644 --- a/streamson-bin/src/main.rs +++ b/streamson-bin/src/main.rs @@ -16,7 +16,7 @@ fn write_file_validator(input: String) -> Result<(), String> { } } -fn main() -> Result<(), error::Generic> { +fn main() -> Result<(), error::General> { let app = App::new(crate_name!()) .author(crate_authors!()) .version(crate_version!()) diff --git a/streamson-lib/src/collector.rs b/streamson-lib/src/collector.rs index 1401a85..0e1d239 100644 --- a/streamson-lib/src/collector.rs +++ b/streamson-lib/src/collector.rs @@ -98,12 +98,12 @@ impl Collector { /// # Errors /// /// If parsing logic finds that JSON is not valid, - /// it returns `error::Generic`. + /// it returns `error::General`. /// /// Note that streamson assumes that its input is a valid /// JSONs and if not. It still might be splitted without an error. /// This is caused because streamson does not validate JSON. - pub fn process(&mut self, input: &[u8]) -> Result { + pub fn process(&mut self, input: &[u8]) -> Result { self.emitter.feed(input); let mut inner_idx = 0; loop { @@ -187,7 +187,7 @@ mod tests { } impl Handler for TestHandler { - fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Generic> { + fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Handler> { self.paths.push(path.to_string()); self.data.push(Bytes::from(data.to_vec())); Ok(()) diff --git a/streamson-lib/src/error.rs b/streamson-lib/src/error.rs index b009a09..c656cc4 100644 --- a/streamson-lib/src/error.rs +++ b/streamson-lib/src/error.rs @@ -1,17 +1,113 @@ //! Module containing errors -use std::{error::Error, fmt}; +use std::{error::Error, fmt, str::Utf8Error}; -/// Generic Error -/// -/// Currently the only error kind is used +/// Matcher related errors #[derive(Debug, PartialEq, Clone)] -pub struct Generic; +pub enum Matcher { + Parse(String), +} -impl Error for Generic {} +impl Error for Matcher {} -impl fmt::Display for Generic { +impl fmt::Display for Matcher { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "GenericError") + match self { + Self::Parse(input) => write!(f, "Failed to parse matcher'{}", input), + } + } +} + +/// Handler related errors +#[derive(Debug, PartialEq, Clone)] +pub struct Handler { + reason: String, +} + +impl Handler { + pub fn new(reason: T) -> Self + where + T: ToString, + { + Self { + reason: reason.to_string(), + } + } +} + +impl Error for Handler {} + +impl fmt::Display for Handler { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Handler failed - {}", self.reason) + } +} + +/// Incorrect input error +#[derive(Debug, PartialEq, Clone)] +pub struct IncorrectInput { + chr: char, + idx: usize, +} + +impl IncorrectInput { + pub fn new(chr: char, idx: usize) -> Self { + Self { chr, idx } + } +} + +impl Error for IncorrectInput {} + +impl fmt::Display for IncorrectInput { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "Incorrect input (character '{}' on idx {})", + self.chr, self.idx + ) + } +} +/// Handler related errors +#[derive(Debug, PartialEq, Clone)] +pub enum General { + Handler(Handler), + Matcher(Matcher), + Utf8Error(Utf8Error), + IncorrectInput(IncorrectInput), +} + +impl Error for General {} +impl fmt::Display for General { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::Handler(err) => err.fmt(f), + Self::Matcher(err) => err.fmt(f), + Self::Utf8Error(err) => err.fmt(f), + Self::IncorrectInput(err) => err.fmt(f), + } + } +} + +impl From for General { + fn from(handler: Handler) -> Self { + General::Handler(handler) + } +} + +impl From for General { + fn from(matcher: Matcher) -> Self { + General::Matcher(matcher) + } +} + +impl From for General { + fn from(utf8: Utf8Error) -> Self { + General::Utf8Error(utf8) + } +} + +impl From for General { + fn from(incorrect_input: IncorrectInput) -> Self { + General::IncorrectInput(incorrect_input) } } diff --git a/streamson-lib/src/handler.rs b/streamson-lib/src/handler.rs index 0bdb6ed..740d237 100644 --- a/streamson-lib/src/handler.rs +++ b/streamson-lib/src/handler.rs @@ -25,7 +25,7 @@ pub trait Handler { /// # Errors /// /// Handler failed (e.g. failed to write to output file). - fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Generic>; + fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Handler>; /// Should path be displayed in the output fn show_path(&self) -> bool { diff --git a/streamson-lib/src/handler/buffer.rs b/streamson-lib/src/handler/buffer.rs index 4da8bd2..00c2c73 100644 --- a/streamson-lib/src/handler/buffer.rs +++ b/streamson-lib/src/handler/buffer.rs @@ -40,7 +40,7 @@ pub struct Buffer { } impl Handler for Buffer { - fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Generic> { + fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Handler> { // TODO we may limit the max VecDeque size and raise // an error when reached diff --git a/streamson-lib/src/handler/file.rs b/streamson-lib/src/handler/file.rs index 45498f4..0aef819 100644 --- a/streamson-lib/src/handler/file.rs +++ b/streamson-lib/src/handler/file.rs @@ -2,10 +2,7 @@ use super::Handler; use crate::error; -use std::{ - fs, - io::{self, Write}, -}; +use std::{fs, io::Write}; /// File handler responsible for storing data to a file. pub struct File { @@ -34,8 +31,8 @@ impl File { /// # Errors /// /// Error might occur when the file fails to open - pub fn new(fs_path: &str) -> io::Result { - let file = fs::File::create(fs_path)?; + pub fn new(fs_path: &str) -> Result { + let file = fs::File::create(fs_path).map_err(|err| error::Handler::new(err.to_string()))?; Ok(Self { file, show_path: false, @@ -92,17 +89,19 @@ impl Handler for File { &self.separator } - fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Generic> { + fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Handler> { if self.show_path { self.file .write(format!("{}: ", path).as_bytes()) - .map_err(|_| error::Generic)?; + .map_err(|err| error::Handler::new(err.to_string()))?; } - self.file.write(data).map_err(|_| error::Generic)?; + self.file + .write(data) + .map_err(|err| error::Handler::new(err.to_string()))?; let separator = self.separator().to_string(); self.file .write(separator.as_bytes()) - .map_err(|_| error::Generic)?; + .map_err(|err| error::Handler::new(err.to_string()))?; Ok(()) } } diff --git a/streamson-lib/src/handler/println.rs b/streamson-lib/src/handler/println.rs index 7800001..45c23b4 100644 --- a/streamson-lib/src/handler/println.rs +++ b/streamson-lib/src/handler/println.rs @@ -76,8 +76,8 @@ impl Handler for PrintLn { &self.separator } - fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Generic> { - let str_data = str::from_utf8(data).map_err(|_| error::Generic)?; + fn handle(&mut self, path: &str, data: &[u8]) -> Result<(), error::Handler> { + let str_data = str::from_utf8(data).map_err(|err| error::Handler::new(err.to_string()))?; if self.show_path() { print!("{}: {}{}", path, str_data, self.separator()); } else { diff --git a/streamson-lib/src/matcher/simple.rs b/streamson-lib/src/matcher/simple.rs index 56b7701..7e0f613 100644 --- a/streamson-lib/src/matcher/simple.rs +++ b/streamson-lib/src/matcher/simple.rs @@ -14,7 +14,7 @@ pub struct Simple { path_expr: String, } -#[derive(Debug)] +#[derive(Debug, PartialEq)] enum SimpleMatcherStates { Normal, Array, @@ -181,9 +181,9 @@ impl MatchMaker for Simple { } impl FromStr for Simple { - type Err = error::Generic; + type Err = error::Matcher; fn from_str(s: &str) -> Result { - // TODO error handling + Simple::is_valid(s)?; Ok(Self { path_expr: s.into(), }) @@ -203,6 +203,59 @@ impl Simple { path_expr: path_expr.to_string(), } } + + /// Check whether the path is valid + fn is_valid(path: &str) -> Result<(), error::Matcher> { + let mut state = SimpleMatcherStates::Normal; + for chr in path.chars() { + state = match state { + SimpleMatcherStates::Normal => match chr { + '[' => SimpleMatcherStates::Array, + '{' => SimpleMatcherStates::Object, + _ => { + return Err(error::Matcher::Parse(path.to_string())); + } + }, + SimpleMatcherStates::Array => match chr { + ']' => SimpleMatcherStates::Normal, + '0'..='9' => SimpleMatcherStates::ArrayCmp, + _ => { + return Err(error::Matcher::Parse(path.to_string())); + } + }, + SimpleMatcherStates::ArrayCmp => match chr { + ']' => SimpleMatcherStates::Normal, + '0'..='9' => SimpleMatcherStates::ArrayCmp, + _ => { + return Err(error::Matcher::Parse(path.to_string())); + } + }, + SimpleMatcherStates::Object => match chr { + '}' => SimpleMatcherStates::Normal, + '"' => SimpleMatcherStates::ObjectCmp, + _ => { + return Err(error::Matcher::Parse(path.to_string())); + } + }, + SimpleMatcherStates::ObjectCmp => match chr { + '"' => SimpleMatcherStates::ObjectCmpEnd, + _ => SimpleMatcherStates::ObjectCmp, + }, + SimpleMatcherStates::ObjectCmpEnd => match chr { + '}' => SimpleMatcherStates::Normal, + _ => { + return Err(error::Matcher::Parse(path.to_string())); + } + }, + _ => unreachable!(), + } + } + if state == SimpleMatcherStates::Normal { + Ok(()) + } else { + Err(error::Matcher::Parse(path.to_string())) + } + } } #[cfg(test)] @@ -255,4 +308,20 @@ mod tests { assert!(simple.match_path(r#"{"People"}[0]{"O\"ll"}"#)); assert!(simple.match_path(r#"{"People"}[0]{"O\\\"ll"}"#)); } + + #[test] + fn simple_parse() { + assert!(Simple::from_str(r#""#).is_ok()); + assert!(Simple::from_str(r#"{}"#).is_ok()); + assert!(Simple::from_str(r#"{}[3]"#).is_ok()); + assert!(Simple::from_str(r#"{"xx"}[]"#).is_ok()); + assert!(Simple::from_str(r#"{}[]"#).is_ok()); + } + + #[test] + fn simple_parse_error() { + assert!(Simple::from_str(r#"{"People""#).is_err()); + assert!(Simple::from_str(r#"[}"#).is_err()); + assert!(Simple::from_str(r#"{"People}"#).is_err()); + } } diff --git a/streamson-lib/src/path.rs b/streamson-lib/src/path.rs index 74d98a9..0a78c94 100644 --- a/streamson-lib/src/path.rs +++ b/streamson-lib/src/path.rs @@ -132,11 +132,10 @@ impl Emitter { } /// Checks which utf8 character is currently process - fn peek(&self) -> Result, error::Generic> { + fn peek(&self) -> Result, error::General> { let get_char = |size: usize| { if self.pending.len() >= self.pending_idx + size { - let decoded = from_utf8(&self.pending[self.pending_idx..self.pending_idx + size]) - .map_err(|_| error::Generic)?; + let decoded = from_utf8(&self.pending[self.pending_idx..self.pending_idx + size])?; Ok(decoded.chars().next()) // Should only return only a single character } else { Ok(None) @@ -148,9 +147,9 @@ impl Emitter { 0b1100_0000..=0b1101_1111 => get_char(2), 0b1110_0000..=0b1110_1111 => get_char(3), 0b1111_0000..=0b1111_0111 => get_char(4), - _ => { + b => { // input is not UTF8 - Err(error::Generic) + Err(from_utf8(&[b]).unwrap_err().into()) } } } else { @@ -164,7 +163,7 @@ impl Emitter { /// * `Err(_)` -> wrong utf-8 /// * Ok(None) -> need more data /// * Ok(usize) -> read X characters (1-4) - fn forward(&mut self) -> Result, error::Generic> { + fn forward(&mut self) -> Result, error::General> { if let Some(chr) = self.peek()? { let len = chr.len_utf8(); self.pending_idx += len; @@ -189,7 +188,7 @@ impl Emitter { } /// Moves cursor forward while characters are namespace - fn remove_whitespaces(&mut self) -> Result, error::Generic> { + fn remove_whitespaces(&mut self) -> Result, error::General> { let mut size = 0; while let Some(chr) = self.peek()? { if !chr.is_ascii_whitespace() { @@ -201,7 +200,7 @@ impl Emitter { Ok(None) } - fn process_remove_whitespace(&mut self) -> Result, error::Generic> { + fn process_remove_whitespace(&mut self) -> Result, error::General> { if self.remove_whitespaces()?.is_none() { self.states.push(States::RemoveWhitespaces); return Ok(Some(Output::Pending)); @@ -209,7 +208,7 @@ impl Emitter { Ok(None) } - fn process_value(&mut self, element: Element) -> Result, error::Generic> { + fn process_value(&mut self, element: Element) -> Result, error::General> { if let Some(chr) = self.peek()? { match chr { '"' => { @@ -261,7 +260,9 @@ impl Emitter { // End of an array or object -> no value matched Ok(None) } - _ => Err(error::Generic), + chr => { + Err(error::IncorrectInput::new(chr, self.total_idx + self.pending_idx).into()) + } } } else { self.states.push(States::Value(element)); @@ -269,7 +270,7 @@ impl Emitter { } } - fn process_str(&mut self, state: StringState) -> Result, error::Generic> { + fn process_str(&mut self, state: StringState) -> Result, error::General> { if let Some(chr) = self.peek()? { match chr { '"' => { @@ -277,7 +278,7 @@ impl Emitter { self.forward()?; self.advance(); let prev_path = self.display_path(); - self.path.pop().ok_or_else(|| error::Generic)?; + self.path.pop().unwrap(); Ok(Some(Output::End(self.total_idx, prev_path))) } else { self.forward()?; @@ -306,7 +307,7 @@ impl Emitter { } } - fn process_number(&mut self) -> Result, error::Generic> { + fn process_number(&mut self) -> Result, error::General> { if let Some(chr) = self.peek()? { if chr.is_digit(10) || chr == '.' { self.forward()?; @@ -315,7 +316,7 @@ impl Emitter { } else { self.advance(); let prev_path = self.display_path(); - self.path.pop().ok_or_else(|| error::Generic)?; + self.path.pop().unwrap(); Ok(Some(Output::End(self.total_idx, prev_path))) } } else { @@ -324,7 +325,7 @@ impl Emitter { } } - fn process_bool(&mut self) -> Result, error::Generic> { + fn process_bool(&mut self) -> Result, error::General> { if let Some(chr) = self.peek()? { if chr.is_alphabetic() { self.forward()?; @@ -333,7 +334,7 @@ impl Emitter { } else { self.advance(); let prev_path = self.display_path(); - self.path.pop().ok_or_else(|| error::Generic)?; + self.path.pop().unwrap(); Ok(Some(Output::End(self.total_idx, prev_path))) } } else { @@ -342,7 +343,7 @@ impl Emitter { } } - fn process_null(&mut self) -> Result, error::Generic> { + fn process_null(&mut self) -> Result, error::General> { if let Some(chr) = self.peek()? { if chr.is_alphabetic() { self.forward()?; @@ -351,7 +352,7 @@ impl Emitter { } else { self.advance(); let prev_path = self.display_path(); - self.path.pop().ok_or_else(|| error::Generic)?; + self.path.pop().unwrap(); Ok(Some(Output::End(self.total_idx, prev_path))) } } else { @@ -360,14 +361,14 @@ impl Emitter { } } - fn process_array(&mut self, idx: usize) -> Result, error::Generic> { + fn process_array(&mut self, idx: usize) -> Result, error::General> { if let Some(chr) = self.peek()? { match chr { ']' => { self.forward()?; self.advance(); let exported_path = self.display_path(); - self.path.pop().ok_or(error::Generic)?; + self.path.pop().unwrap(); Ok(Some(Output::End(self.total_idx, exported_path))) } ',' => { @@ -378,7 +379,9 @@ impl Emitter { self.states.push(States::RemoveWhitespaces); Ok(None) } - _ => Err(error::Generic), + chr => { + Err(error::IncorrectInput::new(chr, self.total_idx + self.pending_idx).into()) + } } } else { self.states.push(States::Array(idx)); @@ -386,14 +389,14 @@ impl Emitter { } } - fn process_object(&mut self) -> Result, error::Generic> { + fn process_object(&mut self) -> Result, error::General> { if let Some(chr) = self.peek()? { match chr { '}' => { self.forward()?; self.advance(); let exported_path = self.display_path(); - self.path.pop().ok_or_else(|| error::Generic)?; + self.path.pop().unwrap(); Ok(Some(Output::End(self.total_idx, exported_path))) } ',' => { @@ -404,7 +407,9 @@ impl Emitter { self.states.push(States::RemoveWhitespaces); Ok(None) } - _ => Err(error::Generic), + chr => { + Err(error::IncorrectInput::new(chr, self.total_idx + self.pending_idx).into()) + } } } else { self.states.push(States::Object); @@ -415,7 +420,7 @@ impl Emitter { fn process_object_key( &mut self, state: ObjectKeyState, - ) -> Result, error::Generic> { + ) -> Result, error::General> { match state { ObjectKeyState::Init => { if let Some(chr) = self.peek()? { @@ -428,8 +433,13 @@ impl Emitter { ))); Ok(None) } - '}' => Ok(None), // end has been reached to Object - _ => Err(error::Generic), // keys are strings in JSON + '}' => Ok(None), // end has been reached to Object + + chr => Err(error::IncorrectInput::new( + chr, + self.total_idx + self.pending_idx, + ) + .into()), // keys are strings in JSON } } else { self.states.push(States::ObjectKey(state)); @@ -442,9 +452,8 @@ impl Emitter { match string_state { StringState::Normal => match chr { '\"' => { - let key = from_utf8(&self.pending[1..self.pending_idx - 1]) - .map_err(|_| error::Generic)? - .to_string(); + let key = + from_utf8(&self.pending[1..self.pending_idx - 1])?.to_string(); self.states.push(States::Value(Element::Key(key))); self.states.push(States::RemoveWhitespaces); self.states.push(States::Colon); @@ -480,10 +489,12 @@ impl Emitter { } } - fn process_colon(&mut self) -> Result, error::Generic> { + fn process_colon(&mut self) -> Result, error::General> { if let Some(chr) = self.peek()? { if chr != ':' { - return Err(error::Generic); + return Err( + error::IncorrectInput::new(chr, self.total_idx + self.pending_idx).into(), + ); } self.forward()?; Ok(None) @@ -499,7 +510,7 @@ impl Emitter { /// /// If invalid JSON is passed and error may be emitted. /// Note that validity of input JSON is not checked. - pub fn read(&mut self) -> Result { + pub fn read(&mut self) -> Result { while let Some(state) = self.states.pop() { match state { States::RemoveWhitespaces => {