From 95fc2b8a8b449560e53999c866e9602f4bca27f1 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 19 Jun 2024 18:10:14 -0700 Subject: [PATCH] feat(fetch): maxRetries for fetch (#31386) Fixes https://github.com/microsoft/playwright/issues/30978 --- docs/src/api/class-apirequestcontext.md | 18 ++++++++++ docs/src/api/class-requestoptions.md | 10 ++++++ docs/src/api/params.md | 6 ++++ packages/playwright-core/src/client/fetch.ts | 8 +++-- .../playwright-core/src/client/network.ts | 2 +- .../playwright-core/src/protocol/validator.ts | 1 + packages/playwright-core/src/server/fetch.ts | 24 ++++++++++++- packages/playwright-core/types/types.d.ts | 36 +++++++++++++++++++ packages/protocol/src/channels.ts | 2 ++ packages/protocol/src/protocol.yml | 1 + tests/library/browsercontext-fetch.spec.ts | 18 ++++++++++ tests/library/global-fetch.spec.ts | 20 ++++++++++- 12 files changed, 140 insertions(+), 6 deletions(-) diff --git a/docs/src/api/class-apirequestcontext.md b/docs/src/api/class-apirequestcontext.md index 7101d1996d..dab99bec2b 100644 --- a/docs/src/api/class-apirequestcontext.md +++ b/docs/src/api/class-apirequestcontext.md @@ -344,6 +344,9 @@ If set changes the fetch method (e.g. [PUT](https://developer.mozilla.org/en-US/ ### option: APIRequestContext.fetch.maxRedirects = %%-js-python-csharp-fetch-option-maxredirects-%% * since: v1.26 +### option: APIRequestContext.fetch.maxRetries = %%-js-python-csharp-fetch-option-maxretries-%% +* since: v1.46 + ## async method: APIRequestContext.get * since: v1.16 - returns: <[APIResponse]> @@ -433,6 +436,9 @@ await request.GetAsync("https://example.com/api/getText", new() { Params = query ### option: APIRequestContext.get.maxRedirects = %%-js-python-csharp-fetch-option-maxredirects-%% * since: v1.26 +### option: APIRequestContext.get.maxRetries = %%-js-python-csharp-fetch-option-maxretries-%% +* since: v1.46 + ## async method: APIRequestContext.head * since: v1.16 - returns: <[APIResponse]> @@ -486,6 +492,9 @@ context cookies from the response. The method will automatically follow redirect ### option: APIRequestContext.head.maxRedirects = %%-js-python-csharp-fetch-option-maxredirects-%% * since: v1.26 +### option: APIRequestContext.head.maxRetries = %%-js-python-csharp-fetch-option-maxretries-%% +* since: v1.46 + ## async method: APIRequestContext.patch * since: v1.16 - returns: <[APIResponse]> @@ -539,6 +548,9 @@ context cookies from the response. The method will automatically follow redirect ### option: APIRequestContext.patch.maxRedirects = %%-js-python-csharp-fetch-option-maxredirects-%% * since: v1.26 +### option: APIRequestContext.patch.maxRetries = %%-js-python-csharp-fetch-option-maxretries-%% +* since: v1.46 + ## async method: APIRequestContext.post * since: v1.16 - returns: <[APIResponse]> @@ -713,6 +725,9 @@ await request.PostAsync("https://example.com/api/uploadScript", new() { Multipar ### option: APIRequestContext.post.maxRedirects = %%-js-python-csharp-fetch-option-maxredirects-%% * since: v1.26 +### option: APIRequestContext.post.maxRetries = %%-js-python-csharp-fetch-option-maxretries-%% +* since: v1.46 + ## async method: APIRequestContext.put * since: v1.16 - returns: <[APIResponse]> @@ -766,6 +781,9 @@ context cookies from the response. The method will automatically follow redirect ### option: APIRequestContext.put.maxRedirects = %%-js-python-csharp-fetch-option-maxredirects-%% * since: v1.26 +### option: APIRequestContext.put.maxRetries = %%-js-python-csharp-fetch-option-maxretries-%% +* since: v1.46 + ## async method: APIRequestContext.storageState * since: v1.16 - returns: <[Object]> diff --git a/docs/src/api/class-requestoptions.md b/docs/src/api/class-requestoptions.md index b6eeb8970a..401e13d944 100644 --- a/docs/src/api/class-requestoptions.md +++ b/docs/src/api/class-requestoptions.md @@ -126,6 +126,16 @@ Whether to ignore HTTPS errors when sending network requests. Maximum number of request redirects that will be followed automatically. An error will be thrown if the number is exceeded. Defaults to `20`. Pass `0` to not follow redirects. +## method: RequestOptions.setMaxRetries +* since: v1.46 +- returns: <[RequestOptions]> + +### param: RequestOptions.setMaxRetries.maxRetries +* since: v1.46 +- `maxRetries` <[int]> + +Maximum number of times socket errors should be retried. Currently only `ECONNRESET` error is retried. An error will be thrown if the limit is exceeded. Defaults to `0` - no retries. + ## method: RequestOptions.setMethod * since: v1.18 - returns: <[RequestOptions]> diff --git a/docs/src/api/params.md b/docs/src/api/params.md index a1dd84c4ef..253ca3a1fd 100644 --- a/docs/src/api/params.md +++ b/docs/src/api/params.md @@ -458,6 +458,12 @@ Whether to ignore HTTPS errors when sending network requests. Defaults to `false Maximum number of request redirects that will be followed automatically. An error will be thrown if the number is exceeded. Defaults to `20`. Pass `0` to not follow redirects. +## js-python-csharp-fetch-option-maxretries +* langs: js, python, csharp +- `maxRetries` <[int]> + +Maximum number of times socket errors should be retried. Currently only `ECONNRESET` error is retried. 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/client/fetch.ts b/packages/playwright-core/src/client/fetch.ts index 4598ac2418..44314c6fa3 100644 --- a/packages/playwright-core/src/client/fetch.ts +++ b/packages/playwright-core/src/client/fetch.ts @@ -41,6 +41,7 @@ export type FetchOptions = { failOnStatusCode?: boolean, ignoreHTTPSErrors?: boolean, maxRedirects?: number, + maxRetries?: number, }; type NewContextOptions = Omit & { @@ -168,11 +169,11 @@ export class APIRequestContext extends ChannelOwner= 0, `'maxRedirects' should be greater than or equal to '0'`); + assert(options.maxRedirects === undefined || options.maxRedirects >= 0, `'maxRedirects' must be greater than or equal to '0'`); + assert(options.maxRetries === undefined || options.maxRetries >= 0, `'maxRetries' must be greater than or equal to '0'`); const url = options.url !== undefined ? options.url : options.request!.url(); const params = objectToArray(options.params); const method = options.method || options.request?.method(); - const maxRedirects = options.maxRedirects; // Cannot call allHeaders() here as the request may be paused inside route handler. const headersObj = options.headers || options.request?.headers() ; const headers = headersObj ? headersObjectToArray(headersObj) : undefined; @@ -234,7 +235,8 @@ export class APIRequestContext extends ChannelOwner implements api.Ro }); } - async fetch(options: FallbackOverrides & { maxRedirects?: number, timeout?: number } = {}): Promise { + async fetch(options: FallbackOverrides & { maxRedirects?: number, maxRetries?: number, timeout?: number } = {}): Promise { return await this._wrapApiCall(async () => { return await this._context.request._innerFetch({ request: this.request(), data: options.postData, ...options }); }); diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 5727192b59..9187dc13d2 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -182,6 +182,7 @@ scheme.APIRequestContextFetchParams = tObject({ failOnStatusCode: tOptional(tBoolean), ignoreHTTPSErrors: tOptional(tBoolean), maxRedirects: tOptional(tNumber), + maxRetries: tOptional(tNumber), }); scheme.APIRequestContextFetchResult = tObject({ response: tType('APIResponse'), diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index 0f87e0046b..c02781f748 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -201,7 +201,7 @@ export abstract class APIRequestContext extends SdkObject { setHeader(headers, 'content-length', String(postData.byteLength)); const controller = new ProgressController(metadata, this); const fetchResponse = await controller.run(progress => { - return this._sendRequest(progress, requestUrl, options, postData); + return this._sendRequestWithRetries(progress, requestUrl, options, postData, params.maxRetries); }); const fetchUid = this._storeResponseBody(fetchResponse.body); this.fetchLog.set(fetchUid, controller.metadata.log); @@ -247,6 +247,28 @@ export abstract class APIRequestContext extends SdkObject { } } + private async _sendRequestWithRetries(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer, maxRetries?: number): Promise & { body: Buffer }>{ + maxRetries ??= 0; + let backoff = 250; + for (let i = 0; i <= maxRetries; i++) { + try { + return await this._sendRequest(progress, url, options, postData); + } catch (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') + throw e; + progress.log(` Received ECONNRESET, will retry after ${backoff}ms.`); + await new Promise(f => setTimeout(f, backoff)); + backoff *= 2; + } + } + throw new Error('Unreachable'); + } + private async _sendRequest(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer): Promise & { body: Buffer }>{ await this._updateRequestCookieHeader(url, options.headers); diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index 28250cb271..67e92a917a 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -15963,6 +15963,12 @@ export interface APIRequestContext { */ maxRedirects?: number; + /** + * Maximum number of times socket errors should be retried. Currently only `ECONNRESET` error is retried. An error + * will be thrown if the limit is exceeded. Defaults to `0` - no retries. + */ + maxRetries?: number; + /** * If set changes the fetch method (e.g. [PUT](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT) or * [POST](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/POST)). If not specified, GET method is used. @@ -16063,6 +16069,12 @@ export interface APIRequestContext { */ maxRedirects?: number; + /** + * Maximum number of times socket errors should be retried. Currently only `ECONNRESET` error is retried. An error + * will be thrown if the limit is exceeded. Defaults to `0` - no retries. + */ + maxRetries?: number; + /** * Provides an object that will be serialized as html form using `multipart/form-data` encoding and sent as this * request body. If this parameter is specified `content-type` header will be set to `multipart/form-data` unless @@ -16143,6 +16155,12 @@ export interface APIRequestContext { */ maxRedirects?: number; + /** + * Maximum number of times socket errors should be retried. Currently only `ECONNRESET` error is retried. An error + * will be thrown if the limit is exceeded. Defaults to `0` - no retries. + */ + maxRetries?: number; + /** * Provides an object that will be serialized as html form using `multipart/form-data` encoding and sent as this * request body. If this parameter is specified `content-type` header will be set to `multipart/form-data` unless @@ -16223,6 +16241,12 @@ export interface APIRequestContext { */ maxRedirects?: number; + /** + * Maximum number of times socket errors should be retried. Currently only `ECONNRESET` error is retried. An error + * will be thrown if the limit is exceeded. Defaults to `0` - no retries. + */ + maxRetries?: number; + /** * Provides an object that will be serialized as html form using `multipart/form-data` encoding and sent as this * request body. If this parameter is specified `content-type` header will be set to `multipart/form-data` unless @@ -16345,6 +16369,12 @@ export interface APIRequestContext { */ maxRedirects?: number; + /** + * Maximum number of times socket errors should be retried. Currently only `ECONNRESET` error is retried. An error + * will be thrown if the limit is exceeded. Defaults to `0` - no retries. + */ + maxRetries?: number; + /** * Provides an object that will be serialized as html form using `multipart/form-data` encoding and sent as this * request body. If this parameter is specified `content-type` header will be set to `multipart/form-data` unless @@ -16425,6 +16455,12 @@ export interface APIRequestContext { */ maxRedirects?: number; + /** + * Maximum number of times socket errors should be retried. Currently only `ECONNRESET` error is retried. An error + * will be thrown if the limit is exceeded. Defaults to `0` - no retries. + */ + maxRetries?: number; + /** * Provides an object that will be serialized as html form using `multipart/form-data` encoding and sent as this * request body. If this parameter is specified `content-type` header will be set to `multipart/form-data` unless diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts index 13d34f1f51..ffb591556c 100644 --- a/packages/protocol/src/channels.ts +++ b/packages/protocol/src/channels.ts @@ -324,6 +324,7 @@ export type APIRequestContextFetchParams = { failOnStatusCode?: boolean, ignoreHTTPSErrors?: boolean, maxRedirects?: number, + maxRetries?: number, }; export type APIRequestContextFetchOptions = { params?: NameValue[], @@ -337,6 +338,7 @@ export type APIRequestContextFetchOptions = { failOnStatusCode?: boolean, ignoreHTTPSErrors?: boolean, maxRedirects?: number, + maxRetries?: number, }; export type APIRequestContextFetchResult = { response: APIResponse, diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index 2bef7c8762..14799ef17e 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -299,6 +299,7 @@ APIRequestContext: failOnStatusCode: boolean? ignoreHTTPSErrors: boolean? maxRedirects: number? + maxRetries: number? returns: response: APIResponse diff --git a/tests/library/browsercontext-fetch.spec.ts b/tests/library/browsercontext-fetch.spec.ts index ba01ccc07c..a8aaeb389f 100644 --- a/tests/library/browsercontext-fetch.spec.ts +++ b/tests/library/browsercontext-fetch.spec.ts @@ -1287,3 +1287,21 @@ it('should not work after context dispose', async ({ context, server }) => { await context.close({ reason: 'Test ended.' }); expect(await context.request.get(server.EMPTY_PAGE).catch(e => e.message)).toContain('Test ended.'); }); + +it('should retrty ECONNRESET', { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/30978' } +}, async ({ context, server }) => { + let requestCount = 0; + server.setRoute('/test', (req, res) => { + if (requestCount++ < 3) { + req.socket.destroy(); + return; + } + res.writeHead(200, { 'content-type': 'text/plain' }); + res.end('Hello!'); + }); + const response = await context.request.get(server.PREFIX + '/test', { maxRetries: 3 }); + expect(response.status()).toBe(200); + expect(await response.text()).toBe('Hello!'); + expect(requestCount).toBe(4); +}); diff --git a/tests/library/global-fetch.spec.ts b/tests/library/global-fetch.spec.ts index a2eb629dc5..264df5c619 100644 --- a/tests/library/global-fetch.spec.ts +++ b/tests/library/global-fetch.spec.ts @@ -436,7 +436,7 @@ it('should throw an error when maxRedirects is less than 0', async ({ playwright const request = await playwright.request.newContext(); for (const method of ['GET', 'PUT', 'POST', 'OPTIONS', 'HEAD', 'PATCH']) - await expect(async () => request.fetch(`${server.PREFIX}/a/redirect1`, { method, maxRedirects: -1 })).rejects.toThrow(`'maxRedirects' should be greater than or equal to '0'`); + await expect(async () => request.fetch(`${server.PREFIX}/a/redirect1`, { method, maxRedirects: -1 })).rejects.toThrow(`'maxRedirects' must be greater than or equal to '0'`); await request.dispose(); }); @@ -483,3 +483,21 @@ it('should throw after dispose', async ({ playwright, server }) => { await request.dispose(); await expect(request.get(server.EMPTY_PAGE)).rejects.toThrow('Target page, context or browser has been closed'); }); + +it('should retry ECONNRESET', { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/30978' } +}, async ({ context, server }) => { + let requestCount = 0; + server.setRoute('/test', (req, res) => { + if (requestCount++ < 3) { + req.socket.destroy(); + return; + } + res.writeHead(200, { 'content-type': 'text/plain' }); + res.end('Hello!'); + }); + const response = await context.request.fetch(server.PREFIX + '/test', { maxRetries: 3 }); + expect(response.status()).toBe(200); + expect(await response.text()).toBe('Hello!'); + expect(requestCount).toBe(4); +});