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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add plugin error propagation on write/flush #12670

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

devyn
Copy link
Contributor

@devyn devyn commented Apr 26, 2024

Description

Yet another attempt to fix the stress_internals::test_wrong_version() test...

This time I think it's probably because we are getting a broken pipe write error when we try to send Hello or perhaps something after it, because the pipe has already been closed by the reader when it saw the invalid version. In that case, an error should be available in state. It probably makes more sense to send that back to the user rather than an unhelpful I/O error.

Tests + Formatting

  • 馃煝 toolkit fmt
  • 馃煝 toolkit clippy
  • 馃煝 toolkit test
  • 馃煝 toolkit test stdlib

Yet another attempt to fix the `stress_internals::test_wrong_version()`
test...

This time I think it's probably because we are getting a broken pipe
write error when we try to send `Hello` or perhaps something after it,
because the pipe has already been closed by the reader when it saw the
invalid version. In that case, an error should be available in state. It
probably makes more sense to send that back to the user rather than an
unhelpful I/O error.
@fdncred fdncred added the pr:plugins This PR is related to plugins label Apr 26, 2024
@fdncred fdncred merged commit d126793 into nushell:main Apr 26, 2024
15 checks passed
@fdncred
Copy link
Collaborator

fdncred commented Apr 26, 2024

thanks, hopefully this fixes it.

@devyn devyn deleted the plugin-error-propagation-on-write branch April 26, 2024 11:37
@hustcer hustcer added this to the v0.93.0 milestone Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:plugins This PR is related to plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants