Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/bidi/bidiBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class BidiBrowser extends Browser {
});

await browser._browserSession.send('network.addDataCollector', {
dataTypes: [bidi.Network.DataType.Response],
dataTypes: [bidi.Network.DataType.Request, bidi.Network.DataType.Response],
maxEncodedDataSize: 20_000_000, // same default as in CDP: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/inspector/inspector_network_agent.cc;l=134;drc=4128411589187a396829a827f59a655bed876aa7
});

Expand Down
25 changes: 14 additions & 11 deletions packages/playwright-core/src/server/bidi/bidiNetworkManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class BidiNetworkManager {
eventsHelper.removeEventListeners(this._eventListeners);
}

private _onBeforeRequestSent(param: bidi.Network.BeforeRequestSentParameters) {
private async _onBeforeRequestSent(param: bidi.Network.BeforeRequestSentParameters) {
if (param.request.url.startsWith('data:'))
return;
const redirectedFrom = param.redirectCount ? (this._requests.get(param.request.request) || null) : null;
Expand All @@ -79,7 +79,8 @@ export class BidiNetworkManager {
route = new BidiRouteImpl(this._session, param.request.request);
}
}
const request = new BidiRequest(frame, redirectedFrom, param, route);
const requestData = await getNetworkData(this._session, param.request, bidi.Network.DataType.Request).catch(() => null);
Copy link
Member

Choose a reason for hiding this comment

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

We cannot do asynchronous staff in an event handler, the dispatcher is not waiting for the listener to complete and will fire subsequent events. With the current implementation the request may not be in the map when _onResponseStarted arrives and we'll miss the response.

Ideally we'd have an asynchronous request.body() and not have to wait here at all, but all the postData* methods in playwright API today are synchronous (which is a regret).

Alternatively, we could buffer all network events for a given request. However, to preserve their relative order we’d effectively need to buffer all network events, which complicates things. It also means unrelated events could interleave differently. E.g. a console message might appear before a network request. If we don't buffer up all the requests and wait for the request bodies independently, users may see sub-resource requests before the main request.

Considering all the above, we should add an async getter for the request body and have Bidi implement only that. Let's postpone this change until that is done. Filed #38150 for this.

const request = new BidiRequest(frame, redirectedFrom, param, route, requestData);
this._requests.set(request._id, request);
this._page.frameManager.requestStarted(request.request, route);
}
Expand All @@ -88,11 +89,7 @@ export class BidiNetworkManager {
const request = this._requests.get(params.request.request);
if (!request)
return;
const getResponseBody = async () => {
const { bytes } = await this._session.send('network.getData', { request: params.request.request, dataType: bidi.Network.DataType.Response });
const encoding = bytes.type === 'base64' ? 'base64' : 'utf8';
return Buffer.from(bytes.value, encoding);
};
const getResponseBody = async () => (await getNetworkData(this._session, params.request, bidi.Network.DataType.Response))!;
const timings = params.request.timings;
const startTime = timings.requestTime;
function relativeToStart(time: number): number {
Expand Down Expand Up @@ -236,14 +233,12 @@ class BidiRequest {
// store the first and only Route in the chain (if any).
_originalRequestRoute: BidiRouteImpl | undefined;

constructor(frame: frames.Frame, redirectedFrom: BidiRequest | null, payload: bidi.Network.BeforeRequestSentParameters, route: BidiRouteImpl | undefined) {
constructor(frame: frames.Frame, redirectedFrom: BidiRequest | null, payload: bidi.Network.BeforeRequestSentParameters, route: BidiRouteImpl | undefined, postData: Buffer | null) {
this._id = payload.request.request;
if (redirectedFrom)
redirectedFrom._redirectedTo = this;
// TODO: missing in the spec?
const postDataBuffer = null;
this.request = new network.Request(frame._page.browserContext, frame, null, redirectedFrom ? redirectedFrom.request : null, payload.navigation ?? undefined, payload.request.url,
resourceTypeFromBidi(payload.request.destination, payload.request.initiatorType, payload.initiator?.type), payload.request.method, postDataBuffer, fromBidiHeaders(payload.request.headers));
resourceTypeFromBidi(payload.request.destination, payload.request.initiatorType, payload.initiator?.type), payload.request.method, postData, fromBidiHeaders(payload.request.headers));
// "raw" headers are the same as "provisional" headers in Bidi.
this.request.setRawRequestHeaders(null);
this.request._setBodySize(payload.request.bodySize || 0);
Expand Down Expand Up @@ -390,3 +385,11 @@ function resourceTypeFromBidi(requestDestination: string, requestInitiatorType:
default: return 'other';
}
}

async function getNetworkData(session: BidiSession, request: bidi.Network.RequestData, dataType: bidi.Network.DataType): Promise<Buffer | null> {
if (dataType === 'request' && !request.bodySize)
return null;
const { bytes } = await session.send('network.getData', { request: request.request, dataType });
const encoding = bytes.type === 'base64' ? 'base64' : 'utf8';
return Buffer.from(bytes.value, encoding);
}
Original file line number Diff line number Diff line change
Expand Up @@ -1383,6 +1383,7 @@ export namespace Network {
}
export namespace Network {
export const enum DataType {
Request = 'request',
Response = 'response',
}
}
Expand Down
Loading