feat(client): TrySendError<T> is From<Error>

[cratelyn]

this commit introduces a new trait implementation for hyper::client::conn::TrySendError<T>.

this commit allows a TrySendError<T> to be constructed From a conventional hyper::Error.

one example of a motivating use case for this change is that this is needed in order to use interfaces like
hyper::client::conn::http2::SendRequest::try_send_request() or hyper::client::conn::http1::SendRequest::try_send_request() in the context of tower middleware; the Service<T> trait's signature is such that the same error type be returned from Service::poll_ready() and Service::call().

thus, the ? operator can construct a TrySendError<T> from errors possibly returned by
hyper::client::conn::http2::SendRequest::poll_ready() or hyper::client::conn::http1::SendRequest::poll_ready(), within a Service<T> that eventually calls try_send_request() in the context of Service::call().

---

see:

• feat(client): add SendRequest::try_send_request() method #3691
• https://docs.rs/hyper/latest/hyper/client/conn/struct.TrySendError.html
• https://docs.rs/hyper/latest/hyper/struct.Error.html
• https://docs.rs/hyper/latest/hyper/client/conn/http2/struct.SendRequest.html#method.try_send_request
• https://docs.rs/tower/latest/tower/trait.Service.html#associatedtype.Error
• https://docs.rs/hyper/latest/hyper/client/conn/http1/struct.SendRequest.html#method.poll_ready
• https://docs.rs/hyper/latest/hyper/client/conn/http2/struct.SendRequest.html#method.poll_ready

[seanmonstar]

This one I'm slightly skeptical about. I've generally adopt the opinion of making errors not constructable outside of the conditions that they represent. (I know some people want to construct them during tests, but I find it extremely valuable to reason about the guarantee of an error because of a certain call.)

Is it possible to solve this locally in the service impl? Either by cast them into a Box<dyn StdError>, or with an enum? (Is that a horrible ask?)

[cratelyn]

This one I'm slightly skeptical about. I've generally adopt the opinion of
making errors not constructable outside of the conditions that they
represent. (I know some people want to construct them during tests, but I
find it extremely valuable to reason about the guarantee of an error because
of a certain call.)

definitely, i am sympathetic to your point! i think there are some other ways
of solving this issue, which i will outline below.

on services, and error handling

Is it possible to solve this locally in the service impl?

unfortunately, this isn't ideally handled locally in the Service<T>. hyper
applications that rely on tower often have a "tiered" model, with separate
T-typed targets to represent connection-level and request-level handlers.

tower calls these higher-order services a MakeService<T, R>.
this is a sealed trait, implemented on behalf of types whose
<M as Service<T>>::Response responses implement Service<R>.

very similar to this general pattern is the connection-oriented
[MakeConnection<T>], which is approximately the same thing but for services
that respond with an i/o transport Connection: AsyncRead + AsyncWrite.

hyper, circa 0.14, relied on vendored variation(s) of this. MakeConnection
was vendored and used internally, with hyper::client::connect::Connect
exposed in the public interface.

https://github.com/hyperium/hyper/blob/0.14.x/src/service/make.rs#L11-L20

https://github.com/hyperium/hyper/blob/0.14.x/src/client/connect/mod.rs#L438-L455

while now Connect lives in hyper-util, this is a conceptual model that is
baked into the architecture of many hyper applications in my experience.

for the purposes of this discussion, all of this is relevant because it means
that the business logic for (re)connecting to an endpoint (load-balancing,
backoff, timeouts, [..]) is often a distinct tower::Service<T> that
instantiates and invokes the tower::Service<T> responsible for processing
individual requests.

within such an architecture, a TrySendError<T> that contains a recovered
message should be propagated out of the request-handler, and handled by the
connection layer.

other options

TrySendError: std::error::Error

Is it possible to solve this [..] by casting them into a Box<dyn StdError>,
or with an enum? (Is that a horrible ask?)

hyper::client::conn::TrySendError<T> does not currently implement
std::error::Error, so it cannot be boxed as such.

how would you feel about introducing such an implementation for
TrySendError<T>? presumably, <TrySendError<T> as Error>::source() would
return the inner hyper::Error.

SendRequest::try_poll_ready

I've generally adopt the opinion of making errors not constructable outside
of the conditions that they represent.

this point brought to mind another way to solve this:

we could provide a SendRequest::try_poll_ready, like so:

impl<B> SendRequest<B> {
    /// Polls to determine whether this sender can be used yet for a request.
    ///
    /// If the associated connection is closed, this returns an Error.
    pub fn try_poll_ready<T>(
        &mut self,
        cx: &mut Context<'_>
    ) -> Poll<Result<(), TrySendError<T>>> {
        self
            .dispatch
            .poll_ready(cx)
            .map_err(|error| TrySendError { error, message: None })
    }
}

this would provide Service<T>s backed by a SendRequest<B> with a way to
poll for readiness, if they invoke SendRequest::try_send_request() once
the service is called.

this would also preserve this property you described above, and avoid making
a TrySendError<T> constructable by callers.

in this route, we would likely still want the TrySendError: std::error::Error
implementation proposed above, though the TrySendError: From<hyper::Error>
implementation proposed originally within this PR could be avoided.