From 77b3b0965af1cf28670f9509fa265c2cea78e315 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 8 Sep 2021 10:01:40 -0700 Subject: [PATCH] feat(fetch): timeout option and default timeout (#8762) --- src/client/browserContext.ts | 3 +- src/dispatchers/browserContextDispatcher.ts | 1 + src/protocol/channels.ts | 2 ++ src/protocol/protocol.yml | 1 + src/protocol/validator.ts | 1 + src/server/fetch.ts | 24 +++++++++++-- src/server/types.ts | 1 + tests/browsercontext-fetch.spec.ts | 37 +++++++++++++++++++++ 8 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/client/browserContext.ts b/src/client/browserContext.ts index 729423530b..ee6aeda495 100644 --- a/src/client/browserContext.ts +++ b/src/client/browserContext.ts @@ -216,7 +216,7 @@ export class BrowserContext extends ChannelOwner { + async _fetch(url: string, options: { url?: string, method?: string, headers?: Headers, postData?: string | Buffer, timeout?: number } = {}): Promise { return this._wrapApiCall(async (channel: channels.BrowserContextChannel) => { const postDataBuffer = isString(options.postData) ? Buffer.from(options.postData, 'utf8') : options.postData; const result = await channel.fetch({ @@ -224,6 +224,7 @@ export class BrowserContext extends ChannelOwner Validator): Scheme { method: tOptional(tString), headers: tOptional(tArray(tType('NameValue'))), postData: tOptional(tBinary), + timeout: tOptional(tNumber), }); scheme.BrowserContextGrantPermissionsParams = tObject({ permissions: tArray(tString), diff --git a/src/server/fetch.ts b/src/server/fetch.ts index 0142938e3d..0d07488950 100644 --- a/src/server/fetch.ts +++ b/src/server/fetch.ts @@ -22,6 +22,7 @@ import * as https from 'https'; import { BrowserContext } from './browserContext'; import * as types from './types'; import { pipeline, Readable, Transform } from 'stream'; +import { monotonicTime } from '../utils/utils'; export async function playwrightFetch(context: BrowserContext, params: types.FetchOptions): Promise<{fetchResponse?: types.FetchResponse, error?: string}> { try { @@ -50,11 +51,16 @@ export async function playwrightFetch(context: BrowserContext, params: types.Fet agent = new HttpsProxyAgent(proxyOpts); } + const timeout = context._timeoutSettings.timeout(params); + const deadline = monotonicTime() + timeout; + const fetchResponse = await sendRequest(context, new URL(params.url, context._options.baseURL), { method, headers, agent, - maxRedirects: 20 + maxRedirects: 20, + timeout, + deadline }, params.postData); return { fetchResponse }; } catch (e) { @@ -95,7 +101,7 @@ async function updateRequestCookieHeader(context: BrowserContext, url: URL, opti } } -async function sendRequest(context: BrowserContext, url: URL, options: http.RequestOptions & { maxRedirects: number }, postData?: Buffer): Promise{ +async function sendRequest(context: BrowserContext, url: URL, options: http.RequestOptions & { maxRedirects: number, deadline: number }, postData?: Buffer): Promise{ await updateRequestCookieHeader(context, url, options); return new Promise((fulfill, reject) => { const requestConstructor: ((url: URL, options: http.RequestOptions, callback?: (res: http.IncomingMessage) => void) => http.ClientRequest) @@ -125,11 +131,13 @@ async function sendRequest(context: BrowserContext, url: URL, options: http.Requ delete headers[`content-type`]; } - const redirectOptions: http.RequestOptions & { maxRedirects: number } = { + const redirectOptions: http.RequestOptions & { maxRedirects: number, deadline: number } = { method, headers, agent: options.agent, maxRedirects: options.maxRedirects - 1, + timeout: options.timeout, + deadline: options.deadline }; // HTTP-redirect fetch step 4: If locationURL is null, then return response. @@ -189,6 +197,16 @@ async function sendRequest(context: BrowserContext, url: URL, options: http.Requ body.on('error',reject); }); request.on('error', reject); + const rejectOnTimeout = () => { + reject(new Error(`Request timed out after ${options.timeout}ms`)); + request.abort(); + }; + const remaining = options.deadline - monotonicTime(); + if (remaining <= 0) { + rejectOnTimeout(); + return; + } + request.setTimeout(remaining, rejectOnTimeout); if (postData) request.write(postData); request.end(); diff --git a/src/server/types.ts b/src/server/types.ts index ac6938d200..78ecdd3d9f 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -377,6 +377,7 @@ export type FetchOptions = { method?: string, headers?: { [name: string]: string }, postData?: Buffer, + timeout?: number, }; export type FetchResponse = { diff --git a/tests/browsercontext-fetch.spec.ts b/tests/browsercontext-fetch.spec.ts index 8ad3617ed2..a4e2084650 100644 --- a/tests/browsercontext-fetch.spec.ts +++ b/tests/browsercontext-fetch.spec.ts @@ -193,6 +193,16 @@ it('should add cookies from Set-Cookie header', async ({context, page, server}) expect((await page.evaluate(() => document.cookie)).split(';').map(s => s.trim()).sort()).toEqual(['foo=bar', 'session=value']); }); +it('should not lose body while handling Set-Cookie header', async ({context, page, server}) => { + server.setRoute('/setcookie.html', (req, res) => { + res.setHeader('Set-Cookie', ['session=value', 'foo=bar; max-age=3600']); + res.end('text content'); + }); + // @ts-expect-error + const response = await context._fetch(server.PREFIX + '/setcookie.html'); + expect(await response.text()).toBe('text content'); +}); + it('should handle cookies on redirects', async ({context, server, browserName, isWindows}) => { server.setRoute('/redirect1', (req, res) => { res.setHeader('Set-Cookie', 'r1=v1;SameSite=Lax'); @@ -576,3 +586,30 @@ it('should throw informatibe error on corrupted deflate body', async function({c expect(error.message).toContain(`failed to decompress 'deflate' encoding`); }); +it('should support timeout option', async function({context, server}) { + server.setRoute('/slow', (req, res) => { + res.writeHead(200, { + 'content-length': 4096, + 'content-type': 'text/html', + }); + }); + + // @ts-expect-error + const error = await context._fetch(server.PREFIX + '/slow', { timeout: 10 }).catch(e => e); + expect(error.message).toContain(`Request timed out after 10ms`); +}); + +it('should respect timeout after redirects', async function({context, server}) { + server.setRedirect('/redirect', '/slow'); + server.setRoute('/slow', (req, res) => { + res.writeHead(200, { + 'content-length': 4096, + 'content-type': 'text/html', + }); + }); + + context.setDefaultTimeout(100); + // @ts-expect-error + const error = await context._fetch(server.PREFIX + '/redirect').catch(e => e); + expect(error.message).toContain(`Request timed out after 100ms`); +});