From 2380b07f300b318f173af859a065d1fe48c17f50 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 15 Sep 2021 14:02:55 -0700 Subject: [PATCH] feat(fetch): introduce FetchRequest.dispose, fulfill with global fetch (#8945) --- docs/src/api/class-fetchrequest.md | 6 +++++ src/client/fetch.ts | 21 ++++++++++++--- src/dispatchers/browserContextDispatcher.ts | 6 +---- src/dispatchers/networkDispatchers.ts | 13 +++++---- src/protocol/channels.ts | 4 +++ src/protocol/protocol.yml | 2 ++ src/protocol/validator.ts | 1 + src/server/browserContext.ts | 4 +-- src/server/fetch.ts | 30 ++++++++++++++++++++- src/server/network.ts | 3 ++- tests/browsercontext-fetch.spec.ts | 11 +++++++- tests/page/page-request-fulfill.spec.ts | 12 +++++++++ types/types.d.ts | 13 +++++++++ 13 files changed, 107 insertions(+), 19 deletions(-) diff --git a/docs/src/api/class-fetchrequest.md b/docs/src/api/class-fetchrequest.md index 7622493230..82d4f2affd 100644 --- a/docs/src/api/class-fetchrequest.md +++ b/docs/src/api/class-fetchrequest.md @@ -6,6 +6,12 @@ environment or the service to your e2e test. When used on [Page] or a [BrowserCo the cookies from the corresponding [BrowserContext]. This means that if you log in using this API, your e2e test will be logged in and vice versa. +## async method: FetchRequest.dispose + +All responses received through [`method: FetchRequest.fetch`], [`method: FetchRequest.get`], [`method: FetchRequest.post`] +and other methods are stored in the memory, so that you can later call [`method: FetchResponse.body`]. This method +discards all stored responses, and makes [`method: FetchResponse.body`] throw "Response disposed" error. + ## async method: FetchRequest.fetch - returns: <[FetchResponse]> diff --git a/src/client/fetch.ts b/src/client/fetch.ts index ad4d57d4a4..1e5a735701 100644 --- a/src/client/fetch.ts +++ b/src/client/fetch.ts @@ -17,6 +17,7 @@ import * as api from '../../types/types'; import { HeadersArray } from '../common/types'; import * as channels from '../protocol/channels'; +import { kBrowserOrContextClosedError } from '../utils/errors'; import { assert, headersObjectToArray, isString, objectToArray } from '../utils/utils'; import { ChannelOwner } from './channelOwner'; import * as network from './network'; @@ -41,6 +42,12 @@ export class FetchRequest extends ChannelOwner { + return this._wrapApiCall(async (channel: channels.FetchRequestChannel) => { + await channel.dispose(); + }); + } + async get( urlOrRequest: string | api.Request, options?: { @@ -137,10 +144,16 @@ export class FetchResponse implements api.FetchResponse { async body(): Promise { return this._request._wrapApiCall(async (channel: channels.FetchRequestChannel) => { - const result = await channel.fetchResponseBody({ fetchUid: this._fetchUid() }); - if (!result.binary) - throw new Error('Response has been disposed'); - return Buffer.from(result.binary!, 'base64'); + try { + const result = await channel.fetchResponseBody({ fetchUid: this._fetchUid() }); + if (!result.binary) + throw new Error('Response has been disposed'); + return Buffer.from(result.binary!, 'base64'); + } catch (e) { + if (e.message === kBrowserOrContextClosedError) + throw new Error('Response has been disposed'); + throw e; + } }); } diff --git a/src/dispatchers/browserContextDispatcher.ts b/src/dispatchers/browserContextDispatcher.ts index ecc85439aa..05ea5db256 100644 --- a/src/dispatchers/browserContextDispatcher.ts +++ b/src/dispatchers/browserContextDispatcher.ts @@ -15,7 +15,7 @@ */ import { BrowserContext } from '../server/browserContext'; -import { Dispatcher, DispatcherScope, existingDispatcher, lookupDispatcher } from './dispatcher'; +import { Dispatcher, DispatcherScope, lookupDispatcher } from './dispatcher'; import { PageDispatcher, BindingCallDispatcher, WorkerDispatcher } from './pageDispatcher'; import { FrameDispatcher } from './frameDispatcher'; import * as channels from '../protocol/channels'; @@ -58,10 +58,6 @@ export class BrowserContextDispatcher extends Dispatcher { this._dispatchEvent('close'); this._dispose(); - const fetch = existingDispatcher(this._context.fetchRequest); - // FetchRequestDispatcher is created in the browser rather then context scope but its - // lifetime is bound to the context dispatcher, so we manually dispose it here. - fetch._disposeDispatcher(); }); if (context._browser.options.name === 'chromium') { diff --git a/src/dispatchers/networkDispatchers.ts b/src/dispatchers/networkDispatchers.ts index 0e5516cf15..057b5b8bc6 100644 --- a/src/dispatchers/networkDispatchers.ts +++ b/src/dispatchers/networkDispatchers.ts @@ -171,6 +171,14 @@ export class FetchRequestDispatcher extends Dispatcher { + if (!this._disposed) + super._dispose(); + }); + } + + async dispose(params?: channels.FetchRequestDisposeParams): Promise { + this._object.dispose(); } async fetch(params: channels.FetchRequestFetchParams, metadata?: channels.Metadata): Promise { @@ -204,10 +212,5 @@ export class FetchRequestDispatcher extends Dispatcher { this._object.fetchResponses.delete(params.fetchUid); } - - _disposeDispatcher() { - if (!this._disposed) - super._dispose(); - } } diff --git a/src/protocol/channels.ts b/src/protocol/channels.ts index ad9b69f607..fe655715bc 100644 --- a/src/protocol/channels.ts +++ b/src/protocol/channels.ts @@ -156,6 +156,7 @@ export interface FetchRequestChannel extends Channel { fetch(params: FetchRequestFetchParams, metadata?: Metadata): Promise; fetchResponseBody(params: FetchRequestFetchResponseBodyParams, metadata?: Metadata): Promise; disposeFetchResponse(params: FetchRequestDisposeFetchResponseParams, metadata?: Metadata): Promise; + dispose(params?: FetchRequestDisposeParams, metadata?: Metadata): Promise; } export type FetchRequestFetchParams = { url: string, @@ -194,6 +195,9 @@ export type FetchRequestDisposeFetchResponseOptions = { }; export type FetchRequestDisposeFetchResponseResult = void; +export type FetchRequestDisposeParams = {}; +export type FetchRequestDisposeOptions = {}; +export type FetchRequestDisposeResult = void; export interface FetchRequestEvents { } diff --git a/src/protocol/protocol.yml b/src/protocol/protocol.yml index 4ce6e5df03..431aa7539f 100644 --- a/src/protocol/protocol.yml +++ b/src/protocol/protocol.yml @@ -249,6 +249,8 @@ FetchRequest: parameters: fetchUid: string + dispose: + FetchResponse: type: object diff --git a/src/protocol/validator.ts b/src/protocol/validator.ts index 111732db1b..7faf6d7499 100644 --- a/src/protocol/validator.ts +++ b/src/protocol/validator.ts @@ -162,6 +162,7 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { scheme.FetchRequestDisposeFetchResponseParams = tObject({ fetchUid: tString, }); + scheme.FetchRequestDisposeParams = tOptional(tObject({})); scheme.FetchResponse = tObject({ fetchUid: tString, url: tString, diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts index 89bbfe9d5b..c7756e580e 100644 --- a/src/server/browserContext.ts +++ b/src/server/browserContext.ts @@ -64,7 +64,7 @@ export abstract class BrowserContext extends SdkObject { private _origins = new Set(); readonly _harRecorder: HarRecorder | undefined; readonly tracing: Tracing; - readonly fetchRequest = new BrowserContextFetchRequest(this); + readonly fetchRequest: BrowserContextFetchRequest; constructor(browser: Browser, options: types.BrowserContextOptions, browserContextId: string | undefined) { super(browser, 'browser-context'); @@ -79,6 +79,7 @@ export abstract class BrowserContext extends SdkObject { this._harRecorder = new HarRecorder(this, {...this._options.recordHar, path: path.join(this._browser.options.artifactsDir, `${createGuid()}.har`)}); this.tracing = new Tracing(this); + this.fetchRequest = new BrowserContextFetchRequest(this); } _setSelectors(selectors: Selectors) { @@ -134,7 +135,6 @@ export abstract class BrowserContext extends SdkObject { this._closedStatus = 'closed'; this._deleteAllDownloads(); this._downloads.clear(); - this.fetchRequest.dispose(); if (this._isPersistentContext) this._onClosePersistent(); this._closePromiseFulfill!(new Error('Context closed')); diff --git a/src/server/fetch.ts b/src/server/fetch.ts index 0baa80c1d5..afadba3389 100644 --- a/src/server/fetch.ts +++ b/src/server/fetch.ts @@ -41,16 +41,35 @@ type FetchRequestOptions = { }; export abstract class FetchRequest extends SdkObject { + static Events = { + Dispose: 'dispose', + }; + readonly fetchResponses: Map = new Map(); + protected static allInstances: Set = new Set(); + + static findResponseBody(guid: string): Buffer | undefined { + for (const request of FetchRequest.allInstances) { + const body = request.fetchResponses.get(guid); + if (body) + return body; + } + return undefined; + } constructor(parent: SdkObject) { super(parent, 'fetchRequest'); + FetchRequest.allInstances.add(this); } - dispose() { + protected _disposeImpl() { + FetchRequest.allInstances.delete(this); this.fetchResponses.clear(); + this.emit(FetchRequest.Events.Dispose); } + abstract dispose(): void; + abstract _defaultOptions(): FetchRequestOptions; abstract _addCookies(cookies: types.SetNetworkCookieParam[]): Promise; abstract _cookies(url: string): Promise; @@ -273,6 +292,11 @@ export class BrowserContextFetchRequest extends FetchRequest { constructor(context: BrowserContext) { super(context); this._context = context; + context.once(BrowserContext.Events.Close, () => this._disposeImpl()); + } + + override dispose() { + this.fetchResponses.clear(); } _defaultOptions(): FetchRequestOptions { @@ -302,6 +326,10 @@ export class GlobalFetchRequest extends FetchRequest { super(playwright); } + override dispose() { + this._disposeImpl(); + } + _defaultOptions(): FetchRequestOptions { return { userAgent: '', diff --git a/src/server/network.ts b/src/server/network.ts index 58c13b33b1..f11d556f5a 100644 --- a/src/server/network.ts +++ b/src/server/network.ts @@ -20,6 +20,7 @@ import { assert } from '../utils/utils'; import { ManualPromise } from '../utils/async'; import { SdkObject } from './instrumentation'; import { NameValue } from '../common/types'; +import { FetchRequest } from './fetch'; export function filterCookies(cookies: types.NetworkCookie[], urls: string[]): types.NetworkCookie[] { const parsedURLs = urls.map(s => new URL(s)); @@ -228,7 +229,7 @@ export class Route extends SdkObject { if (body === undefined) { if (overrides.fetchResponseUid) { const context = this._request.frame()._page._browserContext; - const buffer = context.fetchRequest.fetchResponses.get(overrides.fetchResponseUid); + const buffer = context.fetchRequest.fetchResponses.get(overrides.fetchResponseUid) || FetchRequest.findResponseBody(overrides.fetchResponseUid); assert(buffer, 'Fetch response has been disposed'); body = buffer.toString('utf8'); isBase64 = false; diff --git a/tests/browsercontext-fetch.spec.ts b/tests/browsercontext-fetch.spec.ts index 2b2db8c1f8..7eb35de564 100644 --- a/tests/browsercontext-fetch.spec.ts +++ b/tests/browsercontext-fetch.spec.ts @@ -670,7 +670,16 @@ it('should dispose when context closes', async function({context, server}) { expect(await response.json()).toEqual({ foo: 'bar' }); await context.close(); const error = await response.body().catch(e => e); - expect(error.message).toContain('Target page, context or browser has been closed'); + expect(error.message).toContain('Response has been disposed'); +}); + +it('should dispose global request', async function({playwright, context, server}) { + const request = await playwright._newRequest(); + const response = await request.get(server.PREFIX + '/simple.json'); + expect(await response.json()).toEqual({ foo: 'bar' }); + await request.dispose(); + const error = await response.body().catch(e => e); + expect(error.message).toContain('Response has been disposed'); }); it('should throw on invalid first argument', async function({context}) { diff --git a/tests/page/page-request-fulfill.spec.ts b/tests/page/page-request-fulfill.spec.ts index 1936aa2904..64f1c812c9 100644 --- a/tests/page/page-request-fulfill.spec.ts +++ b/tests/page/page-request-fulfill.spec.ts @@ -194,6 +194,18 @@ it('should include the origin header', async ({page, server, isAndroid}) => { expect(interceptedRequest.headers()['origin']).toEqual(server.PREFIX); }); +it('should fulfill with global fetch result', async ({playwright, page, server, isElectron}) => { + it.fixme(isElectron, 'error: Browser context management is not supported.'); + await page.route('**/*', async route => { + const request = await playwright._newRequest(); + const response = await request.get(server.PREFIX + '/simple.json'); + route.fulfill({ response }); + }); + const response = await page.goto(server.EMPTY_PAGE); + expect(response.status()).toBe(200); + expect(await response.json()).toEqual({'foo': 'bar'}); +}); + it('should fulfill with fetch result', async ({page, server, isElectron}) => { it.fixme(isElectron, 'error: Browser context management is not supported.'); await page.route('**/*', async route => { diff --git a/types/types.d.ts b/types/types.d.ts index 90ff4695b9..7d824231b8 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -12625,6 +12625,19 @@ export interface Electron { * logged in and vice versa. */ export interface FetchRequest { + /** + * All responses received through + * [fetchRequest.fetch(urlOrRequest[, options])](https://playwright.dev/docs/api/class-fetchrequest#fetch-request-fetch), + * [fetchRequest.get(urlOrRequest[, options])](https://playwright.dev/docs/api/class-fetchrequest#fetch-request-get), + * [fetchRequest.post(urlOrRequest[, options])](https://playwright.dev/docs/api/class-fetchrequest#fetch-request-post) and + * other methods are stored in the memory, so that you can later call + * [fetchResponse.body()](https://playwright.dev/docs/api/class-fetchresponse#fetch-response-body). This method discards + * all stored responses, and makes + * [fetchResponse.body()](https://playwright.dev/docs/api/class-fetchresponse#fetch-response-body) throw "Response + * disposed" error. + */ + dispose(): Promise; + /** * Sends HTTP(S) fetch and returns its response. The method will populate fetch cookies from the context and update context * cookies from the response. The method will automatically follow redirects.