From 1eb18daab04a74a2ca4aa290689ed5dfb264cc71 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 19 Jun 2024 14:16:28 -0700 Subject: [PATCH] feat(fetch): maxRetries for fetch Fixes https://github.com/microsoft/playwright/issues/30978 --- docs/src/api/class-apirequestcontext.md | 18 ++++++++++ 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 | 22 +++++++++++- 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 | 18 ++++++++++ 11 files changed, 127 insertions(+), 5 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/params.md b/docs/src/api/params.md index a1dd84c4ef..4f6e495974 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 are retried. An error will be thrown if the limit is exceeded. Defaults to `5`. Pass `0` to disable 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..af37d5a3fc 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); }); const fetchUid = this._storeResponseBody(fetchResponse.body); this.fetchLog.set(fetchUid, controller.metadata.log); @@ -247,6 +247,26 @@ export abstract class APIRequestContext extends SdkObject { } } + private async _sendRequestWithRetries(progress: Progress, url: URL, options: SendRequestOptions, postData?: Buffer): Promise & { body: Buffer }>{ + const maxRetries = 5; + let backoff = 250; + for (let i = 0; i <= maxRetries; i++) { + try { + return await this._sendRequest(progress, url, options, postData); + } catch (e) { + if (i === maxRetries - 1 || (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..0cfacaddb0 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 are retried. An error + * will be thrown if the limit is exceeded. Defaults to `5`. Pass `0` to disable 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 are retried. An error + * will be thrown if the limit is exceeded. Defaults to `5`. Pass `0` to disable 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 are retried. An error + * will be thrown if the limit is exceeded. Defaults to `5`. Pass `0` to disable 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 are retried. An error + * will be thrown if the limit is exceeded. Defaults to `5`. Pass `0` to disable 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 are retried. An error + * will be thrown if the limit is exceeded. Defaults to `5`. Pass `0` to disable 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 are retried. An error + * will be thrown if the limit is exceeded. Defaults to `5`. Pass `0` to disable 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..e17f94b49d 100644 --- a/tests/library/global-fetch.spec.ts +++ b/tests/library/global-fetch.spec.ts @@ -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 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.fetch(server.PREFIX + '/test', { maxRetries: 3 }); + expect(response.status()).toBe(200); + expect(await response.text()).toBe('Hello!'); + expect(requestCount).toBe(4); +});