ch-remote: also pretty-print remote server errors

Remote server errors are transferred as raw HTTP body. This way,
we lose the nested structured error information.

This is an attempt to retrieve the errors from the HTTP response
and to align the output with the normal error output.

For example, this produces the following chain of errors. Note
that everything after level 0 was retrieved from the HTTP server
response:

```
Error: ch-remote exited with the following chain of errors:
  0: http client error
  1: Server responded with InternalServerError
  2: Error from API
  3: The disk could not be added to the VM
  4: Failed to validate config
  5: Identifier disk1 is not unique

Debug Info: HttpApiClient(ServerResponse(InternalServerError, Some("Error from API<br>The disk could not be added to the VM<br>Failed to validate config<br>Identifier disk1 is not unique")))
```

In case the JSON can't be parsed properly, ch-remote will print:

```
Error: ch-remote exited with the following chain of errors:
  0: http client error
  X: Can't get remote's error messages from JSON response: EOF while parsing a value at line 1 column 0: body=''

Debug Info: HttpApiClient(ServerResponse(InternalServerError, Some("")))
```

Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com

Author: phip1611

api_client/src/lib.rs

@@ -21,8 +21,14 @@ pub enum Error {
     MissingProtocol,
     #[error("Error parsing HTTP Content-Length field")]
     ContentLengthParsing(#[source] std::num::ParseIntError),
-    #[error("Server responded with an error: {0:?}: {1:?}")]
-    ServerResponse(StatusCode, Option<String>),
+    #[error("Server responded with error {0:?}: {1:?}")]
+    ServerResponse(
+        StatusCode,
+        // TODO: Move `api` module from `vmm` to dedicated crate and use a common type definition
+        Option<
+            String, /* Untyped: Currently Vec<String> of error messages from top to root cause */
+        >,
+    ),
 }
 
 #[derive(Clone, Copy, Debug)]

src/bin/ch-remote.rs

@@ -1132,7 +1132,70 @@ fn main() {
     };
 
     if let Err(top_error) = target_api.do_command(&matches) {
-        cloud_hypervisor::cli_print_error_chain(&top_error, "ch-remote");
+        // Helper to join strings with a newline.
+        fn join_strs(mut acc: String, next: String) -> String {
+            if !acc.is_empty() {
+                acc.push('\n');
+            }
+            acc.push_str(&next);
+            acc
+        }
+
+        // This function helps to modify the Display representation of remote
+        // API failures so that it aligns with the regular output of error
+        // messages. As we transfer a deep/rich chain of errors as String via
+        // the HTTP API, the nested error chain is lost. We retrieve it from
+        // the error response.
+        //
+        // In case the repose itself is broken, the error is printed directly
+        // by using the `X` level.
+        fn server_api_error_display_modifier(
+            level: usize,
+            indention: usize,
+            error: &(dyn std::error::Error + 'static),
+        ) -> Option<String> {
+            if let Some(api_client::Error::ServerResponse(status_code, body)) =
+                error.downcast_ref::<api_client::Error>()
+            {
+                let body = body.as_ref().map(|body| body.as_str()).unwrap_or("");
+
+                // Retrieve the list of error messages back.
+                let lines: Vec<&str> = match serde_json::from_str(body) {
+                    Ok(json) => json,
+                    Err(e) => {
+                        return Some(format!(
+                            "{idention}X: Can't get remote's error messages from JSON response: {e}: body='{body}'",
+                            idention = " ".repeat(indention)
+                        ));
+                    }
+                };
+
+                let error_status = format!("Server responded with {status_code:?}");
+                // Prepend the error status line to the lines iter.
+                let lines = std::iter::once(error_status.as_str()).chain(lines);
+                let error_msg_multiline = lines
+                    .enumerate()
+                    .map(|(index, error_msg)| (index + level, error_msg))
+                    .map(|(level, error_msg)| {
+                        format!(
+                            "{idention}{level}: {error_msg}",
+                            idention = " ".repeat(indention)
+                        )
+                    })
+                    .fold(String::new(), join_strs);
+
+                return Some(error_msg_multiline);
+            }
+
+            None
+        }
+
+        let top_error: &dyn std::error::Error = &top_error;
+        cloud_hypervisor::cli_print_error_chain(
+            top_error,
+            "ch-remote",
+            server_api_error_display_modifier,
+        );
         process::exit(1)
     };
 }

src/lib.rs

@@ -7,7 +7,16 @@ use std::error::Error;
 /// Prints a chain of errors to the user in a consistent manner.
 /// The user will see a clear chain of errors, followed by debug output
 /// for opening issues.
-pub fn cli_print_error_chain(top_error: &dyn Error, component: &str) {
+pub fn cli_print_error_chain<'a>(
+    top_error: &'a (dyn Error + 'static),
+    component: &str,
+    // Function optionally returning the display representation of an error.
+    display_modifier: impl Fn(
+        /* level */ usize,
+        /*indention */ usize,
+        &'a (dyn Error + 'static),
+    ) -> Option<String>,
+) {
     eprint!("Error: {component} exited with the following ");
     if top_error.source().is_none() {
         eprintln!("error:");
@@ -21,7 +30,12 @@ pub fn cli_print_error_chain(top_error: &dyn Error, component: &str) {
         })
         .enumerate()
         .for_each(|(level, error)| {
-            eprintln!("  {level}: {error}",);
+            // Special case: handling of HTTP Server responses in ch-remote
+            if let Some(message) = display_modifier(level, 2, error) {
+                eprintln!("{message}");
+            } else {
+                eprintln!("  {level}: {error}");
+            }
         });
     }
 

src/main.rs

@@ -884,7 +884,7 @@ fn main() {
             0
         }
         Err(top_error) => {
-            cloud_hypervisor::cli_print_error_chain(&top_error, "Cloud Hypervisor");
+            cloud_hypervisor::cli_print_error_chain(&top_error, "Cloud Hypervisor", |_, _, _| None);
             1
         }
     };

vmm/src/api/http/mod.rs

@@ -4,6 +4,7 @@
 //
 
 use std::collections::BTreeMap;
+use std::error::Error;
 use std::fs::File;
 use std::os::unix::io::{IntoRawFd, RawFd};
 use std::os::unix::net::UnixListener;
@@ -69,20 +70,29 @@ pub enum HttpError {
 
 const HTTP_ROOT: &str = "/api/v1";
 
-/// Creates the error response's body meant to be sent back to an API client.
+/// Creates the error response's JSON body meant to be sent back to an API client.
+///
 /// The error message contained in the response is supposed to be user-facing,
 /// thus insightful and helpful while balancing technical accuracy and
 /// simplicity.
 pub fn error_response(error: HttpError, status: StatusCode) -> Response {
     let mut response = Response::new(Version::Http11, status);
-    // We must use debug output here without `#`, as it is currently the only
-    // feasible option to get all relevant error details to the receiver,
-    // i.e., ch-remote, in a balanced form. The Display impl is not guaranteed
-    // to hold all relevant or helpful data.
-    //
-    // TODO: We might print a nice error chain here as well and send it to the
-    // remote, similar to the normal error reporting?
-    response.set_body(Body::new(format!("{error:?}")));
+
+    let error: &dyn Error = &error;
+    // Write the Display::display() output all errors (from top to root).
+    let error_messages = std::iter::successors(Some(error), |sub_error| {
+        // Dereference necessary to mitigate rustc compiler bug.
+        // See <https://github.com/rust-lang/rust/issues/141673>
+        (*sub_error).source()
+    })
+    .map(|error| format!("{error}"))
+    .collect::<Vec<_>>();
+
+    // TODO: Move `api` module from `vmm` to dedicated crate and use a common type definition
+    let json = serde_json::to_string(&error_messages).unwrap();
+
+    let body = Body::new(json);
+    response.set_body(body);
 
     response
 }