Skip to content

Commit

Permalink
feat: Add Builder::try_parse method
Browse files Browse the repository at this point in the history
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 commit 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.
  • Loading branch information
Maximkaaa committed Jul 23, 2024
1 parent faf5b3e commit 11e5d37
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 1 deletion.
4 changes: 4 additions & 0 deletions crates/env_filter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
Expand Down
37 changes: 37 additions & 0 deletions crates/env_filter/src/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::parse_spec;
use crate::parser::ParseResult;
use crate::Directive;
use crate::FilterOp;
use crate::ParseError;

/// A builder for a log filter.
///
Expand Down Expand Up @@ -118,6 +119,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, ParseError> {
let (directives, filter) = parse_spec(filters).ok()?;

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");
Expand Down Expand Up @@ -241,6 +258,7 @@ impl fmt::Debug for Filter {
#[cfg(test)]
mod tests {
use log::{Level, LevelFilter};
use snapbox::{assert_data_eq, str};

use super::{enabled, Builder, Directive, Filter};

Expand Down Expand Up @@ -455,6 +473,25 @@ mod tests {
}
}

#[test]
fn try_parse_valid_filter() {
let logger = Builder::new()
.try_parse("info,crate1::mod1=warn")
.expect("valid filter returned error")
.build();
assert!(enabled(&logger.directives, Level::Warn, "crate1::mod1"));
assert!(enabled(&logger.directives, Level::Info, "crate2::mod2"));
}

#[test]
fn try_parse_invalid_filter() {
let error = Builder::new().try_parse("info,crate1=invalid").unwrap_err();
assert_data_eq!(
error,
str!["error parsing logger filter: invalid logging spec 'invalid'"]
);
}

#[test]
fn match_full_path() {
let logger = make_logger_filter(vec![
Expand Down
1 change: 1 addition & 0 deletions crates/env_filter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,4 @@ use parser::parse_spec;
pub use filter::Builder;
pub use filter::Filter;
pub use filtered_log::FilteredLog;
pub use parser::ParseError;
60 changes: 59 additions & 1 deletion crates/env_filter/src/parser.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use log::LevelFilter;
use std::error::Error;
use std::fmt::{Display, Formatter};

use crate::Directive;
use crate::FilterOp;
Expand All @@ -22,8 +24,35 @@ impl ParseResult {
fn add_error(&mut self, message: String) {
self.errors.push(message);
}

pub(crate) fn ok(self) -> Result<(Vec<Directive>, Option<FilterOp>), ParseError> {
let Self { directives, filter, errors } = self;
if let Some(error) = errors.into_iter().next() {
Err(ParseError { details: error })
} else {
Ok((directives, filter))
}
}
}

/// Error during logger directive parsing process.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct ParseError {
details: String,
}

impl Display for ParseError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(
f,
"error parsing logger filter: {}",
self.details
)
}
}

impl Error for ParseError {}

/// 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) -> ParseResult {
Expand Down Expand Up @@ -86,11 +115,18 @@ pub(crate) fn parse_spec(spec: &str) -> ParseResult {

#[cfg(test)]
mod tests {
use crate::ParseError;
use log::LevelFilter;
use snapbox::{assert_data_eq, str};
use snapbox::{assert_data_eq, str, Data, IntoData};

use super::{parse_spec, ParseResult};

impl IntoData for ParseError {
fn into_data(self) -> Data {
self.to_string().into_data()
}
}

#[test]
fn parse_spec_valid() {
let ParseResult {
Expand Down Expand Up @@ -460,4 +496,26 @@ mod tests {
);
assert_data_eq!(&errors[1], str!["invalid logging spec 'invalid'"]);
}

#[test]
fn parse_error_message_single_error() {
let error = parse_spec("crate1::mod1=debug=info,crate2=debug")
.ok()
.unwrap_err();
assert_data_eq!(
error,
str!["error parsing logger filter: invalid logging spec 'crate1::mod1=debug=info'"]
);
}

#[test]
fn parse_error_message_multiple_errors() {
let error = parse_spec("crate1::mod1=debug=info,crate2=debug,crate3=invalid")
.ok()
.unwrap_err();
assert_data_eq!(
error,
str!["error parsing logger filter: invalid logging spec 'crate1::mod1=debug=info'"]
);
}
}

0 comments on commit 11e5d37

Please sign in to comment.