Skip to content

feat: blocking appender for rolling file and syslog#111

Merged
tisonkun merged 2 commits intomainfrom
blocking-file-appender
Mar 24, 2025
Merged

feat: blocking appender for rolling file and syslog#111
tisonkun merged 2 commits intomainfrom
blocking-file-appender

Conversation

@tisonkun
Copy link
Copy Markdown
Contributor

@tisonkun tisonkun commented Mar 24, 2025

This closes #96

Ok(())
}

fn flush(&self) {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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}");.

Signed-off-by: tison <wander4096@gmail.com>
@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
Signed-off-by: tison <wander4096@gmail.com>
@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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