From cd110e64777705065da7f0d7eedf7d3861e84782 Mon Sep 17 00:00:00 2001 From: Ross Wollman Date: Wed, 25 Aug 2021 13:32:56 -0700 Subject: [PATCH] feat(har): Remotely accessible HAR file (#8385) This change ensure's the HAR file is saved at `recordHar.path` on the client instead of the server. NB: The goal was to make this change transparent to the user and NOT introduce any new APIs. Namely, I want to leave the API open for potential `context.har.start()` and `context.har.stop()`. This does BREAK servers that expect the HAR to be at the `recordHar.path` on the server, but I think that's OK since there haven't been reports of missing HAR on client making me think not many users are getting HAR with client and server on different hosts anyways. Closes #8355 --- src/client/browserContext.ts | 4 +++ src/dispatchers/browserContextDispatcher.ts | 7 +++++ src/protocol/channels.ts | 6 ++++ src/protocol/protocol.yml | 4 +++ src/protocol/validator.ts | 1 + src/server/browserContext.ts | 5 +-- src/server/supplements/har/harRecorder.ts | 13 ++++++++ tests/har.spec.ts | 34 +++++++++++++++++++-- 8 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/client/browserContext.ts b/src/client/browserContext.ts index 30d3ee66f3..6d2377e745 100644 --- a/src/client/browserContext.ts +++ b/src/client/browserContext.ts @@ -344,6 +344,10 @@ export class BrowserContext extends ChannelOwner { await this._browserType?._onWillCloseContext?.(this); + if (this._options.recordHar) { + const har = await this._channel.harExport(); + await har.artifact.saveAs({ path: this._options.recordHar.path }); + } await channel.close(); await this._closedPromise; }); diff --git a/src/dispatchers/browserContextDispatcher.ts b/src/dispatchers/browserContextDispatcher.ts index eb3d6c6042..861d1bbb98 100644 --- a/src/dispatchers/browserContextDispatcher.ts +++ b/src/dispatchers/browserContextDispatcher.ts @@ -217,4 +217,11 @@ export class BrowserContextDispatcher extends Dispatcher { + const artifact = await this._context._harRecorder?.export(); + if (!artifact) + throw new Error('No HAR artifact. Ensure record.harPath is set.'); + return { artifact: new ArtifactDispatcher(this._scope, artifact) }; + } } diff --git a/src/protocol/channels.ts b/src/protocol/channels.ts index 2f45338f61..e3fbc81622 100644 --- a/src/protocol/channels.ts +++ b/src/protocol/channels.ts @@ -731,6 +731,7 @@ export interface BrowserContextChannel extends EventTargetChannel { tracingStart(params: BrowserContextTracingStartParams, metadata?: Metadata): Promise; tracingStop(params?: BrowserContextTracingStopParams, metadata?: Metadata): Promise; tracingExport(params?: BrowserContextTracingExportParams, metadata?: Metadata): Promise; + harExport(params?: BrowserContextHarExportParams, metadata?: Metadata): Promise; } export type BrowserContextBindingCallEvent = { binding: BindingCallChannel, @@ -962,6 +963,11 @@ export type BrowserContextTracingExportOptions = {}; export type BrowserContextTracingExportResult = { artifact: ArtifactChannel, }; +export type BrowserContextHarExportParams = {}; +export type BrowserContextHarExportOptions = {}; +export type BrowserContextHarExportResult = { + artifact: ArtifactChannel, +}; // ----------- Page ----------- export type PageInitializer = { diff --git a/src/protocol/protocol.yml b/src/protocol/protocol.yml index dc80f2fd44..f243c00cae 100644 --- a/src/protocol/protocol.yml +++ b/src/protocol/protocol.yml @@ -707,6 +707,10 @@ BrowserContext: returns: artifact: Artifact + harExport: + returns: + artifact: Artifact + events: bindingCall: diff --git a/src/protocol/validator.ts b/src/protocol/validator.ts index 66ef420f36..de2530935b 100644 --- a/src/protocol/validator.ts +++ b/src/protocol/validator.ts @@ -448,6 +448,7 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { }); scheme.BrowserContextTracingStopParams = tOptional(tObject({})); scheme.BrowserContextTracingExportParams = tOptional(tObject({})); + scheme.BrowserContextHarExportParams = tOptional(tObject({})); scheme.PageSetDefaultNavigationTimeoutNoReplyParams = tObject({ timeout: tNumber, }); diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts index 8dc5f97d2d..5d646ab3eb 100644 --- a/src/server/browserContext.ts +++ b/src/server/browserContext.ts @@ -61,7 +61,7 @@ export abstract class BrowserContext extends SdkObject { readonly _browserContextId: string | undefined; private _selectors?: Selectors; private _origins = new Set(); - private _harRecorder: HarRecorder | undefined; + readonly _harRecorder: HarRecorder | undefined; readonly tracing: Tracing; constructor(browser: Browser, options: types.BrowserContextOptions, browserContextId: string | undefined) { @@ -74,7 +74,8 @@ export abstract class BrowserContext extends SdkObject { this._closePromise = new Promise(fulfill => this._closePromiseFulfill = fulfill); if (this._options.recordHar) - this._harRecorder = new HarRecorder(this, this._options.recordHar); + this._harRecorder = new HarRecorder(this, {...this._options.recordHar, path: path.join(this._browser.options.artifactsDir, `${createGuid()}.har`)}); + this.tracing = new Tracing(this); } diff --git a/src/server/supplements/har/harRecorder.ts b/src/server/supplements/har/harRecorder.ts index c903efc343..31789cfab3 100644 --- a/src/server/supplements/har/harRecorder.ts +++ b/src/server/supplements/har/harRecorder.ts @@ -15,6 +15,7 @@ */ import fs from 'fs'; +import { Artifact } from '../../artifact'; import { BrowserContext } from '../../browserContext'; import * as har from './har'; import { HarTracer } from './harTracer'; @@ -25,11 +26,14 @@ type HarOptions = { }; export class HarRecorder { + private _artifact: Artifact; + private _isFlushed: boolean = false; private _options: HarOptions; private _tracer: HarTracer; private _entries: har.Entry[] = []; constructor(context: BrowserContext, options: HarOptions) { + this._artifact = new Artifact(context, options.path); this._options = options; this._tracer = new HarTracer(context, this, { content: options.omitContent ? 'omit' : 'embedded', @@ -50,8 +54,17 @@ export class HarRecorder { } async flush() { + if (this._isFlushed) + return; + this._isFlushed = true; const log = await this._tracer.stop(); log.entries = this._entries; await fs.promises.writeFile(this._options.path, JSON.stringify({ log }, undefined, 2)); } + + async export(): Promise { + await this.flush(); + this._artifact.reportFinished(); + return this._artifact; + } } diff --git a/tests/har.spec.ts b/tests/har.spec.ts index 42c6320e31..9b818b87ad 100644 --- a/tests/har.spec.ts +++ b/tests/har.spec.ts @@ -23,8 +23,8 @@ import type { BrowserContext, BrowserContextOptions } from '../index'; import type { AddressInfo } from 'net'; import type { Log } from '../src/server/supplements/har/har'; -async function pageWithHar(contextFactory: (options?: BrowserContextOptions) => Promise, testInfo: any) { - const harPath = testInfo.outputPath('test.har'); +async function pageWithHar(contextFactory: (options?: BrowserContextOptions) => Promise, testInfo: any, outputPath: string = 'test.har') { + const harPath = testInfo.outputPath(outputPath); const context = await contextFactory({ recordHar: { path: harPath }, ignoreHTTPSErrors: true }); const page = await context.newPage(); return { @@ -474,3 +474,33 @@ it('should contain http2 for http2 requests', async ({ contextFactory, browserNa expect(log.entries[0].response.httpVersion).toBe('h2'); server.close(); }); + +it('should have different hars for concurrent contexts', async ({ contextFactory }, testInfo) => { + const session0 = await pageWithHar(contextFactory, testInfo, 'test-0.har'); + await session0.page.goto('data:text/html,Zero'); + await session0.page.waitForLoadState('domcontentloaded'); + + const session1 = await pageWithHar(contextFactory, testInfo, 'test-1.har'); + await session1.page.goto('data:text/html,One'); + await session1.page.waitForLoadState('domcontentloaded'); + + // Trigger flushing on the server and ensure they are not racing to same + // location. NB: Run this test with --repeat-each 10. + const [log0, log1] = await Promise.all([ + session0.getLog(), + session1.getLog() + ]); + + { + expect(log0.pages.length).toBe(1); + const pageEntry = log0.pages[0]; + expect(pageEntry.title).toBe('Zero'); + } + + { + expect(log1.pages.length).toBe(1); + const pageEntry = log1.pages[0]; + expect(pageEntry.id).not.toBe(log0.pages[0].id); + expect(pageEntry.title).toBe('One'); + } +});