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

Confusing error (panic) when writing to a broken stdout/pipe #49

Open
mozzieongit opened this issue Dec 11, 2024 · 2 comments
Open

Confusing error (panic) when writing to a broken stdout/pipe #49

mozzieongit opened this issue Dec 11, 2024 · 2 comments

Comments

@mozzieongit
Copy link
Member

While accidentally piping dnst output into an already exited program (it failed due to invalid arguments), dnst panics, as

self.0.write_fmt(args).unwrap();
uses unwrap on the output stream.
This leads to an ugly error that was confusing at first.

$ target/release/dnst nsec3-hash hi | sort -k1s
sort: stray character in field spec: invalid field specification ‘1s’
thread 'main' panicked at /home/mozzie/repos/work/dnst.trees/main/src/env/mod.rs:71:32:
called `Result::unwrap()` on an `Err` value: Error
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Maybe this should change to either:

  1. an attempt to write to stderr that writing to stdout failed (if that also fails, ignore and proceed to exit), and exit with an error code; or
  2. just exit with an error code
@ximon18
Copy link
Member

ximon18 commented Dec 11, 2024

Good catch.

Interesting that it happens at an unwrap that was deliberately ignored.

I vote for option 1 as I've see other programs behave that way, because it has some chance of alerting the user to the reason for the program exiting which otherwise could be confusing.

@mozzieongit
Copy link
Member Author

Ignoring it deliberately was inspired by the rust std library, but it was missed that the std library catches the error and panics purposefully with a error message (other than "unwrapped an error value").

So Option 3: do it like rust' std library:

$ rustc println.rs && ./println | true                                                
thread 'main' panicked at library/std/src/io/stdio.rs:1118:9:
failed printing to stdout: Broken pipe (os error 32)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

From rust-lang/rust io/stdio.rs

if let Err(e) = global_s().write_fmt(args) {
    panic!("failed printing to {label}: {e}");
}

But I agree on the vote for option 1: attempt to print to stderr; the panic could be too scary for users.

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

No branches or pull requests

2 participants