Remove serde derive for ShellError, replace via LabeledError (#12319)

# Description

This changes the interface for plugins to always represent errors as
`LabeledError`s. This is good for altlang plugins, as it would suck for
them to have to implement and track `ShellError`. We save a lot of
generated code from the `ShellError` serde impl too, so `nu` and plugins
get to have a smaller binary size.

Reduces the release binary size by 1.2 MiB on my build configuration.

# User-Facing Changes

- Changes plugin protocol. `ShellError` no longer serialized.
- `ShellError` serialize output is different
- `ShellError` no longer deserializes to exactly the same value as
serialized

# Tests + Formatting

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting

- [ ] Document in plugin protocol reference

crates/nu-plugin/src/plugin/interface/stream/tests.rs

@@ -103,7 +103,8 @@ fn list_reader_recv_wrong_type() -> Result<(), ShellError> {
 #[test]
 fn reader_recv_raw_messages() -> Result<(), ShellError> {
     let (tx, rx) = mpsc::channel();
-    let mut reader = StreamReader::new(0, rx, TestSink::default());
+    let mut reader =
+        StreamReader::<Result<Vec<u8>, ShellError>, _>::new(0, rx, TestSink::default());
 
     tx.send(Ok(Some(StreamData::Raw(Ok(vec![10, 20])))))
         .unwrap();
@@ -458,7 +459,7 @@ fn stream_manager_write_scenario() -> Result<(), ShellError> {
     let expected_values = vec![b"hello".to_vec(), b"world".to_vec(), b"test".to_vec()];
 
     for value in &expected_values {
-        writable.write(Ok(value.clone()))?;
+        writable.write(Ok::<_, ShellError>(value.clone()))?;
     }
 
     // Now try signalling ack

crates/nu-plugin/src/plugin/interface/tests.rs

@@ -184,8 +184,14 @@ fn read_pipeline_data_external_stream() -> Result<(), ShellError> {
 
     test.add(StreamMessage::Data(14, Value::test_int(1).into()));
     for _ in 0..iterations {
-        test.add(StreamMessage::Data(12, Ok(out_pattern.clone()).into()));
-        test.add(StreamMessage::Data(13, Ok(err_pattern.clone()).into()));
+        test.add(StreamMessage::Data(
+            12,
+            StreamData::Raw(Ok(out_pattern.clone())),
+        ));
+        test.add(StreamMessage::Data(
+            13,
+            StreamData::Raw(Ok(err_pattern.clone())),
+        ));
     }
     test.add(StreamMessage::End(12));
     test.add(StreamMessage::End(13));

crates/nu-plugin/src/protocol/mod.rs

@@ -236,7 +236,7 @@ impl From<StreamMessage> for PluginInput {
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub enum StreamData {
     List(Value),
-    Raw(Result<Vec<u8>, ShellError>),
+    Raw(Result<Vec<u8>, LabeledError>),
 }
 
 impl From<Value> for StreamData {
@@ -245,9 +245,15 @@ impl From<Value> for StreamData {
     }
 }
 
+impl From<Result<Vec<u8>, LabeledError>> for StreamData {
+    fn from(value: Result<Vec<u8>, LabeledError>) -> Self {
+        StreamData::Raw(value)
+    }
+}
+
 impl From<Result<Vec<u8>, ShellError>> for StreamData {
     fn from(value: Result<Vec<u8>, ShellError>) -> Self {
-        StreamData::Raw(value)
+        value.map_err(LabeledError::from).into()
     }
 }
 
@@ -264,10 +270,10 @@ impl TryFrom<StreamData> for Value {
     }
 }
 
-impl TryFrom<StreamData> for Result<Vec<u8>, ShellError> {
+impl TryFrom<StreamData> for Result<Vec<u8>, LabeledError> {
     type Error = ShellError;
 
-    fn try_from(data: StreamData) -> Result<Result<Vec<u8>, ShellError>, ShellError> {
+    fn try_from(data: StreamData) -> Result<Result<Vec<u8>, LabeledError>, ShellError> {
         match data {
             StreamData::Raw(value) => Ok(value),
             StreamData::List(_) => Err(ShellError::PluginFailedToDecode {
@@ -277,6 +283,14 @@ impl TryFrom<StreamData> for Result<Vec<u8>, ShellError> {
     }
 }
 
+impl TryFrom<StreamData> for Result<Vec<u8>, ShellError> {
+    type Error = ShellError;
+
+    fn try_from(value: StreamData) -> Result<Result<Vec<u8>, ShellError>, ShellError> {
+        Result::<Vec<u8>, LabeledError>::try_from(value).map(|res| res.map_err(ShellError::from))
+    }
+}
+
 /// A stream control or data message.
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub enum StreamMessage {

crates/nu-protocol/src/errors/shell_error.rs

@@ -3,13 +3,14 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
 use crate::{
-    ast::Operator, engine::StateWorkingSet, format_error, ParseError, Span, Spanned, Value,
+    ast::Operator, engine::StateWorkingSet, format_error, LabeledError, ParseError, Span, Spanned,
+    Value,
 };
 
 /// The fundamental error type for the evaluation engine. These cases represent different kinds of errors
 /// the evaluator might face, along with helpful spans to label. An error renderer will take this error value
 /// and pass it into an error viewer to display to the user.
-#[derive(Debug, Clone, Error, Diagnostic, Serialize, Deserialize, PartialEq)]
+#[derive(Debug, Clone, Error, Diagnostic, PartialEq)]
 pub enum ShellError {
     /// An operator received two arguments of incompatible types.
     ///
@@ -1407,6 +1408,75 @@ impl From<super::LabeledError> for ShellError {
     }
 }
 
+/// `ShellError` always serializes as [`LabeledError`].
+impl Serialize for ShellError {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        LabeledError::from_diagnostic(self).serialize(serializer)
+    }
+}
+
+/// `ShellError` always deserializes as if it were [`LabeledError`], resulting in a
+/// [`ShellError::LabeledError`] variant.
+impl<'de> Deserialize<'de> for ShellError {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        LabeledError::deserialize(deserializer).map(ShellError::from)
+    }
+}
+
 pub fn into_code(err: &ShellError) -> Option<String> {
     err.code().map(|code| code.to_string())
 }
+
+#[test]
+fn shell_error_serialize_roundtrip() {
+    // Ensure that we can serialize and deserialize `ShellError`, and check that it basically would
+    // look the same
+    let original_error = ShellError::CantConvert {
+        span: Span::new(100, 200),
+        to_type: "Foo".into(),
+        from_type: "Bar".into(),
+        help: Some("this is a test".into()),
+    };
+    println!("orig_error = {:#?}", original_error);
+
+    let serialized =
+        serde_json::to_string_pretty(&original_error).expect("serde_json::to_string_pretty failed");
+    println!("serialized = {}", serialized);
+
+    let deserialized: ShellError =
+        serde_json::from_str(&serialized).expect("serde_json::from_str failed");
+    println!("deserialized = {:#?}", deserialized);
+
+    // We don't expect the deserialized error to be the same as the original error, but its miette
+    // properties should be comparable
+    assert_eq!(original_error.to_string(), deserialized.to_string());
+
+    assert_eq!(
+        original_error.code().map(|c| c.to_string()),
+        deserialized.code().map(|c| c.to_string())
+    );
+
+    let orig_labels = original_error
+        .labels()
+        .into_iter()
+        .flatten()
+        .collect::<Vec<_>>();
+    let deser_labels = deserialized
+        .labels()
+        .into_iter()
+        .flatten()
+        .collect::<Vec<_>>();
+
+    assert_eq!(orig_labels, deser_labels);
+
+    assert_eq!(
+        original_error.help().map(|c| c.to_string()),
+        deserialized.help().map(|c| c.to_string())
+    );
+}