Wait for the sender to be ready

[A248]

I adopted the given example in my own code, however, I was receiving occasional error messages because the connection was not ready. This should help API users encountering the same error.

I wonder also if you might consider changing the API to reflect the necessity of using sender.ready() before dispatching a request. Rust, as a language and ecosystem, (as I'm sure you know) often provides compile-time guarantees so that call patterns must reflect contracts. It would be ideal to have a compile-time constraint hat enforces the inability to use a sender before it is ready. Perhaps the handshake method should return (impl Future<Output=Sender>, Connection) instead of simply (Sender, Connection).

[oddgrd]

Hey, thanks for the PR! I was under the impression that you didn't have to check if the sender is ready on the first request, just before sending subsequent requests on the same sender. Did you have to check if the sender was ready for the first request on a sender, or were you sending multiple in your code? Also, while I think the idea of forcing the user to use this correctly is great, in your suggestion of returning (impl Future<Output=Sender>, Connection) from handshake, that would only force the user to check if the sender is ready on the first request on that sender.

No matter what we conclude on the above, I think we can safely conclude that the requirement to check if the sender is ready should be documented in the guide. If we need to check the sender for the first request then we can do that and add a comment, if it's only needed for subsequent requests then we can add another request to the example. We're also planning to write a more advanced client guide, but we haven't gotten around to it yet (suggestions for what to cover are most welcome!).

[A248]

I was under the impression that you didn't have to check if the sender is ready on the first request, just before sending subsequent requests on the same sender. Did you have to check if the sender was ready for the first request on a sender, or were you sending multiple in your code?

Indeed, I was sending multiple requests. Yes, that explains matters. You may close this PR as-is, since it shouldn't be necessary for the majority of users presumably making single HTTP requests.

It may still be possible to prevent API users from making the mistake of forgetting to await sender.ready() before sending additional requests. Leveraging the type system would be possible: for example, by consuming self in send_request and yielding the sender later. However, you may decide whether enforcing this requirement via the type system is overly complicated. Personally, I love it, and would write a PR if you told me to.

Perhaps another solution would be to include a call to self.ready().await in the send_request implementation. That is, automaticallly call and await ready() so that callers do not forget it themselves. The overhead imposed seems minimal in the case where awaiting ready() is unnecessary, since Sender#poll_ready/Giver#poll_want involves only a sequentially-consistent load (SeqCst) and returning objects on the stack, with no heap allocation.

The alternative, also acceptable, is to keep the API as-is.

[oddgrd]

Ah, okay, that makes sense then! I think this PR is still worthwhile, since I've seen this cause confusion in the past as well. I think we should just refactor it to also send another request, we can use another endpoint from httpbin (e.g. http://httpbin.org/status/418), and explain in a comment that on subsequent requests on the same sender checking if the sender is ready is necessary.

Suggestions for improving the API are most welcome, and this is a good time for it, with the release of 1.0 not being completed. I would say that imposing a minimal overhead (what the overhead will be should be benchmarked) might not be desirable since this will be a low-level building block for higher level client libraries. If ease of use is a priority, one could use a higher level client like the high-level 0.14 client (which is also available in hyper-util for 1.0, but it will be replaced by a more modular high level client eventually), or reqwest.

But please don't be discouraged by my feedback, I would recommend opening an issue with your suggestions in the main hyper repository to get feedback from others with more insight. I think they're both interesting and worth considering!