diff --git a/docs/src/api/class-requestoptions.md b/docs/src/api/class-requestoptions.md index 63aa1d6af323e..b2ccbd5d2b832 100644 --- a/docs/src/api/class-requestoptions.md +++ b/docs/src/api/class-requestoptions.md @@ -134,7 +134,7 @@ Defaults to `20`. Pass `0` to not follow redirects. * since: v1.46 - `maxRetries` <[int]> -Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. +Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. ## method: RequestOptions.setMethod * since: v1.18 diff --git a/docs/src/api/class-route.md b/docs/src/api/class-route.md index c4e5fcb2e4c46..4e191dcc315d1 100644 --- a/docs/src/api/class-route.md +++ b/docs/src/api/class-route.md @@ -513,7 +513,7 @@ Defaults to `20`. Pass `0` to not follow redirects. * since: v1.46 - `maxRetries` <[int]> -Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. +Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. ### option: Route.fetch.timeout * since: v1.33 diff --git a/docs/src/api/params.md b/docs/src/api/params.md index a1f909a4fb672..fbe592cfbb422 100644 --- a/docs/src/api/params.md +++ b/docs/src/api/params.md @@ -494,7 +494,7 @@ Defaults to `20`. Pass `0` to not follow redirects. * langs: js, python, csharp - `maxRetries` <[int]> -Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. +Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. ## evaluate-expression - `expression` <[string]> diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index f231c907c0827..b1d85150f907f 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -80,11 +80,17 @@ export type APIRequestFinishedEvent = { type SendRequestOptions = https.RequestOptions & { maxRedirects: number, - deadline: number, + deadline?: number, // applies to the whole retry sequence. headers: HeadersObject, __testHookLookup?: (hostname: string) => LookupAddress[] }; +class RequestTimedOutError extends Error { + constructor(timeout: number) { + super(`Request timed out after ${timeout}ms`); + } +} + export abstract class APIRequestContext extends SdkObject { static Events = { Dispose: 'dispose', @@ -180,7 +186,6 @@ export abstract class APIRequestContext extends SdkObject { const timeout = defaults.timeoutSettings.timeout(params); - const deadline = timeout && (monotonicTime() + timeout); const options: SendRequestOptions = { method, @@ -188,7 +193,6 @@ export abstract class APIRequestContext extends SdkObject { agent, maxRedirects: params.maxRedirects === 0 ? -1 : params.maxRedirects === undefined ? 20 : params.maxRedirects, timeout, - deadline, ...getMatchingTLSOptionsForOrigin(this._defaultOptions().clientCertificates, requestUrl.origin), __testHookLookup: (params as any).__testHookLookup, }; @@ -255,24 +259,33 @@ export abstract class APIRequestContext extends SdkObject { } } - private async _sendRequestWithRetries(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer, maxRetries?: number): Promise & { body: Buffer }>{ - maxRetries ??= 0; + private async _sendRequestWithRetries(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer, maxRetries = 0): Promise & { body: Buffer }>{ let backoff = 250; for (let i = 0; i <= maxRetries; i++) { try { - return await this._sendRequest(progress, url, options, postData); + const requestOptions = { ...options }; + if (options.timeout) + requestOptions.deadline ??= monotonicTime() + options.timeout; + return await this._sendRequest(progress, url, requestOptions, postData); } catch (e) { e = rewriteOpenSSLErrorIfNeeded(e); if (maxRetries === 0) throw e; if (i === maxRetries || (options.deadline && monotonicTime() + backoff > options.deadline)) throw new Error(`Failed after ${i + 1} attempt(s): ${e}`); - // Retry on connection reset only. - if (e.code !== 'ECONNRESET') + + async function retry(reason: string) { + progress.log(` ${reason}, will retry after ${backoff}ms.`); + await new Promise(f => setTimeout(f, backoff)); + backoff *= 2; + } + + if (e instanceof RequestTimedOutError) + await retry('Request timed out'); + else if (e.code === 'ECONNRESET') + await retry('Received ECONNRESET'); + else throw e; - progress.log(` Received ECONNRESET, will retry after ${backoff}ms.`); - await new Promise(f => setTimeout(f, backoff)); - backoff *= 2; } } throw new Error('Unreachable'); @@ -544,7 +557,7 @@ export abstract class APIRequestContext extends SdkObject { if (options.deadline) { const rejectOnTimeout = () => { - reject(new Error(`Request timed out after ${options.timeout}ms`)); + reject(new RequestTimedOutError(options.timeout!)); request.destroy(); }; const remaining = options.deadline - monotonicTime(); diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index 21b531b07acb6..650d62b3c403d 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -17670,8 +17670,9 @@ export interface APIRequestContext { maxRedirects?: number; /** - * Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not - * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. + * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does + * not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no + * retries. */ maxRetries?: number; @@ -17801,8 +17802,9 @@ export interface APIRequestContext { maxRedirects?: number; /** - * Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not - * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. + * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does + * not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no + * retries. */ maxRetries?: number; @@ -17919,8 +17921,9 @@ export interface APIRequestContext { maxRedirects?: number; /** - * Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not - * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. + * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does + * not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no + * retries. */ maxRetries?: number; @@ -18005,8 +18008,9 @@ export interface APIRequestContext { maxRedirects?: number; /** - * Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not - * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. + * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does + * not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no + * retries. */ maxRetries?: number; @@ -18091,8 +18095,9 @@ export interface APIRequestContext { maxRedirects?: number; /** - * Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not - * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. + * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does + * not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no + * retries. */ maxRetries?: number; @@ -18219,8 +18224,9 @@ export interface APIRequestContext { maxRedirects?: number; /** - * Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not - * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. + * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does + * not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no + * retries. */ maxRetries?: number; @@ -18305,8 +18311,9 @@ export interface APIRequestContext { maxRedirects?: number; /** - * Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not - * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. + * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does + * not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no + * retries. */ maxRetries?: number; @@ -20867,8 +20874,9 @@ export interface Route { maxRedirects?: number; /** - * Maximum number of times network errors should be retried. Currently only `ECONNRESET` error is retried. Does not - * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. + * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does + * not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no + * retries. */ maxRetries?: number; diff --git a/tests/library/global-fetch.spec.ts b/tests/library/global-fetch.spec.ts index 52d48c765c1d9..36832a2e229cd 100644 --- a/tests/library/global-fetch.spec.ts +++ b/tests/library/global-fetch.spec.ts @@ -536,3 +536,21 @@ it('should retry ECONNRESET', { expect(requestCount).toBe(4); await request.dispose(); }); + +it('should retry ETIMEDOUT', { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/34207' } +}, async ({ playwright, server }) => { + const request = await playwright.request.newContext(); + let requestCount = 0; + server.setRoute('/test', (req, res) => { + if (requestCount++ < 3) + return; + res.writeHead(200, { 'content-type': 'text/plain' }); + res.end('Hello!'); + }); + const response = await request.fetch(server.PREFIX + '/test', { maxRetries: 3, timeout: 100 }); + expect(response.status()).toBe(200); + expect(await response.text()).toBe('Hello!'); + expect(requestCount).toBe(4); + await request.dispose(); +});