WHATWG
2022-12-11
plaintext logs
prev next
source on github
12:37
<Andreas Kling>
howdy folks! I'm working on implementing <link> on top of fetch primitives, and I've hit a problem:
12:37
<Andreas Kling>
fast-forwarding through all the setup and most of fetch itself, once we get to fully read (https://fetch.spec.whatwg.org/#body-fully-read) step 5, we're asked to react (https://webidl.spec.whatwg.org/#dfn-perform-steps-once-promise-is-settled) which ends up calling PerformPromiseThen (https://tc39.es/ecma262/#sec-performpromisethen) which calls HostMakeJobCallback (https://html.spec.whatwg.org/multipage/webappapis.html#hostmakejobcallback)
12:37
<Andreas Kling>
in HostMakeJobCallback it wants the incumbent settings object, but since I'm just reacting to the HTML tree-build, there's no script-having execution context present. this breaks the assertion in incumbent settings object step 2.1 (https://html.spec.whatwg.org/multipage/webappapis.html#incumbent-settings-object)
12:37
<Andreas Kling>
what am I missing here? was something above supposed to do a "prepare to run callback" (and thus populating the backup incumbent settings object stack)? or could this be a spec issue? (if the spec assumes that fetch operations always happen within some kind of JS execution context)
12:58
<Noam Rosenthal>
Oh hello Andreas Kling
12:58
<Noam Rosenthal>
Fetching always assumes there's an environment settings object
12:58
<Noam Rosenthal>
in this case, the document. <link> always fetches with the document as the environment settings object
12:59
<Noam Rosenthal>
the whole thing with promises is a bit out-of-spec, <link> doesn't work on top of the JS fetch()
13:00
<Noam Rosenthal>
specifically it's the request's client (https://fetch.spec.whatwg.org/#concept-request-client)
13:01
<Andreas Kling>
hmm okay, so am I doing something wrong and I shouldn't end up in "fully read a body" in the first place?
13:02
<Andreas Kling>
or what do you mean the thing with promises is a bit out-of-spec? :)
13:02
<Noam Rosenthal>
oh I see what you mean, I forgot about that bit
13:25
<Noam Rosenthal>

Andreas Kling: following the spec:
PerformPromiseThen 10.b: Let fulfillJob be NewPromiseReactionJob(fulfillReaction, value).
NewPromiseReactionJob 1.e: let handlerResult be Completion(HostCallJobCallback(handler, undefined, « argument »)) (https://tc39.es/ecma262/#sec-newpromisereactionjob
)
HostCallJobCallback(callback, V, argumentsList): 3.: Prepare to run a callback with incumbent settings. https://html.spec.whatwg.org/multipage/webappapis.html#hostcalljobcallback

The latter pushes to the backup stack

13:25
<Noam Rosenthal>
https://html.spec.whatwg.org/multipage/webappapis.html#prepare-to-run-a-callback (1.:Push settings onto the backup incumbent settings object stack.)
13:27
<Noam Rosenthal>
seems super-complex, I've never touched this part of the spec TBH
13:27
<Andreas Kling>
earlier in PerformPromiseThen step 4.a, we call HostMakeJobCallback(https://html.spec.whatwg.org/multipage/webappapis.html#hostmakejobcallback), that's when the assertion fails
13:28
<Andreas Kling>
agree this is very complex
13:56
<Noam Rosenthal>
Andreas Kling: some of it is a bit out of my comfort zone, but I think that if Fetch had a prepare-to/cleanup-after callback pair before/after the promise, with the task destination as the parameter, it would create the right result. I'm sure other people here can validate better though
14:05
<Andreas Kling>
hmm all right. curiously, we already do basically that as a workaround in our implementation of HostEnqueuePromiseJob since a while back: https://github.com/SerenityOS/serenity/commit/25909dcc051db82bf7ab4e554a8eb8ec8b173863
14:06
<Andreas Kling>
would be great to have spec clarity on all relevant situations here. understood that it's outside your comfort zone, thanks for the help so far! :)
14:07
<Noam Rosenthal>
Yea I agree it needs clarity. If this is validated I'd be happy to help with a Fetch PR
14:08
<Andreas Kling>
much appreciated!
14:25
<Noam Rosenthal>
Andreas Kling: I think actually a cleaner version would be to create a read request (https://streams.spec.whatwg.org/#read-request) in fully read a body (https://fetch.spec.whatwg.org/#body-fully-read) and not deal with promises at all
14:28
<Andreas Kling>
I'm not familiar enough with streams (yet) to tell if that's a good idea, but I welcome any simplification :)
14:30
<Noam Rosenthal>
I believe Mattias Buelens can shed some light here
17:10
<Mattias Buelens>
Hmm, this is indeed a bit awkward. We have "read a chunk" which takes a read request with chunk steps and error steps, which nicely keeps promises out of your way. However, "read all bytes" (https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-all-bytes) still returns a promise... I'd say the better solution would be to refactor that abstract op in Streams, so it takes "success steps" and "error steps" instead.