Body-related errors of requests over HTTP/2 are swallowed by Hyper

[nox]

Errors emitted by the HttpBody impl sent as a request body through the HTTP/2 client are not returned to the caller.

hyper/src/proto/h2/client.rs

Lines 243 to 247 in 8a05f8e

	let mut pipe = Box::pin(PipeToSendStream::new(body, body_tx)).map(|res| {
	if let Err(e) = res {
	debug!("client request body error: {}", e);
	}
	});

I understand that this is because contrary to HTTP/1, the request body can be sent at the same as the response is received, so Hyper doesn't wait before returning the response, but that also means that the errors completely disappear and Hyper only notices something went wrong when trying to receive the response body, at which point it returns to the caller an error about the stream being reset.

Any ideas on how to improve that? Maybe we should implement an Extension that lets us await on a one-time channel to get the error?

[seanmonstar]

Are they completely swallowed? I think in all cases, they will bubble out the other sides. For instance, if the error comes from the h2 SendStream, it's likely from a stream or connection error. That will appear when polling for the response data. If the error is from the HttpBody, it will force sending a reset code, which the generic error will appear again when polling for the response data.

Well, I suppose if sending the body is the only thing left, you've already received the response and the full response body, then there'd be no way to see the error. Is that the situation you mean?

[nox]

By swallowed I mean we lose the exact reason that caused the error. Our system emits some errors on purpose and those should not be logged as they are completely expected behaviour, unfortunately they surface as a generic error as you said.

[nox]

What do you think of introducing a public extension in Hyper, something like:

type BoxError = Box<dyn Error + Send + Sync>;

fn request_body_poll_error_extension() -> (RequestBodyPollErrorExtension, RequestBodyPollErrorReceiver);

impl Future for RequestBodyPollErrorReceiver {
    type Output = Result<BoxError, Cancelled>;
}

In the implementation, RequestBodyPollErrorExtension and RequestBodyPollErrorReceiver would just be wrappers over Sender<BoxError> and Receiver<BoxError> from futures::channel::oneshot.

To use that extension, the user calls request_body_poll_error_extension and installs the returned RequestBodyPollErrorExtension in the request's extensions before passing it to the Hyper client.

The h1 support then completely disregards the extension, as it returns request body poll errors directly when sending the request before receiving the response.

On the h2 side, things get interesting.

First, we can patch this snippet to make pipe be a future that properly returns the error without silencing it:

hyper/src/proto/h2/client.rs

Lines 243 to 247 in 8a05f8e

	let mut pipe = Box::pin(PipeToSendStream::new(body, body_tx)).map(|res| {
	if let Err(e) = res {
	debug!("client request body error: {}", e);
	}
	});

Then, in this snippet, we make the eagerly poll fulfill the callback directly if an error has been returned already:

hyper/src/proto/h2/client.rs

Lines 251 to 252 in 8a05f8e

	match Pin::new(&mut pipe).poll(cx) {
	Poll::Ready(_) => (),

If the eager poll is pending, we look up the RequestBodyPollErrorExtension and we wrap pipe in a new future that will send any poll error on the onetime channel, there:

hyper/src/proto/h2/client.rs

Lines 259 to 264 in 8a05f8e

	let pipe = pipe.map(move |x| {
	drop(conn_drop_ref);
	drop(ping);
	x
	});
	self.executor.execute(pipe);

If the Hyper client returns Ok(response) to the user, the user is then free to check the RequestBodyPollErrorReceiver it kept, either on error while receiving the response body, or until the receiver returns Ok(error) (meaning the body was not sent successfully) or Err(Cancelled) (meaning the body was successfully sent).

---

That last part makes me wonder: how can a user even check that their request body was successfully sent over h2, if they are not polling that the response body was properly received?

[nox]

Given the last sentence of my previous comment, maybe the actual mechanism we need is some sort of "request body send observer", that we can await on until the request body was successfully grabbed and sent or an error was returned by poll_data.