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

bkt: stdout pipe failed: failed to write whole buffer #44

Closed
agostonbarna opened this issue Aug 27, 2023 · 4 comments
Closed

bkt: stdout pipe failed: failed to write whole buffer #44

agostonbarna opened this issue Aug 27, 2023 · 4 comments

Comments

@agostonbarna
Copy link

Hi,
I recently ran into an error with bkt:

bkt -- find . -type f | head -n1
./file
bkt: Subprocess execution failed: stdout pipe failed: failed to write whole buffer

This only seem to happen with many files/lines, but I don't get the error when piping the stdout to wc -l, so maybe the issue only happens when the process on the right side of the pipe closes the stdout of the bkt process before it could write out it's whole output (e.g. head, fzf).

bkt -- find . -type f | wc -l
286910

The stdout is actually correct in both cases so it sounds like this bkt error is not really necessary to show to the user.

Reproduced with:

  • bkt version: 0.7.0 (with cargo install bkt)
  • macOS 13.5 (arm64)
  • a fairly up to date arch linux (aarch64).
@dimo414
Copy link
Owner

dimo414 commented Aug 27, 2023

Thanks for the report! 0.7.0 was just released so this is likely a regression (i.e. you can workaround via 0.6.1 in the meantime), I'll see if I can reproduce and fix.

For reference, the error message is coming from write_all() which we don't strictly need to call.

dimo414 added a commit that referenced this issue Aug 27, 2023
DisregardBrokenPipe needs to implement write_all() as well as write()
because the former treats a 0-byte write as an error, whereas DPB should
no-op in that scenario.

Note that this is fix is in main.rs and therefore _only_ fixes the CLI.
Along with the existing TODO on DPB it may be preferable to stop using
write_all() in lib.rs, but for now I'm erring on the side of letting
library users handle failures themselves.

Fixes #44
@dimo414
Copy link
Owner

dimo414 commented Aug 27, 2023

@agostonbarna would you mind trying out these binaries and confirming the issue is resolved on your end? I'll cut a 0.7.1 as soon as I hear from you.

@dimo414 dimo414 pinned this issue Aug 28, 2023
@dimo414 dimo414 unpinned this issue Aug 28, 2023
@agostonbarna
Copy link
Author

Hey @dimo414 ,
Wow, thanks for the quick fix! I checked again with both the previous (0.7.0) and your new bkt version (0.7.1), and I can only reproduce it with the old one, so it looks good 👍

@dimo414
Copy link
Owner

dimo414 commented Aug 28, 2023

Thanks for confirming!

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