Skip to content

chore: make fetch progress "strict" #36318

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 20, 2025
Merged

chore: make fetch progress "strict" #36318

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 17 additions & 32 deletions packages/playwright-core/src/server/fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { BrowserContext, verifyClientCertificates } from './browserContext';
import { Cookie, CookieStore, domainMatches, parseRawCookie } from './cookieStore';
import { MultipartFormData } from './formData';
import { SdkObject } from './instrumentation';
import { ProgressController } from './progress';
import { isAbortError, ProgressController } from './progress';
import { getMatchingTLSOptionsForOrigin, rewriteOpenSSLErrorIfNeeded } from './socksClientCertificatesInterceptor';
import { httpHappyEyeballsAgent, httpsHappyEyeballsAgent, timingForSocket } from './utils/happyEyeballs';
import { Tracing } from './trace/recorder/tracing';
Expand Down Expand Up @@ -84,11 +84,12 @@ export type APIRequestFinishedEvent = {

type SendRequestOptions = https.RequestOptions & {
maxRedirects: number,
deadline: number,
headers: HeadersObject,
__testHookLookup?: (hostname: string) => LookupAddress[]
};

type SendRequestResult = Omit<channels.APIResponse, 'fetchUid'> & { body: Buffer };

export abstract class APIRequestContext extends SdkObject {
static Events = {
Dispose: 'dispose',
Expand Down Expand Up @@ -185,16 +186,11 @@ export abstract class APIRequestContext extends SdkObject {
let maxRedirects = params.maxRedirects ?? (defaults.maxRedirects ?? 20);
maxRedirects = maxRedirects === 0 ? -1 : maxRedirects;

const timeout = params.timeout;
const deadline = timeout && (monotonicTime() + timeout);

const options: SendRequestOptions = {
method,
headers,
agent,
maxRedirects,
timeout,
deadline,
...getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, requestUrl.origin),
__testHookLookup: (params as any).__testHookLookup,
};
Expand All @@ -205,10 +201,10 @@ export abstract class APIRequestContext extends SdkObject {
const postData = serializePostData(params, headers);
if (postData)
setHeader(headers, 'content-length', String(postData.byteLength));
const controller = new ProgressController(metadata, this);
const controller = new ProgressController(metadata, this, 'strict');
const fetchResponse = await controller.run(progress => {
return this._sendRequestWithRetries(progress, requestUrl, options, postData, params.maxRetries);
}, timeout);
}, params.timeout);
const fetchUid = this._storeResponseBody(fetchResponse.body);
this.fetchLog.set(fetchUid, controller.metadata.log);
const failOnStatusCode = params.failOnStatusCode !== undefined ? params.failOnStatusCode : !!defaults.failOnStatusCode;
Expand Down Expand Up @@ -252,10 +248,10 @@ export abstract class APIRequestContext extends SdkObject {
return cookies;
}

private async _updateRequestCookieHeader(url: URL, headers: HeadersObject) {
private async _updateRequestCookieHeader(progress: Progress, url: URL, headers: HeadersObject) {
if (getHeader(headers, 'cookie') !== undefined)
return;
const contextCookies = await this._cookies(url);
const contextCookies = await progress.race(this._cookies(url));
// Browser context returns cookies with domain matching both .example.com and
// example.com. Those without leading dot are only sent when domain is strictly
// matching example.com, but not for sub.example.com.
Expand All @@ -266,31 +262,33 @@ export abstract class APIRequestContext extends SdkObject {
}
}

private async _sendRequestWithRetries(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer, maxRetries?: number): Promise<Omit<channels.APIResponse, 'fetchUid'> & { body: Buffer }>{
private async _sendRequestWithRetries(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer, maxRetries?: number): Promise<SendRequestResult>{
maxRetries ??= 0;
let backoff = 250;
for (let i = 0; i <= maxRetries; i++) {
try {
return await this._sendRequest(progress, url, options, postData);
} catch (e) {
if (isAbortError(e))
throw e;
e = rewriteOpenSSLErrorIfNeeded(e);
if (maxRetries === 0)
throw e;
if (i === maxRetries || (options.deadline && monotonicTime() + backoff > options.deadline))
if (i === maxRetries)
throw new Error(`Failed after ${i + 1} attempt(s): ${e}`);
// Retry on connection reset only.
if (e.code !== 'ECONNRESET')
throw e;
progress.log(` Received ECONNRESET, will retry after ${backoff}ms.`);
await new Promise(f => setTimeout(f, backoff));
await progress.wait(backoff);
backoff *= 2;
}
}
throw new Error('Unreachable');
}

private async _sendRequest(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer): Promise<Omit<channels.APIResponse, 'fetchUid'> & { body: Buffer }>{
await this._updateRequestCookieHeader(url, options.headers);
private async _sendRequest(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer): Promise<SendRequestResult>{
await this._updateRequestCookieHeader(progress, url, options.headers);

const requestCookies = getHeader(options.headers, 'cookie')?.split(';').map(p => {
const [name, value] = p.split('=').map(v => v.trim());
Expand All @@ -305,7 +303,7 @@ export abstract class APIRequestContext extends SdkObject {
};
this.emit(APIRequestContext.Events.Request, requestEvent);

return new Promise((fulfill, reject) => {
const resultPromise = new Promise<SendRequestResult>((fulfill, reject) => {
const requestConstructor: ((url: URL, options: http.RequestOptions, callback?: (res: http.IncomingMessage) => void) => http.ClientRequest)
= (url.protocol === 'https:' ? https : http).request;
// If we have a proxy agent already, do not override it.
Expand Down Expand Up @@ -402,8 +400,6 @@ export abstract class APIRequestContext extends SdkObject {
headers,
agent: options.agent,
maxRedirects: options.maxRedirects - 1,
timeout: options.timeout,
deadline: options.deadline,
...getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, url.origin),
__testHookLookup: options.__testHookLookup,
};
Expand Down Expand Up @@ -492,6 +488,7 @@ export abstract class APIRequestContext extends SdkObject {
body.on('end', notifyBodyFinished);
});
request.on('error', reject);
progress.cleanupWhenAborted(() => request.destroy());

listeners.push(
eventsHelper.addEventListener(this, APIRequestContext.Events.Dispose, () => {
Expand Down Expand Up @@ -543,23 +540,11 @@ export abstract class APIRequestContext extends SdkObject {
progress.log(` ${name}: ${value}`);
}

if (options.deadline) {
const rejectOnTimeout = () => {
reject(new Error(`Request timed out after ${options.timeout}ms`));
request.destroy();
};
const remaining = options.deadline - monotonicTime();
if (remaining <= 0) {
rejectOnTimeout();
return;
}
request.setTimeout(remaining, rejectOnTimeout);
}

if (postData)
request.write(postData);
request.end();
});
return progress.race(resultPromise);
}

private _getHttpCredentials(url: URL) {
Expand Down
Loading