feat: retry timeout errors

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

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

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]>

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<Omit<channels.APIResponse, 'fetchUid'> & { body: Buffer }>{
+  private async _sendRequestWithRetries(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer, maxRetries = 0): Promise<Omit<channels.APIResponse, 'fetchUid'> & { body: Buffer }>{
-    maxRetries ??= 0;
     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();

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
+     * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does
-     * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries.
+     * 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
+     * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does
-     * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries.
+     * 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
+     * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does
-     * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries.
+     * 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
+     * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does
-     * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries.
+     * 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
+     * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does
-     * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries.
+     * 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
+     * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does
-     * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries.
+     * 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
+     * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does
-     * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries.
+     * 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
+     * Maximum number of times network errors should be retried. Only `ECONNRESET` and timeout errors are retried. Does
-     * retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries.
+     * not retry based on HTTP response codes. An error will be thrown if the limit is exceeded. Defaults to `0` - no
+     * retries.
      */
     maxRetries?: number;
 

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();
+});