Return error instead of serializing closures as nil/null for msgpack and json, add --serialize for msgpack(z)

crates/nu-command/src/formats/to/json.rs

@@ -57,7 +57,7 @@ impl Command for ToJson {
         // allow ranges to expand and turn into array
         let input = input.try_expand_range()?;
         let value = input.into_value(span)?;
-        let json_value = value_to_json_value(engine_state, &value, serialize_types)?;
+        let json_value = value_to_json_value(engine_state, &value, span, serialize_types)?;
 
         let json_result = if raw {
             nu_json::to_string_raw(&json_value)
@@ -78,16 +78,12 @@ impl Command for ToJson {
                 };
                 Ok(PipelineData::Value(res, Some(metadata)))
             }
-            _ => Ok(Value::error(
+            _ => Err(ShellError::CantConvert {
-                ShellError::CantConvert {
+                to_type: "JSON".into(),
-                    to_type: "JSON".into(),
+                from_type: value.get_type().to_string(),
-                    from_type: value.get_type().to_string(),
-                    span,
-                    help: None,
-                },
                 span,
-            )
+                help: None,
-            .into_pipeline_data()),
+            }),
         }
     }
 
@@ -118,6 +114,7 @@ impl Command for ToJson {
 pub fn value_to_json_value(
     engine_state: &EngineState,
     v: &Value,
+    call_span: Span,
     serialize_types: bool,
 ) -> Result<nu_json::Value, ShellError> {
     let span = v.span();
@@ -142,7 +139,7 @@ pub fn value_to_json_value(
         ),
 
         Value::List { vals, .. } => {
-            nu_json::Value::Array(json_list(engine_state, vals, serialize_types)?)
+            nu_json::Value::Array(json_list(engine_state, vals, call_span, serialize_types)?)
         }
         Value::Error { error, .. } => return Err(*error.clone()),
         Value::Closure { val, .. } => {
@@ -153,13 +150,23 @@ pub fn value_to_json_value(
                     let contents_string = String::from_utf8_lossy(contents_bytes);
                     nu_json::Value::String(contents_string.to_string())
                 } else {
-                    nu_json::Value::String(format!(
+                    return Err(ShellError::CantConvert {
-                        "unable to retrieve block contents for json block_id {}",
+                        to_type: "string".into(),
-                        val.block_id.get()
+                        from_type: "closure".into(),
-                    ))
+                        span,
+                        help: Some(format!(
+                            "unable to retrieve block contents for closure with id {}",
+                            val.block_id.get()
+                        )),
+                    });
                 }
             } else {
-                nu_json::Value::Null
+                return Err(ShellError::UnsupportedInput {
+                    msg: "closures are currently not deserializable (use --serialize to serialize as a string)".into(),
+                    input: "value originates from here".into(),
+                    msg_span: call_span,
+                    input_span: span,
+                });
             }
         }
         Value::Range { .. } => nu_json::Value::Null,
@@ -171,14 +178,14 @@ pub fn value_to_json_value(
             for (k, v) in &**val {
                 m.insert(
                     k.clone(),
-                    value_to_json_value(engine_state, v, serialize_types)?,
+                    value_to_json_value(engine_state, v, call_span, serialize_types)?,
                 );
             }
             nu_json::Value::Object(m)
         }
         Value::Custom { val, .. } => {
             let collected = val.to_base_value(span)?;
-            value_to_json_value(engine_state, &collected, serialize_types)?
+            value_to_json_value(engine_state, &collected, call_span, serialize_types)?
         }
     })
 }
@@ -186,12 +193,18 @@ pub fn value_to_json_value(
 fn json_list(
     engine_state: &EngineState,
     input: &[Value],
+    call_span: Span,
     serialize_types: bool,
 ) -> Result<Vec<nu_json::Value>, ShellError> {
     let mut out = vec![];
 
     for value in input {
-        out.push(value_to_json_value(engine_state, value, serialize_types)?);
+        out.push(value_to_json_value(
+            engine_state,
+            value,
+            call_span,
+            serialize_types,
+        )?);
     }
 
     Ok(out)

crates/nu-command/src/formats/to/msgpack.rs

@@ -22,6 +22,11 @@ impl Command for ToMsgpack {
     fn signature(&self) -> Signature {
         Signature::build(self.name())
             .input_output_type(Type::Any, Type::Binary)
+            .switch(
+                "serialize",
+                "serialize nushell types that cannot be deserialized",
+                Some('s'),
+            )
             .category(Category::Formats)
     }
 
@@ -69,8 +74,8 @@ MessagePack: https://msgpack.org/
 
     fn run(
         &self,
-        _engine_state: &EngineState,
+        engine_state: &EngineState,
-        _stack: &mut Stack,
+        stack: &mut Stack,
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
@@ -83,7 +88,16 @@ MessagePack: https://msgpack.org/
         let value = input.into_value(value_span)?;
         let mut out = vec![];
 
-        write_value(&mut out, &value, 0)?;
+        let serialize_types = call.has_flag(engine_state, stack, "serialize")?;
+
+        write_value(
+            &mut out,
+            &value,
+            0,
+            engine_state,
+            call.head,
+            serialize_types,
+        )?;
 
         Ok(Value::binary(out, call.head).into_pipeline_data_with_metadata(Some(metadata)))
     }
@@ -148,6 +162,9 @@ pub(crate) fn write_value(
     out: &mut impl io::Write,
     value: &Value,
     depth: usize,
+    engine_state: &EngineState,
+    call_span: Span,
+    serialize_types: bool,
 ) -> Result<(), WriteError> {
     use mp::ValueWriteError::InvalidMarkerWrite;
     let span = value.span();
@@ -196,6 +213,9 @@ pub(crate) fn write_value(
                 out,
                 &Value::list(val.into_range_iter(span, Signals::empty()).collect(), span),
                 depth,
+                engine_state,
+                call_span,
+                serialize_types,
             )?;
         }
         Value::String { val, .. } => {
@@ -208,13 +228,20 @@ pub(crate) fn write_value(
             mp::write_map_len(out, convert(val.len(), span)?).err_span(span)?;
             for (k, v) in val.iter() {
                 mp::write_str(out, k).err_span(span)?;
-                write_value(out, v, depth + 1)?;
+                write_value(out, v, depth + 1, engine_state, call_span, serialize_types)?;
             }
         }
         Value::List { vals, .. } => {
             mp::write_array_len(out, convert(vals.len(), span)?).err_span(span)?;
             for val in vals {
-                write_value(out, val, depth + 1)?;
+                write_value(
+                    out,
+                    val,
+                    depth + 1,
+                    engine_state,
+                    call_span,
+                    serialize_types,
+                )?;
             }
         }
         Value::Nothing { .. } => {
@@ -222,11 +249,32 @@ pub(crate) fn write_value(
                 .map_err(InvalidMarkerWrite)
                 .err_span(span)?;
         }
-        Value::Closure { .. } => {
+        Value::Closure { val, .. } => {
-            // Closures can't be converted
+            if serialize_types {
-            mp::write_nil(out)
+                let block = engine_state.get_block(val.block_id);
-                .map_err(InvalidMarkerWrite)
+                let closure_string: &str = if let Some(span) = block.span {
-                .err_span(span)?;
+                    let contents_bytes = engine_state.get_span_contents(span);
+                    &String::from_utf8_lossy(contents_bytes)
+                } else {
+                    return Err(WriteError::Shell(Box::new(ShellError::CantConvert {
+                        to_type: "string".into(),
+                        from_type: "closure".into(),
+                        span,
+                        help: Some(format!(
+                            "unable to retrieve block contents for closure with id {}",
+                            val.block_id.get()
+                        )),
+                    })));
+                };
+                mp::write_str(out, closure_string).err_span(span)?;
+            } else {
+                return Err(WriteError::Shell(Box::new(ShellError::UnsupportedInput {
+                    msg: "closures are currently not deserializable (use --serialize to serialize as a string)".into(),
+                    input: "value originates from here".into(),
+                    msg_span: call_span,
+                    input_span: span,
+                })));
+            }
         }
         Value::Error { error, .. } => {
             return Err(WriteError::Shell(error.clone()));
@@ -249,7 +297,14 @@ pub(crate) fn write_value(
             mp::write_bin(out, val).err_span(span)?;
         }
         Value::Custom { val, .. } => {
-            write_value(out, &val.to_base_value(span)?, depth)?;
+            write_value(
+                out,
+                &val.to_base_value(span)?,
+                depth,
+                engine_state,
+                call_span,
+                serialize_types,
+            )?;
         }
     }
     Ok(())

crates/nu-command/src/formats/to/msgpackz.rs

@@ -32,6 +32,11 @@ impl Command for ToMsgpackz {
                 "Window size for brotli compression (default 20)",
                 Some('w'),
             )
+            .switch(
+                "serialize",
+                "serialize nushell types that cannot be deserialized",
+                Some('s'),
+            )
             .category(Category::Formats)
     }
 
@@ -69,6 +74,7 @@ impl Command for ToMsgpackz {
             .get_flag(engine_state, stack, "window-size")?
             .map(to_u32)
             .transpose()?;
+        let serialize_types = call.has_flag(engine_state, stack, "serialize")?;
 
         let value_span = input.span().unwrap_or(call.head);
         let value = input.into_value(value_span)?;
@@ -80,7 +86,14 @@ impl Command for ToMsgpackz {
             window_size.map(|w| w.item).unwrap_or(DEFAULT_WINDOW_SIZE),
         );
 
-        write_value(&mut out, &value, 0)?;
+        write_value(
+            &mut out,
+            &value,
+            0,
+            engine_state,
+            call.head,
+            serialize_types,
+        )?;
         out.flush()
             .map_err(|err| IoError::new(err.kind(), call.head, None))?;
         drop(out);

crates/nu-command/src/formats/to/nuon.rs

@@ -69,16 +69,9 @@ impl Command for ToNuon {
         match nuon::to_nuon(engine_state, &value, style, Some(span), serialize_types) {
             Ok(serde_nuon_string) => Ok(Value::string(serde_nuon_string, span)
                 .into_pipeline_data_with_metadata(Some(metadata))),
-            _ => Ok(Value::error(
+            Err(error) => {
-                ShellError::CantConvert {
+                Ok(Value::error(error, span).into_pipeline_data_with_metadata(Some(metadata)))
-                    to_type: "NUON".into(),
+            }
-                    from_type: value.get_type().to_string(),
-                    span,
-                    help: None,
-                },
-                span,
-            )
-            .into_pipeline_data_with_metadata(Some(metadata))),
         }
     }
 

crates/nu-command/src/network/http/client.rs

@@ -276,7 +276,7 @@ fn send_json_request(
 ) -> Result<Response, ShellErrorOrRequestError> {
     match body {
         Value::Int { .. } | Value::Float { .. } | Value::List { .. } | Value::Record { .. } => {
-            let data = value_to_json_value(engine_state, &body, serialize_types)?;
+            let data = value_to_json_value(engine_state, &body, span, serialize_types)?;
             send_cancellable_request(request_url, Box::new(|| req.send_json(data)), span, signals)
         }
         // If the body type is string, assume it is string json content.

crates/nuon/src/to.rs

@@ -109,7 +109,7 @@ fn value_to_string(
                 }
             } else {
                 Err(ShellError::UnsupportedInput {
-                    msg: "closures are currently not nuon-compatible".into(),
+                    msg: "closures are currently not deserializable (use --serialize to serialize as a string)".into(),
                     input: "value originates from here".into(),
                     msg_span: span,
                     input_span: v.span(),