Promote the error print of guest-components

[Xynnn007]

Currently, the error information printed by guest-components is not well-formatted. An expected one from my point of view should looks like

some error
Caused by: level 1 reason
Caused by: some more detailed reason
...

This follows the discussion of confidential-containers/trustee#327 (comment)

BTW, we should think about whether the concrete error information should be reported to higher level. A most simple way is to report a rough error type to the caller of the guest components, and the concrete guest component itself should log the error information in the format mentioned above.

wdyt? @mkulke and probably @kartikjoshi21

[mkulke]

The caller of guest-components would be api-server-rest in this case?

I think we should probably define what is sensible, ASR is mostly a convenience facility, so it would be nice for it to produce some readable error messages. I can image that e.g. a 404 we receive from KBS can be propagated to the ASR response, but for anything else it could be a simple 500 http status, with additional information in the process logs. An in-container user would not be able to resolve an internal error in normal circumstances.

not sure there is a logging crate that will print multiline error traces. we might have to build it ourselves

[fitzthum]

Yes, possibly related to #529

Also keep in mind that sometimes we should be careful about what error messages are exposed to the host.

[Xynnn007]

not sure there is a logging crate that will print multiline error traces. we might have to build it ourselves

With thiserror, we could impl a macro format_error where we needs to generate a multiple line error information as mentioned in the message

macro_rules! format_error {
    ($err:expr) => {{
        let mut error_string = format!("{}", $err);
        let mut current_error = $err.source();
        while let Some(source) = current_error {
            error_string.push_str(&format!("\nCaused by: {}", source));
            current_error = source.source();
        }
        error_string
    }};
}

And every time we wrap an inner error type with #[from], a new line generates

#[derive(Error, Debug)]
pub enum ErrorLevel2 {
    #[error("A new line error information")]
    SomeKind(#[from] ErrorLevel1)
}

I think we should probably define what is sensible, ASR is mostly a convenience facility, so it would be nice for it to produce some readable error messages. I can image that e.g. a 404 we receive from KBS can be propagated to the ASR response, but for anything else it could be a simple 500 http status, with additional information in the process logs. An in-container user would not be able to resolve an internal error in normal circumstances.

It makes sense that we should concrete internal error information should be not be delivered. I also agree that your potential point that we should do error truncation before delivering to concrete end user.

Let me make a table to show where the errors information will be exposed to non-guest-components

Interface	source
Host	kata-agent's CDH call <-- CDH API error
App container	ASR's web api <-- CDH/AA API error

so we have two choices to truncate the error information. either in

1. CDH/AA's API implementation: Means that CDH/AA will log the whole error information, but with a brief error information in API response.
2. or the caller side (ASR and kata-agent): Means that kata-agent and ASR will get the whole error information, but it will only leave the first line of the error and abondon other lines. Related to api-server-rest: Responded with 200 OK although providing error in the body #529, if error happens in ASR, a 500 http status will be returned.

Both are practical. One thing to note that if we use way 1, a CDH client tool will not get the whole error message. So I am with way 2. The rationale behind is that kata-agent and ASR are all abstract "guest-components", so logs are secure to transfer in-between them.

[mkulke]

a CDH client tool will not get the whole error message.

I think that's the point.

I'm not sure how confidentiality changes this, but in general a web service like KBS should only yield information in errors that is the business of the client. Internal errors (database connection problems, etc) are not the client's business, they cannot act on it and it might reveal configuration details that are not to be exposed (think: "error: mycloud_client_id abcd1234 has insufficient quota") to the caller. so internal errors should be limited to 500 imo and maybe a correlation/transaction id, so a service owner can find it easier in the service logs.

Not sure how this applies to ASR, if ASR is supposed to be only a transparent proxy to CDH's ttrpc apis, ASR can be more liberal with the errors it forwards to the caller.