Skip to content

Commit

Permalink
Fix Pending Request causing timeout (#636)
Browse files Browse the repository at this point in the history
Don't wait for requests that have been not intercepted (`intercepting` is not set) and are not loaded asynchronously (`asyncLoading` is not set) in awaitPageResources() when page is done. Occasionally, it seems some pending requests that only get added via `Network.requestWillBeSent` but never receive a finished/failed message may persist in the pending request list, and will now be discarded.
(Large requests that have a streaming response body will have either `intercepting or `asyncLoading` set and will not be affected)
  • Loading branch information
ikreymer committed Jul 9, 2024
1 parent bfe42ad commit 0038e3a
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 7 deletions.
22 changes: 18 additions & 4 deletions src/util/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,18 +328,19 @@ export class Recorder {
}

handleRequestWillBeSent(params: Protocol.Network.RequestWillBeSentEvent) {
// only handling redirect here, committing last response in redirect chain
// request data stored from requestPaused
const { redirectResponse, requestId, request, type } = params;

const { headers, method, url } = request;

logNetwork("Network.requestWillBeSent", {
requestId,
url,
redirectResponse,
...this.logDetails,
});

// handling redirect here, committing last response in redirect chain
// request data stored from requestPaused
if (redirectResponse) {
this.handleRedirectResponse(params);
} else {
Expand Down Expand Up @@ -808,8 +809,17 @@ export class Recorder {
if (reqresp.payload) {
this.removeReqResp(requestId);
await this.serializeToWARC(reqresp);
// no url, likely invalid
} else if (!reqresp.url) {
// if no url, and not fetch intercept or async loading,
// drop this request, as it was not being loaded
} else if (
!reqresp.url ||
(!reqresp.intercepting && !reqresp.asyncLoading)
) {
logger.debug(
"Removing pending request that was never fetched",
{ requestId, url: reqresp.url, ...this.logDetails },
"recorder",
);
this.removeReqResp(requestId);
}
}
Expand All @@ -825,13 +835,17 @@ export class Recorder {
url: string;
expectedSize?: number;
readSize?: number;
resourceType?: string;
} = { requestId, url };
if (reqresp.expectedSize) {
entry.expectedSize = reqresp.expectedSize;
}
if (reqresp.readSize) {
entry.readSize = reqresp.readSize;
}
if (reqresp.resourceType) {
entry.resourceType = reqresp.resourceType;
}
pending.push(entry);
}

Expand Down
7 changes: 4 additions & 3 deletions src/util/reqresp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ export class RequestResponseInfo {

frameId?: string;

fetch = false;

resourceType?: string;

// TODO: Fix this the next time the file is edited.
Expand All @@ -65,6 +63,9 @@ export class RequestResponseInfo {
readSize: number = 0;
expectedSize: number = 0;

// set to true to indicate request intercepted via Fetch.requestPaused
intercepting = false;

// set to true to indicate async loading in progress
asyncLoading: boolean = false;

Expand All @@ -83,7 +84,7 @@ export class RequestResponseInfo {

this.responseHeadersList = params.responseHeaders;

this.fetch = true;
this.intercepting = true;

this.frameId = params.frameId;
}
Expand Down

0 comments on commit 0038e3a

Please sign in to comment.