Skip to content

Conversation

juliandescottes
Copy link
Contributor

@juliandescottes juliandescottes commented Sep 26, 2025

Fixes #748

  • Adds another entry point expected to be called from the fetch spec clone network request body (alternatively could be called explicitly from beforeRequestSent for now)
  • Extracts a generic algorithm maybe collect network data from maybe collect network response body
  • Update maybe collect network response body to call maybe collect network data
  • Adds a new algorithm maybe collect network request body called from the beforeRequestSent steps

Otherwise this mostly reuses the logic added for response body collection. Commands do not need to be udpated AFAICT


Preview | Diff

@juliandescottes juliandescottes force-pushed the issue748-request-body branch 3 times, most recently from 850fc7d to 1e7d64d Compare September 26, 2025 11:08
index.bs Outdated
</div>

<div algorithm>
To <dfn export>clone network request body</dfn> given |request|:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd propose to use typed arguments for externally used hooks: #1004

Suggested change
To <dfn export>clone network request body</dfn> given |request|:
To <dfn export>clone network request body</dfn> given [=/request=] |request|:

Copy link
Contributor

@sadym-chromium sadym-chromium Sep 26, 2025

Choose a reason for hiding this comment

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

oh, despite the comment, it is not used externally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In theory it should be called from fetch, but yes at the moment it's blocked on whatwg/fetch#1540

Alternatively I can rewrite the PR to trigger this algorithm from the beforeRequestSent event handler if you prefer?

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is not a blocker (I already approved the PR).

I cannot see in the fetch RP any reference to this hook. Maybe this comment meant to underline that this is a part of the WebDriver BiDi before request sent hook, which is invoked by the fetch spec in fetch RP?

Copy link
Contributor Author

@juliandescottes juliandescottes Sep 29, 2025

Choose a reason for hiding this comment

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

Oh no you're right, I haven't updated James' PR. Not quite sure how to do that but I can give it a try.

And in theory, WebDriver BiDi before request sent is a bit too late, because other consumers might have read the stream already, so we really need to clone it as soon as possible.

@juliandescottes
Copy link
Contributor Author

Thanks for the review! I addressed the suggestions to add types. Let me know if you want to switch to an internal trigger for the algorithm for now. I'll discuss internally to make the fetch PR move.

@juliandescottes juliandescottes merged commit 092df07 into w3c:main Oct 1, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support reading request body
3 participants