"Return a network error" doesn't seem to go through "process response" #1164
Comments
|
Thanks! This is a regression from the "abort when" language that was added. We probably should not use that language before going in parallel. That does assume that fetch and terminate are invoked from the same thread, but that's hopefully reasonable? |
This is sort-of related to #536, since there are specs which call fetch in parallel. I guess we could pretend that they don't for now, then figure out a way that fetch should be called in parallel. If we're assuming that fetch must be called from the main thread, I can just remove the abort language from those parts. Sound good? |
|
I will take care of this as part of that issue. My idea is that for "in parallel" invocation of fetch you still have to give callbacks that will be called in a parallel queue that fetch creates. And there is no more returning of a response to the caller. |
|
Sounds good. I'll remove the abort stuff. |
|
No I won't (following IRC chat) |
This makes some rather big changes: * Requests no longer have a synchronous flag. Blocking a thread is now up to the caller. * Fetch therefore no longer returns a response directly. In parallel callers will need to pass in "callbacks" that are either invoked on a parallel queue or on an event loop. * To hold onto these callbacks as well as some other information a fetch params struct is now passed around the various fetch algorithms. This will allow for cleanup around termination and aborting in the future. Potentially some bookkeeping state from request can move there going forward. * There's a dedicated navigate-redirect fetch algorithm for HTML as the HTTP-redirect fetch algorithm now wants a fetch params instance. * Some allowance for aborting early on in fetch was removed as all that is run synchronously with the code that invoked fetch to begin with. Closes #1164. * Algorithms that needed to be adjusted were changed to use the new Infra patterns for parameters. I also tried to improve naming, e.g., makeCORSPreflight rather than CORS-preflight flag. * "process response done" was removed as it seemed redundant with "response response end-of-body"; possible leftover from trailers. * Removed duplicate task queueing at the end of main fetch for request's body. Fixes #536.
This makes some rather big changes: * Requests no longer have a synchronous flag. Blocking a thread is now up to the caller. * Fetch therefore no longer returns a response directly. In parallel callers will need to pass in "callbacks" that are either invoked on a parallel queue or on an event loop. * To hold onto these callbacks as well as some other information a fetch params struct is now passed around the various fetch algorithms. This will allow for cleanup around termination and aborting in the future. Potentially some bookkeeping state from request can move there going forward. * There's a dedicated navigate-redirect fetch algorithm for HTML as the HTTP-redirect fetch algorithm now wants a fetch params instance. This also returns a response which fixes a problem where HTTP-redirect fetch would return a network error that was then ignored. * Some allowance for aborting early on in fetch was removed as all that is run synchronously with the code that invoked fetch to begin with. Closes #1164. * Algorithms that needed to be adjusted were changed to use the new Infra patterns for parameters. I also tried to improve naming, e.g., makeCORSPreflight rather than CORS-preflight flag. * "process response done" was removed as it seemed redundant with "response response end-of-body"; possible leftover from trailers. * Removed duplicate task queuing at the end of main fetch for request's body. Fixes #536.
Consider what happens when we reach step 10 of
fetch(), which calls the "fetch" algorithm.The only way the promise ever gets resolved or rejected is once "process response" gets invoked.
However, it's possible to (synchronously) reach a step which reads "Return a network error", e.g. fetch step 2.3, or main fetch step 3.2, 3.3. In which case "process response" never gets invoked, so in theory the promise doesn't settle.
The text was updated successfully, but these errors were encountered: