Specify "upload body of a request" #449
Conversation
|
|
||
| <li><p>When <var>read</var> is fulfilled with a value that matches with neither of the | ||
| above patterns, or <var>read</var> is rejected, <a lt=terminated for=fetch>terminate</a> the | ||
| ongoing fetch with reason <i>fatal</i>. |
There was a problem hiding this comment.
This seems a little weird since we don't have a handle on the ongoing fetch and it's not passed to this algorithm. Perhaps it's okay for now though, but we should maybe file a follow up issue to see if we can fix that somehow.
| <li> | ||
| <p>When <var>read</var> is fulfilled with an object whose <code>done</code> | ||
| property is false and whose <code>value</code> property is a | ||
| <code>Uint8Array</code> object, run these substeps: |
There was a problem hiding this comment.
Is this the right way of phrasing this, @domenic?
There was a problem hiding this comment.
It seems solid for a web spec. ES-style specs could use more formal internal slot inspection but that doesn't seem necessary here.
There was a problem hiding this comment.
I guess we can make it formal once IDL formalizes promises.
There was a problem hiding this comment.
Oh, I see, I was focusing on the Uint8Array issue, but the "object whose done property is false" part is the imprecise part. In particular, what if obj.done or obj.value is a throwing getter? Let me push a commit to fix.
There was a problem hiding this comment.
Nevermind, these objects are created by the streams, and can never have getters or non-booleans or similar. The only "bad" thing that can happen is value not being a Uint8Array, and we have that covered.
| @@ -215,16 +215,6 @@ for <var>request</var>. | |||
|
|
|||
| <hr> | |||
There was a problem hiding this comment.
This can also be removed it seems if we move this algorithm.
There was a problem hiding this comment.
And can we remove two references to this item in Main Fetch section?
There was a problem hiding this comment.
References to this <hr>? I'm not sure I understand.
There was a problem hiding this comment.
Ah, sorry, I misunderstood.
| @@ -1121,6 +1109,46 @@ or "<code>worker</code>". | |||
| <li><p>Return <var>newRequest</var>. | |||
| </ol> | |||
|
|
|||
| <p>To <dfn export for=request id=concept-request-upload-body>upload body</dfn> of a | |||
There was a problem hiding this comment.
Nit: I would phrase this as "upload a body for a request".
There was a problem hiding this comment.
I think we should avoid "upload" entirely. The protocol specification doesn't use it either and it doesn't make much sense for non-HTTP URLs (although I guess those will fail to function anyway).
There was a problem hiding this comment.
Then which term should we use? In other words, what are the "primitive" operations on the network layer which I can use?
There was a problem hiding this comment.
If read cannot be used, "transmit" would be okay.
|
@yutakahirano I'd appreciate if you could mention what was wrong with "read". And also what the plan is for responses. |
For me, send / upload / transmit are easier to understand the intention. When you read something, you can do anything (including nothing) to it and so I feel it vague. For responses, isn't the 15th step of HTTP-network fetch enough? |
|
@yutakahirano that only covers a response coming from the network. What about a response coming from a service worker? |
It's written in https://w3c.github.io/ServiceWorker/#dom-fetchevent-respondwith. |
|
Interesting. I think a lot of that would be more logically placed in Fetch. Otherwise we have to repeat all that text for foreign fetch service workers. Would you agree with that? |
Yes, but as in a comment, I hope we can simplify the description with pipeTo and transform streams someday (IIUC transform stream is not ready). Is it OK to move the algorithm at that time? |
|
Maybe, it depends on whether that or foreign fetch happens earlier. Can you open a new tracking issue against Fetch for it? (I'm fine not doing it right away.) |
|
I'll also let @domenic have another look before landing this. |
Done: #450 |
...and replace "read a request" with it.
Fixing #441.