Preparing request for async

[KiChjang]

cc #4576
This is a commit to refactor all of the existing request code to support async listeners in the future

[hoppipolla-critic-bot]

Critic review: https://critic.hoppipolla.co.uk/r/5206

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

[jdm]

I've started reviewing this. Apologies for the extremely delay!

[jdm]

Huh, I was convinced that this was a bigger PR than it actually is. I feel worse about not having got around to it now. Anyways, onwards!

This feels like an unusual way to go about this. IIRC there are no actual consumers of the fetch code right now, so creating this intermediate state of conceptually allowing both async and sync fetches to occur doesn't seem valuable to me. Additionally, I'm not really enthusiastic about overloading the Result type this way - while we can argue that a network error result can be represented by an Err, I think that an Err result should really indicate that our fetch infrastructure messed up and it's impossible to determine the result of a network request instead.

Here's my proposal for how to move forward - we go full async all the time. We also don't try to shoehorn in the AsyncResponseListener stuff into the interface, since that operates at the level of the network protocol: "the response has started", "here's some data", "no more response notifications will arrive". Instead, let's make an AsyncFetchListener interface that has listener methods for whatever makes sense for Fetch - presumably one that takes a Response argument, at least. We can then call that directly instead of returning Result values, and we can create a bridge between the AsyncResponseListener infrastructure if necessary. See the code in cors.rs that does a similar thing for prior art. Does that make sense?

Given the very different path forward that I've proposed, I don't see much point in leaving this particular PR open. I'm glad you started this work so that we could have this conversation, though!

---

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{components/net/fetch/request.rs, line 19 [r1] (raw file):}
When would we have a successful fetch result with no response?

---

Comments from the review on Reviewable.io

[Manishearth]

Note that Rust may get ES6-style generator support (eddy is working on an rfc), so if we just use async everywhere and simulate sync via blocking, we can later easily convert the API into one which is both sync and async with zero cost for sync. Perhaps.

I'd love our entire net infra to move to a generator-based model.