Skip to content

feat: blocking appender for rolling file and syslog #111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 24, 2025

Conversation

tisonkun
Copy link
Contributor

@tisonkun tisonkun commented Mar 24, 2025

This closes #96

Ok(())
}

fn flush(&self) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andylokandy maybe flush should return anyhow::Result<()> and we handle error all at one level up:

    fn log(&self, record: &Record) {
        for dispatch in &self.dispatches {
            if let Err(err) = dispatch.log(record) {
                handle_error(record, err);
            }
        }
    }

    fn flush(&self) {
        for dispatch in &self.dispatches {
            if let Err(err) = dispatch.flush() {
                handle_flush_error(err);
            }
        }
    }

also maybe add a ErrorSink trait for custom error listener.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep the behavior in sync with non-blocking its variant. is the non blocking one return result on flush?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The non-blocking one do flush in the worker thread where the error is handled as eprintln!("failed to write log: {err}");.

@tisonkun tisonkun force-pushed the blocking-file-appender branch 2 times, most recently from 1e56fc8 to 570cf0b Compare March 24, 2025 07:15
@tisonkun tisonkun changed the title refactor: blocking file appender feat: blocking file + syslog appender Mar 24, 2025
@tisonkun tisonkun changed the title feat: blocking file + syslog appender feat: blocking appender for rolling file and syslog Mar 24, 2025
@tisonkun tisonkun force-pushed the blocking-file-appender branch from 570cf0b to bff61e0 Compare March 24, 2025 07:18
@tisonkun tisonkun merged commit 6cfe443 into main Mar 24, 2025
9 checks passed
@tisonkun tisonkun deleted the blocking-file-appender branch March 24, 2025 07:24
@andylokandy
Copy link
Contributor

I'm curious why BlockingRollingFile is necessary since there RollingFile. It should be intuitive that RollingFile is blocking if non_blocking is not used.

@tisonkun
Copy link
Contributor Author

It should be intuitive that RollingFile is blocking if non_blocking is not used.

RollingFile accepts always NonBlocking. Do we have some way to paramizied this?

@andylokandy
Copy link
Contributor

RollingFile accepts always NonBlocking. Do we have some way to paramizied this?

Sound reasonable. I'll take some time to play with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement blocking appender for RollingFile and Syslog
2 participants