Skip to content

Commit

Permalink
feat(fetch): maxRetries for fetch (#31386)
Browse files Browse the repository at this point in the history
Fixes #30978
  • Loading branch information
yury-s authored Jun 20, 2024
1 parent 2dfda0a commit 95fc2b8
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 6 deletions.
18 changes: 18 additions & 0 deletions docs/src/api/class-apirequestcontext.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]>
Expand Down Expand Up @@ -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]>
Expand Down Expand Up @@ -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]>
Expand Down Expand Up @@ -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]>
Expand Down Expand Up @@ -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]>
Expand Down Expand Up @@ -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]>
Expand Down
10 changes: 10 additions & 0 deletions docs/src/api/class-requestoptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]>
Expand Down
6 changes: 6 additions & 0 deletions docs/src/api/params.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]>

Expand Down
8 changes: 5 additions & 3 deletions packages/playwright-core/src/client/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export type FetchOptions = {
failOnStatusCode?: boolean,
ignoreHTTPSErrors?: boolean,
maxRedirects?: number,
maxRetries?: number,
};

type NewContextOptions = Omit<channels.PlaywrightNewRequestOptions, 'extraHTTPHeaders' | 'storageState' | 'tracesDir'> & {
Expand Down Expand Up @@ -168,11 +169,11 @@ export class APIRequestContext extends ChannelOwner<channels.APIRequestContextCh
throw new TargetClosedError(this._closeReason);
assert(options.request || typeof options.url === 'string', 'First argument must be either URL string or Request');
assert((options.data === undefined ? 0 : 1) + (options.form === undefined ? 0 : 1) + (options.multipart === undefined ? 0 : 1) <= 1, `Only one of 'data', 'form' or 'multipart' can be specified`);
assert(options.maxRedirects === undefined || options.maxRedirects >= 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;
Expand Down Expand Up @@ -234,7 +235,8 @@ export class APIRequestContext extends ChannelOwner<channels.APIRequestContextCh
timeout: options.timeout,
failOnStatusCode: options.failOnStatusCode,
ignoreHTTPSErrors: options.ignoreHTTPSErrors,
maxRedirects: maxRedirects,
maxRedirects: options.maxRedirects,
maxRetries: options.maxRetries,
...fixtures
});
return new APIResponse(this, result.response);
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/client/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ export class Route extends ChannelOwner<channels.RouteChannel> implements api.Ro
});
}

async fetch(options: FallbackOverrides & { maxRedirects?: number, timeout?: number } = {}): Promise<APIResponse> {
async fetch(options: FallbackOverrides & { maxRedirects?: number, maxRetries?: number, timeout?: number } = {}): Promise<APIResponse> {
return await this._wrapApiCall(async () => {
return await this._context.request._innerFetch({ request: this.request(), data: options.postData, ...options });
});
Expand Down
1 change: 1 addition & 0 deletions packages/playwright-core/src/protocol/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
24 changes: 23 additions & 1 deletion packages/playwright-core/src/server/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -247,6 +247,28 @@ 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 }>{
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<Omit<channels.APIResponse, 'fetchUid'> & { body: Buffer }>{
await this._updateRequestCookieHeader(url, options.headers);

Expand Down
36 changes: 36 additions & 0 deletions packages/playwright-core/types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions packages/protocol/src/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ export type APIRequestContextFetchParams = {
failOnStatusCode?: boolean,
ignoreHTTPSErrors?: boolean,
maxRedirects?: number,
maxRetries?: number,
};
export type APIRequestContextFetchOptions = {
params?: NameValue[],
Expand All @@ -337,6 +338,7 @@ export type APIRequestContextFetchOptions = {
failOnStatusCode?: boolean,
ignoreHTTPSErrors?: boolean,
maxRedirects?: number,
maxRetries?: number,
};
export type APIRequestContextFetchResult = {
response: APIResponse,
Expand Down
1 change: 1 addition & 0 deletions packages/protocol/src/protocol.yml
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ APIRequestContext:
failOnStatusCode: boolean?
ignoreHTTPSErrors: boolean?
maxRedirects: number?
maxRetries: number?
returns:
response: APIResponse

Expand Down
18 changes: 18 additions & 0 deletions tests/library/browsercontext-fetch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
20 changes: 19 additions & 1 deletion tests/library/global-fetch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand Down Expand Up @@ -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);
});

0 comments on commit 95fc2b8

Please sign in to comment.