rustfmt --check: handle writing to a broken pipe gracefully #6012
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The rustfmt tool panics when any writes to the output file descriptor fail, whether sending color codes to the terminal, regular writes, or flushing stdout. It's fairly easy to hit this by accidentally writing to a broken pipe. For example, I hit this when I tried to pipe into `less` but made a typo: $ rustfmt --check src/main.rs | les panic This patch updates rustfmt to handle BrokenPipe / EPIPE failures more gracefully. Any failures lead to the program immediately exiting, since there is no sense in continuing to write to a broken pipe, except for the final flush() which doesn't need to exit early since it's already at the end of program execution. Any write errors aside from BrokenPipe still result in panic!(), since any other errors are unexpected. This leaves the program behaving the same as before for any other write errors.
|
Probably better not to 
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this addresses a panic when
rustfmt --checkmakes awrite()call that fails. There have been a few issues that already discussed this:#5114
#2926
On one of the issues it was noted that it would be difficult to hit this outside of contrived situations like piping to "false", but I think it really is pretty easy to hit -- I got this panic by accidentally making a typo in "less", and piping to "les":
With this patch I opted to leave the panic behavior in place for any write errors other than EPIPE, so that the tool's behavior is unchanged except in this specific situation. However, it could be preferable to take the opportunity to remove the panics entirely and handle all write errors gracefully. I can update the patch to not panic at all on write errors, if others agree.