Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore(bidi): add support for fetching request bodies#38212

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
hbenl wants to merge1 commit intomicrosoft:main
base:main
Choose a base branch
Loading
fromhbenl:async-request-bodies

Conversation

@hbenl
Copy link
Contributor

I have also updated the tests to userequest.body* instead ofrequest.postData* except for the following:

  • I changed the tests intests/page/network-post-data.spec.ts to be skipped for BiDi because these tests are mirrored by those intests/page/network-request-body.spec.ts
  • I didn't changetests/page/page-request-fallback.spec.ts because it only tests overridden request bodies which are always available synchronously

Fixes the following tests in Firefox:

  • tests/page/network-request-body.spec.ts:
    • "should return correct request body buffer for utf-8 body"
    • "should return request body w/o content-type"
    • "should throw on invalid JSON in post data"
    • "should return body for PUT requests"
    • "should get request body for file/blob"
    • "should get request body for navigator.sendBeacon api calls"
  • tests/page/page-network-request.spec.ts:
    • "should parse the data if content-type is application/x-www-form-urlencoded"
    • "should parse the data if content-type is application/x-www-form-urlencoded; charset=UTF-8"
    • "should parse the json post data"
    • "should return multipart/form-data"
    • "should return postData"
    • "should work with binary post data"
    • "should work with binary post data and interception"
  • tests/page/page-request-intercept.spec.ts:
    • "request.postData is not null when fetching FormData with a Blob"
    • "should intercept multipart/form-data request body"
  • tests/page/page-route.spec.ts:
    • "should intercept when postData is more than 1MB"

@hbenlhbenl requested a review fromyury-sNovember 13, 2025 16:50
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

asyncbody():Promise<Buffer|null>{
if(this._overrides?.postData)
returnthis._overrides?.postData;
if(typeofthis._postData==='function')
Copy link
Member

Choose a reason for hiding this comment

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

Let's keeppostDataBuffer as is and introduce a separatebodyCallback which will be non-null only in Bidi for now. This method should become something like this:

if(this._overrides?.postData)returnthis._overrides?.postData;if(this._bodyCallback)returnawaitthis._bodyCallback();returnthis._postData;

letintercepted=false;

awaitpage.route(regexp,(route,request)=>{
awaitpage.route(regexp,async(route,request)=>{
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to change route handlers to async as it may change the order of the network events. At line 270 of this test we don't await continue to keep the callback sync.

import{testasit,expect}from'./pageTest';

it('should return correct postData buffer for utf-8 body',async({ page, server})=>{
it('should return correct postData buffer for utf-8 body',async({ page, server, channel})=>{
Copy link
Member

Choose a reason for hiding this comment

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

You can skip entire file by adding top levelit.skip(({ channel }) => channel?.startsWith('bidi-chrom') || channel?.startsWith('moz-firefox'), 'request.postData is not supported with BiDi'); instead of adding it to every test.

@github-actions
Copy link
Contributor

Test results for "MCP"

2 flaky⚠️ [chrome] › mcp/cdp.spec.ts:24 › cdp server `@mcp-windows-latest`
⚠️ [chrome] › mcp/cdp.spec.ts:35 › cdp server reuse tab `@mcp-windows-latest`

2430 passed, 116 skipped


Mergeworkflow run.

@github-actions
Copy link
Contributor

Test results for "tests 1"

1 failed
❌ [playwright-test] › runner.spec.ts:118 › should ignore subprocess creation error because of SIGINT@macos-latest-node18-2

2 flaky⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1079 › cli codegen › should not throw csp directive violation errors `@firefox-ubuntu-22.04-node18`
⚠️ [firefox-page] › page/page-wait-for-function.spec.ts:104 › should work with strict CSP policy `@firefox-ubuntu-22.04-node18`

40312 passed, 787 skipped


Mergeworkflow run.

@hbenlhbenl requested a review fromyury-sNovember 18, 2025 11:26
@yury-s
Copy link
Member

Unfortunately the new API was reverted in#38281 as we couldn't find a single use that it was fixing in Playwright today. I tried switching toNetwork.getRequestPostData in Chromium in hope that it would fixthe problem for blob/file requests ormultipart uploads, but unfortunately it doesn't. We can reconsider adding the async APIs if it fixes some of the scenarios, but looks like this needs to be fixed on Chromium/WebKit side first.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@yury-syury-sAwaiting requested review from yury-s

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hbenl@yury-s

[8]ページ先頭

©2009-2025 Movatter.jp