Add Builder::try_parse() method

Current implementation of the `Builder::parse()` method prints out specification errors to `stderr` and then just ignores them. This is fine for most console applications, but in some cases better control over what is happening is needed:

* Sometimes there is no access to `stderr`, in that case there is no way to find out what went wrong
* For some applications it's more desirable to fail on startup than to run with (partially) invalid configuration.

For such cases this PR introduces a new method `try_parse` that does the same thing as the `parse` method, but returns an `Err` in case an error in the specification is found.

This change was suggested in #323.

crates/env_filter/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
 
+### Features
+
+- Added `env_filter::Builder::try_parse(&self, &str)` method (failable version of `env_filter::Builder::parse()`)
+
 ## [0.1.0] - 2024-01-19
 
 <!-- next-url -->

crates/env_filter/src/filter.rs

@@ -6,6 +6,7 @@ use log::{LevelFilter, Metadata, Record};
 
 use crate::enabled;
 use crate::parse_spec;
+use crate::parser::{try_parse_spec, ParsingError};
 use crate::Directive;
 use crate::FilterOp;
 
@@ -107,6 +108,22 @@ impl Builder {
         self
     }
 
+    /// Parses the directive string, returning an error if the given directive string is invalid.
+    ///
+    /// See the [Enabling Logging] section for more details.
+    ///
+    /// [Enabling Logging]: ../index.html#enabling-logging
+    pub fn try_parse(&mut self, filters: &str) -> Result<&mut Self, ParsingError> {
+        let (directives, filter) = try_parse_spec(filters, false)?;
+
+        self.filter = filter;
+
+        for directive in directives {
+            self.insert_directive(directive);
+        }
+        Ok(self)
+    }
+
     /// Build a log filter.
     pub fn build(&mut self) -> Filter {
         assert!(!self.built, "attempt to re-use consumed builder");

crates/env_filter/src/lib.rs

@@ -56,3 +56,4 @@ use parser::parse_spec;
 pub use filter::Builder;
 pub use filter::Filter;
 pub use filtered_log::FilteredLog;
+pub use parser::ParsingError;

crates/env_filter/src/parser.rs

@@ -1,25 +1,60 @@
 use log::LevelFilter;
+use std::error::Error;
+use std::fmt::{Debug, Display, Formatter};
 
 use crate::Directive;
 use crate::FilterOp;
 
+/// Error during logger directive parsing process.
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct ParsingError {
+    /// Details of what exactly was wrong in the directive.
+    pub details: String,
+}
+
+impl Display for ParsingError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "error parsing logger filter: {}", self.details)
+    }
+}
+
+impl Error for ParsingError {}
+
 /// Parse a logging specification string (e.g: `crate1,crate2::mod3,crate3::x=error/foo`)
 /// and return a vector with log directives.
 pub(crate) fn parse_spec(spec: &str) -> (Vec<Directive>, Option<FilterOp>) {
-    #![allow(clippy::print_stderr)] // compatibility
+    try_parse_spec(spec, true).unwrap_or_else(|_| {
+        // This never happens, but better have this branch than add .expect()
+        (vec![], None)
+    })
+}
 
+/// Parse a logging specification string and return a vector with log directives.
+///
+/// If `skip_errors` argument is set to true, the errors will be printed out into the `stderr` and
+/// ignored, returning as the result correct directives from the given string.
+///
+/// If `skip_errors` is false, the first encountered error in an invalid specification string will
+/// be returned.
+pub(crate) fn try_parse_spec(
+    spec: &str,
+    skip_errors: bool,
+) -> Result<(Vec<Directive>, Option<FilterOp>), ParsingError> {
     let mut dirs = Vec::new();
 
     let mut parts = spec.split('/');
     let mods = parts.next();
     let filter = parts.next();
     if parts.next().is_some() {
-        eprintln!(
-            "warning: invalid logging spec '{}', \
+        err_or_print(
+            skip_errors,
+            format!(
+                "warning: invalid logging spec '{}', \
              ignoring it (too many '/'s)",
-            spec
-        );
-        return (dirs, None);
+                spec
+            ),
+        )?;
+        return Ok((dirs, None));
     }
     if let Some(m) = mods {
         for s in m.split(',').map(|ss| ss.trim()) {
@@ -42,20 +77,26 @@ pub(crate) fn parse_spec(spec: &str) -> (Vec<Directive>, Option<FilterOp>) {
                         if let Ok(num) = part1.parse() {
                             (num, Some(part0))
                         } else {
-                            eprintln!(
-                                "warning: invalid logging spec '{}', \
+                            err_or_print(
+                                skip_errors,
+                                format!(
+                                    "warning: invalid logging spec '{}', \
                                  ignoring it",
-                                part1
-                            );
+                                    part1
+                                ),
+                            )?;
                             continue;
                         }
                     }
                     _ => {
-                        eprintln!(
-                            "warning: invalid logging spec '{}', \
+                        err_or_print(
+                            skip_errors,
+                            format!(
+                                "warning: invalid logging spec '{}', \
                              ignoring it",
-                            s
-                        );
+                                s
+                            ),
+                        )?;
                         continue;
                     }
                 };
@@ -66,22 +107,39 @@ pub(crate) fn parse_spec(spec: &str) -> (Vec<Directive>, Option<FilterOp>) {
         }
     }
 
-    let filter = filter.and_then(|filter| match FilterOp::new(filter) {
-        Ok(re) => Some(re),
-        Err(e) => {
-            eprintln!("warning: invalid regex filter - {}", e);
-            None
-        }
-    });
+    let filter = filter
+        .map(|filter| match FilterOp::new(filter) {
+            Ok(re) => Ok(Some(re)),
+            Err(e) => err_or_print(
+                skip_errors,
+                format!("warning: invalid regex filter - {}", e),
+            )
+            .map(|_| None),
+        })
+        .transpose()?
+        .flatten();
+
+    Ok((dirs, filter))
+}
+
+fn err_or_print(skip_errors: bool, error_message: String) -> Result<(), ParsingError> {
+    #![allow(clippy::print_stderr)] // compatibility
 
-    (dirs, filter)
+    if skip_errors {
+        eprintln!("{error_message}");
+        Ok(())
+    } else {
+        Err(ParsingError {
+            details: error_message,
+        })
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use log::LevelFilter;
 
-    use super::parse_spec;
+    use super::{parse_spec, try_parse_spec};
 
     #[test]
     fn parse_spec_valid() {
@@ -108,6 +166,11 @@ mod tests {
         assert!(filter.is_none());
     }
 
+    #[test]
+    fn try_parse_spec_invalid_crate() {
+        assert!(try_parse_spec("crate1::mod1=warn=info,crate2=debug", false).is_err());
+    }
+
     #[test]
     fn parse_spec_invalid_level() {
         // test parse_spec with 'noNumber' as log level
@@ -118,6 +181,11 @@ mod tests {
         assert!(filter.is_none());
     }
 
+    #[test]
+    fn try_parse_spec_invalid_level() {
+        assert!(try_parse_spec("crate1::mod1=noNumber,crate2=debug", false).is_err());
+    }
+
     #[test]
     fn parse_spec_string_level() {
         // test parse_spec with 'warn' as log level
@@ -128,6 +196,11 @@ mod tests {
         assert!(filter.is_none());
     }
 
+    #[test]
+    fn try_parse_spec_string_level() {
+        assert!(try_parse_spec("crate1::mod1=wrong,crate2=warn", false).is_err());
+    }
+
     #[test]
     fn parse_spec_empty_level() {
         // test parse_spec with '' as log level
@@ -138,6 +211,11 @@ mod tests {
         assert!(filter.is_none());
     }
 
+    #[test]
+    fn try_parse_spec_empty_level() {
+        assert!(try_parse_spec("crate1::mod1=wrong,crate2=", false).is_err());
+    }
+
     #[test]
     fn parse_spec_empty_level_isolated() {
         // test parse_spec with "" as log level (and the entire spec str)
@@ -146,6 +224,13 @@ mod tests {
         assert!(filter.is_none());
     }
 
+    #[test]
+    fn try_parse_spec_empty_level_isolated() {
+        let (dirs, filter) = try_parse_spec("", false).expect("parsing empty level returned Err");
+        assert_eq!(dirs.len(), 0);
+        assert!(filter.is_none());
+    }
+
     #[test]
     fn parse_spec_blank_level_isolated() {
         // test parse_spec with a white-space-only string specified as the log
@@ -155,6 +240,14 @@ mod tests {
         assert!(filter.is_none());
     }
 
+    #[test]
+    fn try_parse_spec_blank_level_isolated() {
+        let (dirs, filter) =
+            try_parse_spec("     ", false).expect("parsing blank level returned Err");
+        assert_eq!(dirs.len(), 0);
+        assert!(filter.is_none());
+    }
+
     #[test]
     fn parse_spec_blank_level_isolated_comma_only() {
         // The spec should contain zero or more comma-separated string slices,
@@ -165,6 +258,14 @@ mod tests {
         assert!(filter.is_none());
     }
 
+    #[test]
+    fn try_parse_spec_blank_level_isolated_comma_only() {
+        let (dirs, filter) =
+            try_parse_spec(",", false).expect("parsing isolated comma returned Err");
+        assert_eq!(dirs.len(), 0);
+        assert!(filter.is_none());
+    }
+
     #[test]
     fn parse_spec_blank_level_isolated_comma_blank() {
         // The spec should contain zero or more comma-separated string slices,
@@ -176,6 +277,14 @@ mod tests {
         assert!(filter.is_none());
     }
 
+    #[test]
+    fn try_parse_spec_blank_level_isolated_comma_blank() {
+        let (dirs, filter) = try_parse_spec(",     ", false)
+            .expect("parsing isolated comma with blank returned Err");
+        assert_eq!(dirs.len(), 0);
+        assert!(filter.is_none());
+    }
+
     #[test]
     fn parse_spec_blank_level_isolated_blank_comma() {
         // The spec should contain zero or more comma-separated string slices,
@@ -187,6 +296,14 @@ mod tests {
         assert!(filter.is_none());
     }
 
+    #[test]
+    fn try_parse_spec_blank_level_isolated_blank_comma() {
+        let (dirs, filter) = try_parse_spec("     ,", false)
+            .expect("parsing blank with isolated comma returned Err");
+        assert_eq!(dirs.len(), 0);
+        assert!(filter.is_none());
+    }
+
     #[test]
     fn parse_spec_global() {
         // test parse_spec with no crate
@@ -261,4 +378,17 @@ mod tests {
         assert_eq!(dirs[0].level, LevelFilter::max());
         assert!(filter.is_some() && filter.unwrap().to_string() == "a*c");
     }
+
+    #[test]
+    fn parse_spec_with_multiple_filters() {
+        // Having multiple / is invalid format and should be ignored
+        let (dirs, filter) = parse_spec("crate1/a.c/a*c");
+        assert_eq!(dirs.len(), 0);
+        assert!(filter.is_none());
+    }
+
+    #[test]
+    fn try_parse_with_multiple_filters() {
+        assert!(try_parse_spec("crate1/a.c/a*c", false).is_err());
+    }
 }