Re: [whatwg/fetch] Move `finalize and report timing` to controller (PR #1413)

> Thanks, these simplifications have helped my understanding. I still have some high-level comments here before going into the details.
> 
> 1. There's at least two other places that create a fetch params and they don't pass in a controller:
>    
>    * https://fetch.spec.whatwg.org/#ref-for-fetch-params%E2%91%A0%E2%91%A0 (HTTP-network-or-cache fetch)
>    * https://fetch.spec.whatwg.org/#ref-for-fetch-params%E2%91%A0%E2%91%A2 (CORS-preflight fetch)

OK, I believe I can bring back the default.

> 2. I'm starting to wonder why the fetch algorithm does not get to call conclude on the same task that sets the "done flag". Any additional metadata required could be passed in on the request concept.

That was the initial idea. Where the current version makes some sense is:
- Some times timing is not reported, e.g. cross-origin style imports don't report their subresources. I guess this information is known at request time though and we can pass "do not report" instead of an initiator etc..
- We went down this road to allow callers to decide whether to report timing based on the response. In essence, this is not used much, but some callers do decide that a response is a network error - e.g. the script caller checks for particular mime-types. I believe that perhaps it's not necessary for those semantic errors to count as network errors in resource timing, and perhaps it would be much simpler to go back to reporting resource timing automatically.

> 3. I'm not sure I understand your last comment in [Move `finalize and report timing` to controller #1413 (comment)](https://github.com/whatwg/fetch/pull/1413#discussion_r854322363) but it seems we now ended up with "body info" as the struct name, but the struct members still duplicate part of that name. Could you perhaps try another explanation how this new struct avoids issues in the future? I'm still not entirely sold on it.

It would let avoid extra piping-through patches for navigation timing every time we add a timing parameter, like these two PRs:
https://github.com/w3c/navigation-timing/pull/176 and https://github.com/whatwg/html/pull/7722. Whenever we add a response-associated timing bit it has to somehow be patched through the navigation-timing and it's laborious to keep adding an HTML/nav-timing patch each time where the patch doesn't have real content. 

> 4. Why do both the controller and fetch params hold access to timing info? I think we should deduplicate that to avoid confusion, no? But also, if we do 2 then the controller might not have to hold timing info at all.

I think the controller doesn't have to hold timing at all anyway with the conclude steps mechanism.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1413#issuecomment-1135754742

You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/fetch/pull/1413/c1135754742@github.com>

Received on Tuesday, 24 May 2022 10:43:41 UTC