Commit Graph

4 Commits

Author SHA1 Message Date
Devyn Cairns
d126793290
Add plugin error propagation on write/flush (#12670)
# 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`
2024-04-26 06:23:58 -05:00
Devyn Cairns
c52884b3c8
Fix (and test) for a deadlock that can happen while waiting for protocol info (#12633)
# Description

The local socket PR introduced a `Waitable` type, which could either
hold a value or be waited on until a value is available. Unlike a
channel, it would always return that value once set.

However, one issue with this design was that there was no way to detect
whether a value would ever be written. This splits the writer into a
different type `WaitableMut`, so that when it is dropped, waiting
threads can fail (because they'll never get a value).

# Tests + Formatting

A test has been added to `stress_internals` to make sure this fails in
the right way.

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
2024-04-24 08:44:04 -05:00
Devyn Cairns
8b7696f4c1
stress_internals: exit(1) on io error (#12612)
# Description

The `stress_internals` tests can fail sometimes, but usually not on the
CI, because Nushell exits while the plugin is still trying to read or
maybe write something, leading to a broken pipe.

`nu-plugin` already exits with 1 without printing a message on a
protocol-level I/O error, so this just doing the same thing.

I think there's probably a way to correct the plugin handling so that we
wait for plugins to shut down before exiting and this doesn't happen,
but this is the quick fix in the meantime.
2024-04-21 23:48:09 +02:00
Devyn Cairns
c06ef201b7
Local socket mode and foreground terminal control for plugins (#12448)
# Description

Adds support for running plugins using local socket communication
instead of stdio. This will be an optional thing that not all plugins
have to support.

This frees up stdio for use to make plugins that use stdio to create
terminal UIs, cc @amtoine, @fdncred.

This uses the [`interprocess`](https://crates.io/crates/interprocess)
crate (298 stars, MIT license, actively maintained), which seems to be
the best option for cross-platform local socket support in Rust. On
Windows, a local socket name is provided. On Unixes, it's a path. The
socket name is kept to a relatively small size because some operating
systems have pretty strict limits on the whole path (~100 chars), so on
macOS for example we prefer `/tmp/nu.{pid}.{hash64}.sock` where the hash
includes the plugin filename and timestamp to be unique enough.

This also adds an API for moving plugins in and out of the foreground
group, which is relevant for Unixes where direct terminal control
depends on that.

TODO:

- [x] Generate local socket path according to OS conventions
- [x] Add support for passing `--local-socket` to the plugin executable
instead of `--stdio`, and communicating over that instead
- [x] Test plugins that were broken, including
[amtoine/nu_plugin_explore](https://github.com/amtoine/nu_plugin_explore)
- [x] Automatically upgrade to using local sockets when supported,
falling back if it doesn't work, transparently to the user without any
visible error messages
  - Added protocol feature: `LocalSocket`
- [x] Reset preferred mode to `None` on `register`
- [x] Allow plugins to detect whether they're running on a local socket
and can use stdio freely, so that TUI plugins can just produce an error
message otherwise
  - Implemented via `EngineInterface::is_using_stdio()`
- [x] Clean up foreground state when plugin command exits on the engine
side too, not just whole plugin
- [x] Make sure tests for failure cases work as intended
  - `nu_plugin_stress_internals` added

# User-Facing Changes
- TUI plugins work
- Non-Rust plugins could optionally choose to use this
- This might behave differently, so will need to test it carefully
across different operating systems

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting
- [ ] Document local socket option in plugin contrib docs
- [ ] Document how to do a terminal UI plugin in plugin contrib docs
- [ ] Document: `EnterForeground` engine call
- [ ] Document: `LeaveForeground` engine call
- [ ] Document: `LocalSocket` protocol feature
2024-04-15 18:28:18 +00:00