RollingFileAppender::new() panics if the file can't be opened
#1953
Labels
Milestone
Comments
hawkw
added
crate/appender
|
I would consider changing the Footnotes
|
|
If a |
hawkw
added a commit
that referenced
this issue
Jul 18, 2022
Several currently open PRs, such as #2225 and #2221, add new configuration parameters to the rolling file appender in `tracing-appender`. The best way to add new optional configuration settings without breaking existing APIs or creating a very large number of new constructors is to add a builder interface. Since a number of PRs would all need to add the builder API, introducing potential conflicts, this branch _just_ adds the builder interface without adding any new configuration options. Once this merges, the existing in-flight PRs can be rebased onto this branch to use the builder interface without conflicting with each other. Also, the `Builder::build` method is fallible and returns a `Result`, rather than panicking. This is a relatively common pattern in Rust --- for example, `std::thread::Builder::spawn` returns a `Result` if a new thread cannot be spawned, while `std::thread::spawn` simply panics. This allows users to handle appender initialization errors gracefully without breaking the API of the existing `new` constructor. Fixes #1953
hawkw
added a commit
that referenced
this issue
Jul 18, 2022
## Motivation Several currently open PRs, such as #2225 and #2221, add new configuration parameters to the rolling file appender in `tracing-appender`. The best way to add new optional configuration settings without breaking existing APIs or creating a very large number of new constructors is to add a builder interface. ## Solution Since a number of PRs would all need to add the builder API, introducing potential conflicts, this branch _just_ adds the builder interface without adding any new configuration options. Once this merges, the existing in-flight PRs can be rebased onto this branch to use the builder interface without conflicting with each other. Also, the `Builder::build` method is fallible and returns a `Result`, rather than panicking. This is a relatively common pattern in Rust --- for example, `std::thread::Builder::spawn` returns a `Result` if a new thread cannot be spawned, while `std::thread::spawn` simply panics. This allows users to handle appender initialization errors gracefully without breaking the API of the existing `new` constructor. Fixes #1953 Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this issue
Jul 20, 2022
## Motivation Several currently open PRs, such as #2225 and #2221, add new configuration parameters to the rolling file appender in `tracing-appender`. The best way to add new optional configuration settings without breaking existing APIs or creating a very large number of new constructors is to add a builder interface. ## Solution Since a number of PRs would all need to add the builder API, introducing potential conflicts, this branch _just_ adds the builder interface without adding any new configuration options. Once this merges, the existing in-flight PRs can be rebased onto this branch to use the builder interface without conflicting with each other. Also, the `Builder::build` method is fallible and returns a `Result`, rather than panicking. This is a relatively common pattern in Rust --- for example, `std::thread::Builder::spawn` returns a `Result` if a new thread cannot be spawned, while `std::thread::spawn` simply panics. This allows users to handle appender initialization errors gracefully without breaking the API of the existing `new` constructor. Fixes #1953 Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this issue
Jul 20, 2022
## Motivation Several currently open PRs, such as #2225 and #2221, add new configuration parameters to the rolling file appender in `tracing-appender`. The best way to add new optional configuration settings without breaking existing APIs or creating a very large number of new constructors is to add a builder interface. ## Solution Since a number of PRs would all need to add the builder API, introducing potential conflicts, this branch _just_ adds the builder interface without adding any new configuration options. Once this merges, the existing in-flight PRs can be rebased onto this branch to use the builder interface without conflicting with each other. Also, the `Builder::build` method is fallible and returns a `Result`, rather than panicking. This is a relatively common pattern in Rust --- for example, `std::thread::Builder::spawn` returns a `Result` if a new thread cannot be spawned, while `std::thread::spawn` simply panics. This allows users to handle appender initialization errors gracefully without breaking the API of the existing `new` constructor. Fixes #1953 Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this issue
Jul 20, 2022
## Motivation Several currently open PRs, such as #2225 and #2221, add new configuration parameters to the rolling file appender in `tracing-appender`. The best way to add new optional configuration settings without breaking existing APIs or creating a very large number of new constructors is to add a builder interface. ## Solution Since a number of PRs would all need to add the builder API, introducing potential conflicts, this branch _just_ adds the builder interface without adding any new configuration options. Once this merges, the existing in-flight PRs can be rebased onto this branch to use the builder interface without conflicting with each other. Also, the `Builder::build` method is fallible and returns a `Result`, rather than panicking. This is a relatively common pattern in Rust --- for example, `std::thread::Builder::spawn` returns a `Result` if a new thread cannot be spawned, while `std::thread::spawn` simply panics. This allows users to handle appender initialization errors gracefully without breaking the API of the existing `new` constructor. Fixes #1953 Signed-off-by: Eliza Weisman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug Report
Version
tracing-appender v0.2.0
Crates
tracing-appender
Description
tracing-appender panics if it can't create the writer. This happens if the file can't be opened for appending. For example, if I do
tracing_appender::rolling::daily("/", "foo"), on macOS this panics with an IO error about "Read-only file system" (on other OSes I'd expect a permissions error if I'm not running as root).Opening the file synchronously is a great idea, it means I can actually handle errors instead of having them just be printed to stderr, but this needs to actually be returned as a
Resultrather than panicking. This includes not justRollingFileAppender::new()but also the convenience functions (tracing_appender::rolling::{daily, hourly, minutely, never}). This is of course a breaking change.The text was updated successfully, but these errors were encountered: