Improve CLI HTTP error messages

[mozzieongit]

Originally, we often just printed the debug output of errors. That is very confusing and noisy. This PR changes that to chaining together the human-readable error descriptions of nested errors (relevant e.g. for tcp connection errors).

Example errors:

2026... ERROR cascade: Error: HTTP response decoding failed: expected value at line 1 column 1
2026... ERROR cascade: Error: HTTP connection failed: client error (Connect): tcp connect error: Connection refused (os error 111)
2026... ERROR cascade: Error: HTTP connection timed out
2026... ERROR cascade: Error: HTTP request failed: client error (SendRequest): connection closed before message completed

2026... ERROR cascade: Error: HTTP request failed with status code 404

Fixes: #481

[bal-e]

Looks good!

[mozzieongit]

As #481 explicitly mentions 404 errors, I set out to also make them better visible. As the code got very repetitive, I created new helper functions on the client.

The error output can now contain:

2026... ERROR cascade: Error: HTTP request failed with status code 404

[bal-e]

Not only does this improve functionality, we also lose a good chunk of boilerplate! I only have some minor comments.

[bal-e]

Thanks for documenting the need for the dependency! If it's only needed for cascade-api, perhaps cascade-api should re-export its serde dependency?

[mozzieongit]

So, instead of CLI's Cargo.toml including serde, the API crate would do pub use serde::DeserializeOwned somewhere and the CLI would do use api_crate::DeserializeOwned?

[bal-e]

The API crate would expose the whole serde crate, as cascade_api::serde or cascade_api::dep::serde (the latter is a convention also found in domain).

[mozzieongit]

Done

[bal-e]

I like the way this method now presents higher-level error types to the user. However, there may be information in the fmt::Display impl of err that we don't include (e.g. in the unknown error kind case). Would it be worthwhile to also log the full error (e.g. via tracing::debug!) so users can provide us more details easily?

[mozzieongit]

What do you mean with "full error"?
Display? That is only a very reduces error message without much info.
Debug? I think everything there is included in the source() concatenation.

I don't know what the Display would include that the error.source() doesn't.

Some quick examples:

Display: DEBUG cascade::client: Formatting error: error sending request for url (http://127.0.0.1:4539/zone/add)
Debug: DEBUG cascade::client: Formatting error: reqwest::Error { kind: Request, url: "http://127.0.0.1:4539/zone/add", source: hyper_util::client::legacy::Error(SendRequest, hyper::Error(IncompleteMessage)) }
Concat'd-string: ERROR cascade: HTTP request failed: client error (SendRequest): connection closed before message completed


Display: DEBUG cascade::client: Formatting error: HTTP status client error (404 Not Found) for url (http://127.0.0.1:4539/zone/example.com/status)
Debug: DEBUG cascade::client: Formatting error: reqwest::Error { kind: Status(404, None), url: "http://127.0.0.1:4539/zone/example.com/status" }
Concat'd-string: ERROR cascade: HTTP request failed with status code 404

Display: DEBUG cascade::client: Formatting error: error sending request for url (http://127.0.0.1:4539/zone/example.com/status)
Debug: DEBUG cascade::client: Formatting error: reqwest::Error { kind: Request, url: "http://127.0.0.1:4539/zone/example.com/status", source: hyper_util::client::legacy::Error(Connect, ConnectError("tcp connect error", 127.0.0.1:4539, Os { code: 111, kind: ConnectionRefused, message: "Connection refused" })) }
Concat'd-str: ERROR cascade: HTTP connection failed: client error (Connect): tcp connect error: Connection refused (os error 111)

[bal-e]

Thanks for sharing such detailed examples! I was mostly thinking of the else branch in the code, which you've commented as

Covers unknown errors, non-OK HTTP status codes, errors "related to the request" [1], errors "related to the request or response body" [1], errors "from a type Builder" [1], errors "from a RedirectPolicy." [1], errors "related to a protocol upgrade request" [1]

That sounds like a lot of different possibilities. Are we sure the source() printing loop will allow us to disambiguate between them?

[mozzieongit]

I'm pretty sure the source-loop will cover them all.

[mozzieongit]

I've changed the comment to say that it covers all other errors, as the explicit mention of all those types didn't really add any value.

Also, the Display impl prints less (and less useful) information than the source-loop will give, and the Debug print contains all the information that the source-loop will print, just with all the wrapping type names, but not more from my testing. The source-loop will allow us to disambiguate between the errors just as much as the Debug-print would.

[mozzieongit]

Interesting that CI didn't run for debad66 and c3eb99c