Futher improve messages related to error propagation on plugin calls (#12646)

# Description
Trying to give as much context as possible. Now there should be a
spanned error with the call span if possible, and the propagated error
as an inner error if there was one in every case.

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

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

@@ -106,6 +106,8 @@ struct PluginCallState {
     ctrlc: Option<Arc<AtomicBool>>,
     /// Channel to receive context on to be used if needed
     context_rx: Option<mpsc::Receiver<Context>>,
+    /// Span associated with the call, if any
+    span: Option<Span>,
     /// Channel for plugin custom values that should be kept alive for the duration of the plugin
     /// call. The plugin custom values on this channel are never read, we just hold on to it to keep
     /// them in memory so they can be dropped at the end of the call. We hold the sender as well so
@@ -299,6 +301,7 @@ impl PluginInterfaceManager {
                     context_tx: None,
                     keep_plugin_custom_values_tx: Some(state.keep_plugin_custom_values.0.clone()),
                     entered_foreground: false,
+                    span: state.span,
                 };
 
                 let handler = move || {
@@ -692,6 +695,7 @@ impl PluginInterface {
             context_tx: Some(context_tx),
             keep_plugin_custom_values_tx: Some(keep_plugin_custom_values.0.clone()),
             entered_foreground: false,
+            span: call.span(),
         };
 
         // Prepare the call with the state.
@@ -735,24 +739,24 @@ impl PluginInterface {
                     dont_send_response,
                     ctrlc,
                     context_rx: Some(context_rx),
+                    span: call.span(),
                     keep_plugin_custom_values,
                     remaining_streams_to_read: 0,
                 },
             ))
-            .map_err(|_| ShellError::GenericError {
+            .map_err(|_| {
-                error: format!("Plugin `{}` closed unexpectedly", self.state.source.name()),
+                let existing_error = self.state.error.get().cloned();
-                msg: "can't complete this operation because the plugin is closed".into(),
+                ShellError::GenericError {
-                span: match &call {
+                    error: format!("Plugin `{}` closed unexpectedly", self.state.source.name()),
-                    PluginCall::Signature => None,
+                    msg: "can't complete this operation because the plugin is closed".into(),
-                    PluginCall::Run(CallInfo { call, .. }) => Some(call.head),
+                    span: call.span(),
-                    PluginCall::CustomValueOp(val, _) => Some(val.span),
+                    help: Some(format!(
-                },
+                        "the plugin may have experienced an error. Try loading the plugin again \
-                help: Some(format!(
-                    "the plugin may have experienced an error. Try loading the plugin again \
                         with `{}`",
-                    self.state.source.identity.use_command(),
+                        self.state.source.identity.use_command(),
-                )),
+                    )),
-                inner: vec![],
+                    inner: existing_error.into_iter().collect(),
+                }
             })?;
 
         // Starting a plugin call adds a lock on the GC. Locks are not added for streams being read
@@ -818,14 +822,21 @@ impl PluginInterface {
             }
         }
         // If we fail to get a response, check for an error in the state first, and return it if
-        // set. This is probably a much more helpful error than 'failed to receive response'
+        // set. This is probably a much more helpful error than 'failed to receive response' alone
-        if let Some(error) = self.state.error.get() {
+        let existing_error = self.state.error.get().cloned();
-            Err(error.clone())
+        Err(ShellError::GenericError {
-        } else {
+            error: format!(
-            Err(ShellError::PluginFailedToDecode {
+                "Failed to receive response to plugin call from `{}`",
-                msg: "Failed to receive response to plugin call".into(),
+                self.state.source.identity.name()
-            })
+            ),
-        }
+            msg: "while waiting for this operation to complete".into(),
+            span: state.span,
+            help: Some(format!(
+                "try restarting the plugin with `{}`",
+                self.state.source.identity.use_command()
+            )),
+            inner: existing_error.into_iter().collect(),
+        })
     }
 
     /// Handle an engine call and write the response.
@@ -870,7 +881,20 @@ impl PluginInterface {
     ) -> Result<PluginCallResponse<PipelineData>, ShellError> {
         // Check for an error in the state first, and return it if set.
         if let Some(error) = self.state.error.get() {
-            return Err(error.clone());
+            return Err(ShellError::GenericError {
+                error: format!(
+                    "Failed to send plugin call to `{}`",
+                    self.state.source.identity.name()
+                ),
+                msg: "the plugin encountered an error before this operation could be attempted"
+                    .into(),
+                span: call.span(),
+                help: Some(format!(
+                    "try loading the plugin again with `{}`",
+                    self.state.source.identity.use_command(),
+                )),
+                inner: vec![error.clone()],
+            });
         }
 
         let result = self.write_plugin_call(call, context.as_deref())?;
@@ -1087,6 +1111,8 @@ pub struct CurrentCallState {
     /// The plugin call entered the foreground: this should be cleaned up automatically when the
     /// plugin call returns.
     entered_foreground: bool,
+    /// The span that caused the plugin call.
+    span: Option<Span>,
 }
 
 impl CurrentCallState {

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

@@ -196,6 +196,7 @@ fn fake_plugin_call(
             dont_send_response: false,
             ctrlc: None,
             context_rx: None,
+            span: None,
             keep_plugin_custom_values: mpsc::channel(),
             remaining_streams_to_read: 0,
         },
@@ -502,6 +503,7 @@ fn manager_handle_engine_call_after_response_received() -> Result<(), ShellError
             dont_send_response: false,
             ctrlc: None,
             context_rx: Some(context_rx),
+            span: None,
             keep_plugin_custom_values: mpsc::channel(),
             remaining_streams_to_read: 1,
         },
@@ -567,6 +569,7 @@ fn manager_send_plugin_call_response_removes_context_only_if_no_streams_to_read(
                 dont_send_response: false,
                 ctrlc: None,
                 context_rx: None,
+                span: None,
                 keep_plugin_custom_values: mpsc::channel(),
                 remaining_streams_to_read: n as i32,
             },
@@ -602,6 +605,7 @@ fn manager_consume_stream_end_removes_context_only_if_last_stream() -> Result<()
                 dont_send_response: false,
                 ctrlc: None,
                 context_rx: None,
+                span: None,
                 keep_plugin_custom_values: mpsc::channel(),
                 remaining_streams_to_read: n as i32,
             },

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

@@ -156,6 +156,15 @@ impl<D> PluginCall<D> {
             }
         })
     }
+
+    /// The span associated with the call.
+    pub fn span(&self) -> Option<Span> {
+        match self {
+            PluginCall::Signature => None,
+            PluginCall::Run(CallInfo { call, .. }) => Some(call.head),
+            PluginCall::CustomValueOp(val, _) => Some(val.span),
+        }
+    }
 }
 
 /// Operations supported for custom values.