confidential-containers / trustee

Attestation and Secret Delivery Components
Apache License 2.0
64 stars 87 forks source link

Should we be concerned about "Big Ball of Mud" error messages? #344

Open mkulke opened 8 months ago

mkulke commented 8 months ago

When performing API requests to the kbs/as services (REST, gRPC) I often found it curious that consumers were exposed to a lot of internals of the service that is being called in an error case. I think this is due to a practice of [Big Ball of Mud] (https://www.lpalmieri.com/posts/error-handling-rust/#avoid-ball-of-mud-error-enums) Errors. We amass error messages as an error rolls down the call stack and when it ends up at the consumer there is a long API response: "Error: ... Error: ... Error ..." that looks like a flattened backtrace.

I think the consumers should only be exposed to the information that is necessary and actionable on the side of the consumer (i.e. not: "database connection problem", but yes: "client token expired" or "500: uh oh, call support" ).

mkulke commented 8 months ago

Example would be a snippet like this. e.to_string() is likely to convey much more information that we want to convey.

impl AttestationAgentService for AA {
    async fn get_token(...) {
            ....
            .map_err(|e| {
                error!("Call AA to get token failed: {}", e);
                Status::internal(format!("[ERROR:{}] AA get token failed: {}", AGENT_NAME, e))
            })?;