Skip to content
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

error logs enabled by default, causing glitches in UI #238

Closed
osa1 opened this issue Aug 17, 2020 · 7 comments
Closed

error logs enabled by default, causing glitches in UI #238

osa1 opened this issue Aug 17, 2020 · 7 comments
Labels
Milestone

Comments

@osa1
Copy link
Owner

osa1 commented Aug 17, 2020

Try this config with a rustls build:

    - addr: irc.rizon.net
      port: 6697
      tls: true
      realname: test
      nicks: ['test']

tiny will be dumping this stuff to stderr:

[2020-08-17T07:34:05Z] ERROR [/home/omer/.cargo/registry/src/github.com-1ecc6299db9ec823/rustls-0.18.0/src/session.rs:512] TLS alert received: Message {
    typ: Alert,
    version: TLSv1_2,
    payload: Alert(
        AlertMessagePayload {
            level: Fatal,
            description: HandshakeFailure,
        },
    ),
}
[2020-08-17T07:34:05Z] ERROR [/home/omer/.cargo/registry/src/github.com-1ecc6299db9ec823/rustls-0.18.0/src/session.rs:512] TLS alert received: Message {
    typ: Alert,
    version: TLSv1_2,
    payload: Alert(
        AlertMessagePayload {
            level: Fatal,
            description: HandshakeFailure,
        },
    ),
}

Normally logs enabled manually, with something like RUST_LOG=debug tiny. I don't know why these are enabled by default.

@osa1 osa1 added the bug label Aug 17, 2020
@osa1 osa1 added this to the 0.6.1 milestone Aug 17, 2020
@osa1
Copy link
Owner Author

osa1 commented Aug 19, 2020

It turns out this is not a rustls problem -- currently we enable error logs by default. I realized this after 58f03de which adds some more error logs which are enabled by default currently.

@osa1 osa1 changed the title rustls logs stuff to stderr by default error logs enabled by default, causing glitches in UI Aug 20, 2020
@osa1
Copy link
Owner Author

osa1 commented Aug 20, 2020

I briefly looked into fixing this, but I think it's not easily possible. We'll probably need to copy the default logger implementation from env_logger/log and modify it so that by default all logs are disabled, and the env variable overrides those defaults. Something like

    env_logger::builder()
        .target(env_logger::Target::Stderr)
        .filter_level(log::LevelFilter::Off)
        ...

doesn't work as this overrides the env variable. We want to override the default, not the settings generated from the env variable.


Here's another idea which may be a better fix for this issue. Leave error logs on by default, but redirect all logs to a file by default (e.g. tiny_logs.txt). I think we could even redirect all of stderr to a file by default, as it doesn't make sense to print to the terminal in a TUI app.

So perhaps it's a good idea to create two files by default: tiny_stderr.txt and tiny_logs.txt. These are purely for debugging purposes. The question is where to create those. One idea is in the log dir, but that's not always available.

@osa1
Copy link
Owner Author

osa1 commented Aug 21, 2020

Ugh.. it seems like env_logger doesn't support logging to a file: https://docs.rs/env_logger/0.7.1/env_logger/enum.Target.html

@osa1
Copy link
Owner Author

osa1 commented Aug 26, 2020

Perhaps we can use flexi_logger which can log to files.

osa1 added a commit that referenced this issue Aug 26, 2020
@osa1
Copy link
Owner Author

osa1 commented Aug 26, 2020

I just pushed a branch that uses flexi_logger instead of env_logger. It already fixes the bug as apparently flexi_logger doesn't enable ERROR logs by default.

Another benefit is the release binary size gets 11.6% smaller! (from 6766352 bytes to 5975432 bytes) Amazing.

Only problem is flexi_logger creates the log file by default. I'd prefer it if it created the file on first log. That way we could avoid creating a file most of the time. Perhaps it's already possible somehow with a setting but I couldn't immediately see it in the documentation and I have to get back to work now.

@osa1
Copy link
Owner Author

osa1 commented Aug 26, 2020

Another benefit is the release binary size gets 11.6% smaller! (from 6766352 bytes to 5975432 bytes) Amazing.

This is probably because I disabled default features which we don't use. We could do the same in env_logger as well.

@osa1
Copy link
Owner Author

osa1 commented Aug 26, 2020

If I use this for env_logger:

env_logger = { version = "0.7", default-features = false, features = ["humantime"] }

Then flexi_logger is larger. Binary size increases by 5.2% (from 5677784 bytes to 5975432 bytes). So flexi_logger is actually worse.

@osa1 osa1 closed this as completed in 2a24948 Sep 9, 2020
osa1 added a commit that referenced this issue Sep 9, 2020
@osa1 osa1 modified the milestones: 0.6.1, 0.7.0 Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant