[Enhancement] Writer error handling (#496)

Remove panic, unreachable, etc from writer and replace with internal errors

guard-examples/library/src/main.rs

@@ -24,7 +24,10 @@ fn main() -> anyhow::Result<()> {
     let mut reader = Reader::new(ReadBuffer::Cursor(Cursor::new(Vec::from(
         payload.as_bytes(),
     ))));
-    let mut writer = Writer::new_with_err(WriteBuffer::Vec(vec![]), WriteBuffer::Vec(vec![]));
+    let mut writer = Writer::new_with_err(WriteBuffer::Vec(vec![]), WriteBuffer::Vec(vec![]))
+        .unwrap_or_else(|err| {
+            panic!("Error: {}", err);
+        });
 
     let cmd = ValidateBuilder::default()
         .payload(true)

guard/src/main.rs

@@ -15,14 +15,18 @@ fn main() -> Result<(), Error> {
 
     let mut writer = match &args.command {
         Commands::ParseTree(cmd) => match &cmd.output {
-            Some(path) => Writer::new(WBFile(File::create(path)?)),
-            None => Writer::new(Stdout(std::io::stdout())),
+            Some(path) => {
+                Writer::new(WBFile(File::create(path)?)).expect("Failed to create writer.")
+            }
+            None => Writer::new(Stdout(std::io::stdout())).expect("Failed to create writer."),
         },
         Commands::Rulegen(cmd) => match &cmd.output {
-            Some(path) => Writer::new(WBFile(File::create(path)?)),
-            None => Writer::new(Stdout(std::io::stdout())),
+            Some(path) => {
+                Writer::new(WBFile(File::create(path)?)).expect("Failed to create writer.")
+            }
+            None => Writer::new(Stdout(std::io::stdout())).expect("Failed to create writer."),
         },
-        _ => Writer::new(Stdout(std::io::stdout())),
+        _ => Writer::new(Stdout(std::io::stdout())).expect("Failed to create writer."),
     };
 
     let mut reader = Reader::new(ReadBuffer::Stdin(std::io::stdin()));

guard/src/rules/errors.rs

@@ -1,5 +1,6 @@
 use std::convert::Infallible;
 use std::fmt::{Debug, Formatter};
+use std::string::FromUtf8Error;
 use thiserror::Error;
 
 use crate::rules::parser::{ParserError, Span};
@@ -48,7 +49,7 @@ pub enum Error {
     #[error("Error occurred while attempting to write junit report")]
     XMLError(#[from] quick_xml::Error),
     #[error("{0}")]
-    InternalError(InternalError),
+    InternalError(#[from] InternalError),
 }
 
 #[derive(Debug, Error)]
@@ -57,6 +58,14 @@ pub enum InternalError {
     InvalidKeyType(String),
     #[error("internal error {0}")]
     UnresolvedKeyForReporter(String),
+    #[error("{0}")]
+    FromUtf8Error(#[from] FromUtf8Error),
+    #[error("{0}")]
+    IncompatibleWriterError(String),
+    #[error("{0}")]
+    UnsupportedBufferError(String),
+    #[error("{0}")]
+    UnsupportedOperationError(String),
 }
 
 #[derive(Debug, Error)]

guard/src/utils/writer.rs

@@ -1,6 +1,9 @@
+use crate::rules::errors::InternalError::{
+    FromUtf8Error, IncompatibleWriterError, UnsupportedBufferError, UnsupportedOperationError,
+};
+use crate::Error;
 use std::fs::File;
 use std::io::{Read, Stderr, Stdout, Write};
-use std::string::FromUtf8Error;
 
 #[derive(Debug)]
 pub struct Writer {
@@ -18,58 +21,80 @@ impl Default for Writer {
 }
 
 impl Writer {
-    pub fn new(buffer: WriteBuffer) -> Self {
-        if let WriteBuffer::Stderr(_) = buffer {
-            panic!("unable to use stderr as regular buffer");
+    pub fn new(buffer: WriteBuffer) -> crate::rules::Result<Self> {
+        if buffer.is_err() {
+            return Err(Error::from(IncompatibleWriterError(
+                "Unable to use stderr as a regular buffer.".to_string(),
+            )));
         }
 
-        Self {
+        Ok(Self {
             buffer,
             err: WriteBuffer::Stderr(std::io::stderr()),
-        }
+        })
     }
 
-    pub fn new_with_err(buffer: WriteBuffer, err: WriteBuffer) -> Self {
-        if let WriteBuffer::Stderr(_) = buffer {
-            panic!("unable to use stderr as regular buffer");
+    pub fn new_with_err(buffer: WriteBuffer, err: WriteBuffer) -> crate::rules::Result<Self> {
+        if buffer.is_err() {
+            return Err(Error::from(IncompatibleWriterError(
+                "Unable to use stderr as a regular buffer.".to_string(),
+            )));
         }
 
-        Self { buffer, err }
+        Ok(Self { buffer, err })
     }
 
     pub fn write_err(&mut self, s: String) -> std::io::Result<()> {
         writeln!(self.err, "{s}")
     }
 
-    pub fn err_to_stripped(self) -> Result<String, FromUtf8Error> {
+    pub fn err_to_stripped(self) -> crate::rules::Result<String> {
         match self.err {
-            WriteBuffer::Vec(vec) => String::from_utf8(strip_ansi_escapes::strip(vec).unwrap()),
+            WriteBuffer::Vec(vec) => String::from_utf8(strip_ansi_escapes::strip(vec)?)
+                .map_err(|e| Error::from(FromUtf8Error(e))),
             WriteBuffer::File(mut file) => {
                 let mut data = String::new();
                 file.read_to_string(&mut data)
                     .expect("Unable to read from file");
 
-                String::from_utf8(strip_ansi_escapes::strip(data).unwrap())
+                String::from_utf8(strip_ansi_escapes::strip(data)?)
+                    .map_err(|e| Error::from(FromUtf8Error(e)))
             }
-            _ => unreachable!(),
+            WriteBuffer::Stdout(..) => Err(Error::from(UnsupportedOperationError(
+                "Unable to call err_to_stripped() on a stdout buffer.".to_string(),
+            ))),
+            WriteBuffer::Stderr(..) => Err(Error::from(UnsupportedOperationError(
+                "Unable to call err_to_stripped() on a stderr buffer.".to_string(),
+            ))),
         }
     }
 
-    pub fn into_string(self) -> Result<String, FromUtf8Error> {
+    pub fn into_string(self) -> crate::rules::Result<String> {
         self.buffer.into_string()
     }
 
-    pub fn stripped(self) -> Result<String, FromUtf8Error> {
+    pub fn stripped(self) -> crate::rules::Result<String> {
         match self.buffer {
-            WriteBuffer::Vec(vec) => String::from_utf8(strip_ansi_escapes::strip(vec).unwrap()),
+            WriteBuffer::Vec(vec) => {
+                let stripped = strip_ansi_escapes::strip(vec)?;
+
+                String::from_utf8(stripped).map_err(|e| Error::from(FromUtf8Error(e)))
+            }
             WriteBuffer::File(mut file) => {
                 let mut data = String::new();
                 file.read_to_string(&mut data)
                     .expect("Unable to read from file");
 
-                String::from_utf8(strip_ansi_escapes::strip(data).unwrap())
+                let stripped = strip_ansi_escapes::strip(data.into_bytes())?;
+
+                String::from_utf8(stripped).map_err(|e| Error::from(FromUtf8Error(e)))
             }
-            _ => unreachable!(),
+            WriteBuffer::Stdout(..) => Err(Error::from(UnsupportedBufferError(
+                "Unable to strip ANSI escapes from stdout buffer.".to_string(),
+            ))),
+            WriteBuffer::Stderr(..) => Err(Error::from(UnsupportedBufferError(
+                "Unable to strip ANSI escapes from stderr buffer.".to_string(),
+            ))),
         }
     }
 }
@@ -93,15 +118,25 @@ pub enum WriteBuffer {
 }
 
 impl WriteBuffer {
-    fn into_string(self) -> Result<String, FromUtf8Error> {
+    fn is_err(&self) -> bool {
+        matches!(self, WriteBuffer::Stderr(_))
+    }
+    fn into_string(self) -> crate::rules::Result<String> {
         match self {
-            WriteBuffer::Stdout(..) => unimplemented!(),
-            WriteBuffer::Stderr(..) => unimplemented!(),
-            WriteBuffer::Vec(vec) => String::from_utf8(vec),
+            WriteBuffer::Stdout(..) => Err(Error::from(UnsupportedOperationError(
+                "Unable to call into_string() on a stdout buffer.".to_string(),
+            ))),
+            WriteBuffer::Stderr(..) => Err(Error::from(UnsupportedOperationError(
+                "Unable to call into_string() on a stderr buffer.".to_string(),
+            ))),
+            WriteBuffer::Vec(vec) => {
+                String::from_utf8(vec).map_err(|e| Error::from(FromUtf8Error(e)))
+            }
             WriteBuffer::File(mut file) => {
                 let mut data = String::new();
                 file.read_to_string(&mut data)
                     .expect("Unable to read from file");
+
                 Ok(data)
             }
         }

guard/tests/parse_tree.rs

@@ -72,7 +72,7 @@ mod parse_tree_tests {
     #[test]
     fn test_json_output() {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = ParseTreeTestRunner::default()
             .print_json()
             .rules("validate/rules-dir/s3_bucket_server_side_encryption_enabled.guard")
@@ -111,7 +111,7 @@ mod parse_tree_tests {
         #[case] expected_status_code: i32,
     ) {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = ParseTreeTestRunner::default()
             .rules(rules_arg)
             .run(&mut writer, &mut reader);
@@ -131,7 +131,8 @@ mod parse_tree_tests {
         #[case] expected_status_code: i32,
     ) {
         let mut reader = Reader::default();
-        let mut writer = Writer::new_with_err(WBVec(vec![]), WBVec(vec![]));
+        let mut writer =
+            Writer::new_with_err(WBVec(vec![]), WBVec(vec![])).expect("Failed to create writer.");
         let status_code = ParseTreeTestRunner::default()
             .rules(rules_arg)
             .run(&mut writer, &mut reader);
@@ -169,7 +170,7 @@ mod parse_tree_tests {
         #[case] expected_status_code: i32,
     ) {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = ParseTreeTestRunner::default()
             .rules(rules_arg)
             .run(&mut writer, &mut reader);

guard/tests/rulegen.rs

@@ -66,7 +66,7 @@ mod rulegen_tests {
         #[case] expected_status_code: i32,
     ) {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = RulegenTestRunner::default()
             .template(template_arg)
             .run(&mut writer, &mut reader);

guard/tests/test_command.rs

@@ -134,7 +134,7 @@ mod test_command_tests {
     #[case("yaml")]
     fn test_data_file_with_shorthand_reference(#[case] file_type: &str) -> Result<(), Error> {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .test_data(Some(&format!(
                 "resources/test-command/data-dir/s3_bucket_logging_enabled_tests.{}",
@@ -159,7 +159,7 @@ mod test_command_tests {
     #[case("yaml")]
     fn test_data_file(#[case] file_type: &str) -> Result<(), Error> {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .test_data(Some(&format!(
                 "resources/test-command/data-dir/s3_bucket_server_side_encryption_enabled.{}",
@@ -182,7 +182,7 @@ mod test_command_tests {
     #[test]
     fn test_parse_error_when_guard_rule_has_syntax_error() {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .test_data(Some("resources/test-command/data-dir/test.yaml"))
             .rules(Some("resources/test-command/rule-dir/invalid_rule.guard"))
@@ -203,7 +203,8 @@ mod test_command_tests {
     #[test]
     fn test_parse_error_when_file_dne() {
         let mut reader = Reader::default();
-        let mut writer = Writer::new_with_err(WBVec(vec![]), WBVec(vec![]));
+        let mut writer =
+            Writer::new_with_err(WBVec(vec![]), WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .test_data(Some("resources/test-command/data-dir/test.yaml"))
             .rules(Some("/resources/test-command/data-dir/invalid_rule.guard"))
@@ -221,7 +222,7 @@ mod test_command_tests {
     #[test]
     fn test_data_file_verbose() -> Result<(), Error> {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .test_data(Some(
                 "resources/test-command/data-dir/s3_bucket_server_side_encryption_enabled.yaml",
@@ -244,7 +245,7 @@ mod test_command_tests {
     #[test]
     fn test_with_rules_dir_verbose() {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .directory(Option::from("resources/test-command/dir"))
             .directory_only()
@@ -264,7 +265,7 @@ mod test_command_tests {
     #[case("junit")]
     fn test_structured_single_report(#[case] output: &str) {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .test_data(Option::from(
                 "resources/test-command/data-dir/s3_bucket_server_side_encryption_enabled.yaml",
@@ -295,7 +296,7 @@ mod test_command_tests {
     #[case("junit")]
     fn test_structured_directory_report(#[case] output: &str) {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .directory(Option::from("resources/test-command/dir"))
             .output_format(output)
@@ -320,7 +321,7 @@ mod test_command_tests {
     #[case("yaml")]
     fn test_structured_report_with_illegal_args(#[case] output: &str) {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .directory(Option::from("resources/test-command/dir"))
             .output_format(output)
@@ -333,7 +334,7 @@ mod test_command_tests {
     #[test]
     fn test_with_function_expr() {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .test_data(Option::from(
                 "resources/test-command/functions/data/template.yaml",
@@ -350,7 +351,7 @@ mod test_command_tests {
     #[test]
     fn test_with_failure() {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .test_data(Option::from(
                 "resources/test-command/data-dir/failing_test.yaml",
@@ -366,7 +367,7 @@ mod test_command_tests {
     #[test]
     fn test_sarif_output_with_expected_failures() {
         let mut reader = Reader::default();
-        let mut writer = Writer::new(WBVec(vec![]));
+        let mut writer = Writer::new(WBVec(vec![])).expect("Failed to create writer.");
         let status_code = TestCommandTestRunner::default()
             .test_data(Option::from(
                 "resources/test-command/data-dir/failing_test.yaml",

guard/tests/utils.rs

@@ -73,7 +73,9 @@ pub fn sanitize_junit_writer(writer: Writer) -> Writer {
     let rgx = Regex::new(r#"time="\d+""#).unwrap();
     let res = rgx.replace_all(&buf, r#"time="0""#);
 
-    Writer::new(WBVec(res.as_bytes().to_vec()))
+    let writer = Writer::new(WBVec(res.as_bytes().to_vec())).expect("Failed to create writer.");
+
+    writer
 }
 
 #[allow(dead_code)]
@@ -83,7 +85,9 @@ pub fn sanitize_sarif_writer(writer: Writer) -> Writer {
     let rgx = Regex::new(r#"("uri": ".*")"#).unwrap();
     let res = rgx.replace_all(&buf, r#""uri": "some/path""#);
 
-    Writer::new(WBVec(res.as_bytes().to_vec()))
+    let writer = Writer::new(WBVec(res.as_bytes().to_vec())).expect("Failed to create writer.");
+
+    writer
 }
 
 pub fn get_full_path_for_resource_file(path: &str) -> String {