Get metrics even a request fails

[dotSlashLu]

Hi, thanks for this great lib!

I'm using it to implement a network debugging tool for our client, utilizing it's great metrics interface. In this context, error is expected and I want to extract metrics from the request. Is this currently possible? The phase throwing the error can be observed from the error's Kind, so specifically I need remote_addr of this failed request.

[sagebind]

Thanks for filing an issue! It is not currently possible to get access to a Metrics handle without a Response, but I can see how that might be useful.

so specifically I need remote_addr of this failed request.

There may or may not be a remote address depending on whether we actually managed to connect to a server or not. Though, the remote address isn't part of the metrics API but a different method. This seems pretty useful to add though to the Error type, we just have to make sure that the Error struct doesn't become too bloated at the risk of performance penalty.

What if we added a method to Error like this?

impl Error {
    /// Get the remote socket address of the last-used connection involved in
    /// this error, if known.
    ///
    /// If the request that caused this error failed to connect to any server,
    /// then this will be `None`.
    pub fn remote_addr(&self) -> Option<SocketAddr>;
}

[dotSlashLu]

That would be really nice in my scenario!

However, like you said, this implementation might bloat the Error type. Due to my limited experience on Error interface design, I can't give you any insightful opinion on this. But maybe it's better to just tuck this information in the error's context? For example, "Timed out connecting to {SockAddr}"?

[sagebind]

Are you just looking to have a more descriptive error message? Or do you need to actually extract the remote address for something? Because trying to scrape it from the error message or error context sounds risky from a stability perspective if the error message format changes (which isn't part of the API stability guarantee).

[sagebind]

I have a branch that adds local_addr and remote_addr methods to the Error type in a branch here: https://github.com/sagebind/isahc/pull/337. I see no issue with exposing these since Error already heap-allocates in order to avoid taking up too much stack space. (You don't want to penalize the performance of the success path by having an error type that takes up a lot of stack space.)

[sagebind]

This is now exposed in the 1.5.0 release!