Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use internalResponse at end of fetch handover #1645

Merged
merged 5 commits into from
May 22, 2023
Merged

Conversation

noamr
Copy link
Contributor

@noamr noamr commented Apr 30, 2023

This ensures that callers of processResponseEndOfBody and processResponseConsumeBody would receive the correct body and response without additional steps.

Closes #1512

  • At least two implementers are interested (and none opposed):
  • Tests are written and can be reviewed and commented upon at:
  • Implementation bugs are filed:
    • Chromium: …
    • Gecko: …
    • WebKit: …
    • Deno (not for CORS changes): …
  • MDN issue is filed: …

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks mostly okay, except for some editorial work. Also, we should update the warning (as per #1512 (comment)) to indicate you might get bytes back from an opaque response and thus you have to tread carefully when not using CORS.

fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
@noamr
Copy link
Contributor Author

noamr commented May 8, 2023

This looks mostly okay, except for some editorial work. Also, we should update the warning (as per #1512 (comment)) to indicate you might get bytes back from an opaque response and thus you have to tread carefully when not using CORS.

Regarding the comment, I want to see where we're going with #1614 before I create a merge conflict in the how-to-fetch section... WDYT?

annevk added a commit that referenced this pull request May 8, 2023
@annevk
Copy link
Member

annevk commented May 8, 2023

Fair, I've taken a pass at that PR and will try to do a pass at least daily until it's done.

@annevk
Copy link
Member

annevk commented May 11, 2023

It has landed. This should now be safe to rebase and then you can add the relevant text.

@noamr
Copy link
Contributor Author

noamr commented May 11, 2023

It has landed. This should now be safe to rebase and then you can add the relevant text.

Thanks! On it

noamr added 4 commits May 11, 2023 19:52
This ensures that callers of `processResponseEndOfBody` and
`processResponseConsumeBody` would receive the correct body and
response without additional steps.

Closes whatwg#1512
@noamr
Copy link
Contributor Author

noamr commented May 11, 2023

It has landed. This should now be safe to rebase and then you can add the relevant text.

Done, please take a look if this warning is what you had in mind.

@annevk annevk merged commit 9003266 into whatwg:main May 22, 2023
2 checks passed
annevk added a commit that referenced this pull request May 25, 2023
Upon reviewing #1645 again it struck me that the first argument to processResponseConsumeBody should not be the internal response. The caller would still have all the same capabilities when it's not and it's safer if it's not.
annevk added a commit that referenced this pull request May 28, 2023
Upon reviewing #1645 again it struck me that the first argument to processResponseConsumeBody should not be the internal response. The caller would still have all the same capabilities when it's not and it's safer if it's not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

The end of fetch response handover needs to operate on the internal response
2 participants