From 28e925c001c8e22d02e1a9b65d9a84ca28e318b0 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 21 May 2025 18:16:05 +0000 Subject: [PATCH] chore: remove protocol recordHar option in favor of explicit harStart (#36030) --- .../playwright-core/src/client/android.ts | 1 + .../playwright-core/src/client/browser.ts | 1 + .../src/client/browserContext.ts | 39 +++++++++---------- .../playwright-core/src/client/browserType.ts | 1 + .../playwright-core/src/client/electron.ts | 1 + .../playwright-core/src/protocol/validator.ts | 7 +--- .../src/server/browserContext.ts | 4 -- .../src/server/har/harRecorder.ts | 2 +- .../src/server/har/harTracer.ts | 2 + packages/protocol/src/channels.d.ts | 12 +----- packages/protocol/src/protocol.yml | 4 +- tests/library/har.spec.ts | 18 +-------- 12 files changed, 30 insertions(+), 62 deletions(-) diff --git a/packages/playwright-core/src/client/android.ts b/packages/playwright-core/src/client/android.ts index 5b0b7efaec..fd94fd2629 100644 --- a/packages/playwright-core/src/client/android.ts +++ b/packages/playwright-core/src/client/android.ts @@ -259,6 +259,7 @@ export class AndroidDevice extends ChannelOwner i const result = await this._channel.launchBrowser(contextOptions); const context = BrowserContext.from(result.context) as BrowserContext; context._setOptions(contextOptions, {}); + await context._initializeHarFromOptions(options.recordHar); return context; } diff --git a/packages/playwright-core/src/client/browser.ts b/packages/playwright-core/src/client/browser.ts index 1884752ea0..5031656dc7 100644 --- a/packages/playwright-core/src/client/browser.ts +++ b/packages/playwright-core/src/client/browser.ts @@ -84,6 +84,7 @@ export class Browser extends ChannelOwner implements ap const contextOptions = await prepareBrowserContextParams(this._platform, options); const response = forReuse ? await this._channel.newContextForReuse(contextOptions) : await this._channel.newContext(contextOptions); const context = BrowserContext.from(response.context); + await context._initializeHarFromOptions(options.recordHar); await this._browserType._didCreateContext(context, contextOptions, this._options, options.logger || this._logger); return context; } diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 7d89f87ac1..0862264e32 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -156,10 +156,19 @@ export class BrowserContext extends ChannelOwner ])); } + async _initializeHarFromOptions(recordHar: BrowserContextOptions['recordHar']) { + if (!recordHar) + return; + const defaultContent = recordHar.path.endsWith('.zip') ? 'attach' : 'embed'; + await this._recordIntoHAR(recordHar.path, null, { + url: recordHar.urlFilter, + updateContent: recordHar.content ?? (recordHar.omitContent ? 'omit' : defaultContent), + updateMode: recordHar.mode ?? 'full', + }); + } + _setOptions(contextOptions: channels.BrowserNewContextParams, browserOptions: LaunchOptions) { this._options = contextOptions; - if (this._options.recordHar) - this._harRecorders.set('', { path: this._options.recordHar.path, content: this._options.recordHar.content }); this.tracing._tracesDir = browserOptions.tracesDir; } @@ -343,15 +352,17 @@ export class BrowserContext extends ChannelOwner await this._updateWebSocketInterceptionPatterns(); } - async _recordIntoHAR(har: string, page: Page | null, options: { url?: string | RegExp, notFound?: 'abort' | 'fallback', update?: boolean, updateContent?: 'attach' | 'embed', updateMode?: 'minimal' | 'full'} = {}): Promise { + async _recordIntoHAR(har: string, page: Page | null, options: { url?: string | RegExp, updateContent?: 'attach' | 'embed' | 'omit', updateMode?: 'minimal' | 'full'} = {}): Promise { const { harId } = await this._channel.harStart({ page: page?._channel, - options: prepareRecordHarOptions({ - path: har, + options: { + zip: har.endsWith('.zip'), content: options.updateContent ?? 'attach', + urlGlob: isString(options.url) ? options.url : undefined, + urlRegexSource: isRegExp(options.url) ? options.url.source : undefined, + urlRegexFlags: isRegExp(options.url) ? options.url.flags : undefined, mode: options.updateMode ?? 'minimal', - urlFilter: options.url - })! + }, }); this._harRecorders.set(harId, { path: har, content: options.updateContent ?? 'attach' }); } @@ -512,19 +523,6 @@ async function prepareStorageState(platform: Platform, options: BrowserContextOp } } -function prepareRecordHarOptions(options: BrowserContextOptions['recordHar']): channels.RecordHarOptions | undefined { - if (!options) - return; - return { - path: options.path, - content: options.content || (options.omitContent ? 'omit' : undefined), - urlGlob: isString(options.urlFilter) ? options.urlFilter : undefined, - urlRegexSource: isRegExp(options.urlFilter) ? options.urlFilter.source : undefined, - urlRegexFlags: isRegExp(options.urlFilter) ? options.urlFilter.flags : undefined, - mode: options.mode - }; -} - export async function prepareBrowserContextParams(platform: Platform, options: BrowserContextOptions): Promise { if (options.videoSize && !options.videosPath) throw new Error(`"videoSize" option requires "videosPath" to be specified`); @@ -537,7 +535,6 @@ export async function prepareBrowserContextParams(platform: Platform, options: B extraHTTPHeaders: options.extraHTTPHeaders ? headersObjectToArray(options.extraHTTPHeaders) : undefined, storageState: await prepareStorageState(platform, options), serviceWorkers: options.serviceWorkers, - recordHar: prepareRecordHarOptions(options.recordHar), colorScheme: options.colorScheme === null ? 'no-override' : options.colorScheme, reducedMotion: options.reducedMotion === null ? 'no-override' : options.reducedMotion, forcedColors: options.forcedColors === null ? 'no-override' : options.forcedColors, diff --git a/packages/playwright-core/src/client/browserType.ts b/packages/playwright-core/src/client/browserType.ts index da0cc5a9d8..b0dc15ab11 100644 --- a/packages/playwright-core/src/client/browserType.ts +++ b/packages/playwright-core/src/client/browserType.ts @@ -113,6 +113,7 @@ export class BrowserType extends ChannelOwner imple return await this._wrapApiCall(async () => { const result = await this._channel.launchPersistentContext(persistentParams); const context = BrowserContext.from(result.context); + await context._initializeHarFromOptions(options.recordHar); await this._didCreateContext(context, contextParams, options, logger); return context; }); diff --git a/packages/playwright-core/src/client/electron.ts b/packages/playwright-core/src/client/electron.ts index b3b509a11b..e67ba70b86 100644 --- a/packages/playwright-core/src/client/electron.ts +++ b/packages/playwright-core/src/client/electron.ts @@ -60,6 +60,7 @@ export class Electron extends ChannelOwner implements timeout: new TimeoutSettings(this._platform).launchTimeout(options), }; const app = ElectronApplication.from((await this._channel.launch(params)).electronApplication); + await app._context._initializeHarFromOptions(options.recordHar); app._context._setOptions(params, options); return app; } diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index f2e3475894..fe2d07b8e8 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -193,7 +193,7 @@ scheme.SerializedError = tObject({ value: tOptional(tType('SerializedValue')), }); scheme.RecordHarOptions = tObject({ - path: tString, + zip: tOptional(tBoolean), content: tOptional(tEnum(['embed', 'attach', 'omit'])), mode: tOptional(tEnum(['full', 'minimal'])), urlGlob: tOptional(tString), @@ -632,7 +632,6 @@ scheme.BrowserTypeLaunchPersistentContextParams = tObject({ height: tNumber, })), })), - recordHar: tOptional(tType('RecordHarOptions')), strictSelectors: tOptional(tBoolean), serviceWorkers: tOptional(tEnum(['allow', 'block'])), selectorEngines: tOptional(tArray(tType('SelectorEngine'))), @@ -721,7 +720,6 @@ scheme.BrowserNewContextParams = tObject({ height: tNumber, })), })), - recordHar: tOptional(tType('RecordHarOptions')), strictSelectors: tOptional(tBoolean), serviceWorkers: tOptional(tEnum(['allow', 'block'])), selectorEngines: tOptional(tArray(tType('SelectorEngine'))), @@ -793,7 +791,6 @@ scheme.BrowserNewContextForReuseParams = tObject({ height: tNumber, })), })), - recordHar: tOptional(tType('RecordHarOptions')), strictSelectors: tOptional(tBoolean), serviceWorkers: tOptional(tEnum(['allow', 'block'])), selectorEngines: tOptional(tArray(tType('SelectorEngine'))), @@ -2459,7 +2456,6 @@ scheme.ElectronLaunchParams = tObject({ ignoreHTTPSErrors: tOptional(tBoolean), locale: tOptional(tString), offline: tOptional(tBoolean), - recordHar: tOptional(tType('RecordHarOptions')), recordVideo: tOptional(tObject({ dir: tString, size: tOptional(tObject({ @@ -2699,7 +2695,6 @@ scheme.AndroidDeviceLaunchBrowserParams = tObject({ height: tNumber, })), })), - recordHar: tOptional(tType('RecordHarOptions')), strictSelectors: tOptional(tBoolean), serviceWorkers: tOptional(tEnum(['allow', 'block'])), selectorEngines: tOptional(tArray(tType('SelectorEngine'))), diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index c8172ee0ed..c1d38e37e6 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -109,10 +109,6 @@ export abstract class BrowserContext extends SdkObject { this._selectors = new Selectors(options.selectorEngines || [], options.testIdAttributeName); this.fetchRequest = new BrowserContextAPIRequestContext(this); - - if (this._options.recordHar) - this._harRecorders.set('', new HarRecorder(this, null, this._options.recordHar)); - this.tracing = new Tracing(this, browser.options.tracesDir); this.clock = new Clock(this); } diff --git a/packages/playwright-core/src/server/har/harRecorder.ts b/packages/playwright-core/src/server/har/harRecorder.ts index 3edcf8f787..7d99a37790 100644 --- a/packages/playwright-core/src/server/har/harRecorder.ts +++ b/packages/playwright-core/src/server/har/harRecorder.ts @@ -42,7 +42,7 @@ export class HarRecorder implements HarTracerDelegate { constructor(context: BrowserContext, page: Page | null, options: channels.RecordHarOptions) { this._artifact = new Artifact(context, path.join(context._browser.options.artifactsDir, `${createGuid()}.har`)); const urlFilterRe = options.urlRegexSource !== undefined && options.urlRegexFlags !== undefined ? new RegExp(options.urlRegexSource, options.urlRegexFlags) : undefined; - const expectsZip = options.path.endsWith('.zip'); + const expectsZip = !!options.zip; const content = options.content || (expectsZip ? 'attach' : 'embed'); this._tracer = new HarTracer(context, page, this, { content, diff --git a/packages/playwright-core/src/server/har/harTracer.ts b/packages/playwright-core/src/server/har/harTracer.ts index b1ff48b73d..2efce8ecc5 100644 --- a/packages/playwright-core/src/server/har/harTracer.ts +++ b/packages/playwright-core/src/server/har/harTracer.ts @@ -106,6 +106,8 @@ export class HarTracer { eventsHelper.addEventListener(this._context, BrowserContext.Events.RequestFulfilled, request => this._onRequestFulfilled(request)), eventsHelper.addEventListener(this._context, BrowserContext.Events.RequestContinued, request => this._onRequestContinued(request)), ); + for (const page of this._context.pages()) + this._createPageEntryIfNeeded(page); } } diff --git a/packages/protocol/src/channels.d.ts b/packages/protocol/src/channels.d.ts index ca60fb2146..d3b1c29b84 100644 --- a/packages/protocol/src/channels.d.ts +++ b/packages/protocol/src/channels.d.ts @@ -324,7 +324,7 @@ export type SerializedError = { }; export type RecordHarOptions = { - path: string, + zip?: boolean, content?: 'embed' | 'attach' | 'omit', mode?: 'full' | 'minimal', urlGlob?: string, @@ -1045,7 +1045,6 @@ export type BrowserTypeLaunchPersistentContextParams = { height: number, }, }, - recordHar?: RecordHarOptions, strictSelectors?: boolean, serviceWorkers?: 'allow' | 'block', selectorEngines?: SelectorEngine[], @@ -1129,7 +1128,6 @@ export type BrowserTypeLaunchPersistentContextOptions = { height: number, }, }, - recordHar?: RecordHarOptions, strictSelectors?: boolean, serviceWorkers?: 'allow' | 'block', selectorEngines?: SelectorEngine[], @@ -1246,7 +1244,6 @@ export type BrowserNewContextParams = { height: number, }, }, - recordHar?: RecordHarOptions, strictSelectors?: boolean, serviceWorkers?: 'allow' | 'block', selectorEngines?: SelectorEngine[], @@ -1315,7 +1312,6 @@ export type BrowserNewContextOptions = { height: number, }, }, - recordHar?: RecordHarOptions, strictSelectors?: boolean, serviceWorkers?: 'allow' | 'block', selectorEngines?: SelectorEngine[], @@ -1387,7 +1383,6 @@ export type BrowserNewContextForReuseParams = { height: number, }, }, - recordHar?: RecordHarOptions, strictSelectors?: boolean, serviceWorkers?: 'allow' | 'block', selectorEngines?: SelectorEngine[], @@ -1456,7 +1451,6 @@ export type BrowserNewContextForReuseOptions = { height: number, }, }, - recordHar?: RecordHarOptions, strictSelectors?: boolean, serviceWorkers?: 'allow' | 'block', selectorEngines?: SelectorEngine[], @@ -4329,7 +4323,6 @@ export type ElectronLaunchParams = { ignoreHTTPSErrors?: boolean, locale?: string, offline?: boolean, - recordHar?: RecordHarOptions, recordVideo?: { dir: string, size?: { @@ -4363,7 +4356,6 @@ export type ElectronLaunchOptions = { ignoreHTTPSErrors?: boolean, locale?: string, offline?: boolean, - recordHar?: RecordHarOptions, recordVideo?: { dir: string, size?: { @@ -4755,7 +4747,6 @@ export type AndroidDeviceLaunchBrowserParams = { height: number, }, }, - recordHar?: RecordHarOptions, strictSelectors?: boolean, serviceWorkers?: 'allow' | 'block', selectorEngines?: SelectorEngine[], @@ -4822,7 +4813,6 @@ export type AndroidDeviceLaunchBrowserOptions = { height: number, }, }, - recordHar?: RecordHarOptions, strictSelectors?: boolean, serviceWorkers?: 'allow' | 'block', selectorEngines?: SelectorEngine[], diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index f4c81d8a0c..f3f6fdaad3 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -324,7 +324,7 @@ SerializedError: RecordHarOptions: type: object properties: - path: string + zip: boolean? content: type: enum? literals: @@ -611,7 +611,6 @@ ContextOptions: properties: width: number height: number - recordHar: RecordHarOptions? strictSelectors: boolean? serviceWorkers: type: enum? @@ -3724,7 +3723,6 @@ Electron: ignoreHTTPSErrors: boolean? locale: string? offline: boolean? - recordHar: RecordHarOptions? recordVideo: type: object? properties: diff --git a/tests/library/har.spec.ts b/tests/library/har.spec.ts index 88523e6ebb..82c1148289 100644 --- a/tests/library/har.spec.ts +++ b/tests/library/har.spec.ts @@ -42,11 +42,6 @@ async function pageWithHar(contextFactory: (options?: BrowserContextOptions) => }; } -it('should throw without path', async ({ browser }) => { - const error = await browser.newContext({ recordHar: {} as any }).catch(e => e); - expect(error.message).toContain('recordHar.path: expected string, got undefined'); -}); - it('should have version and creator', async ({ contextFactory, server }, testInfo) => { const { page, getLog } = await pageWithHar(contextFactory, testInfo); await page.goto(server.EMPTY_PAGE); @@ -90,17 +85,8 @@ it('should have pages in persistent context', async ({ launchPersistent, browser await page.waitForLoadState('domcontentloaded'); await context.close(); const log = JSON.parse(fs.readFileSync(harPath).toString())['log']; - let pageEntry; - if (browserName === 'webkit') { - // Explicit locale emulation forces a new page creation when - // doing a new context. - // See https://github.com/microsoft/playwright/blob/13dd41c2e36a63f35ddef5dc5dec322052d670c6/packages/playwright-core/src/server/browserContext.ts#L232-L242 - expect(log.pages.length).toBe(2); - pageEntry = log.pages[1]; - } else { - expect(log.pages.length).toBe(1); - pageEntry = log.pages[0]; - } + expect(log.pages.length).toBe(1); + const pageEntry = log.pages[0]; expect(pageEntry.id).toBeTruthy(); expect(pageEntry.title).toBe('Hello'); });