diff --git a/src/client/tracing.ts b/src/client/tracing.ts index bf3546bb9c..d845e6a5d8 100644 --- a/src/client/tracing.ts +++ b/src/client/tracing.ts @@ -32,12 +32,6 @@ export class Tracing implements api.Tracing { }); } - async _reset() { - await this._context._wrapApiCall(async (channel: channels.BrowserContextChannel) => { - return await channel.tracingReset(); - }); - } - async _export(options: { path: string }) { await this._context._wrapApiCall(async (channel: channels.BrowserContextChannel) => { await this._doExport(channel, options.path); @@ -46,9 +40,9 @@ export class Tracing implements api.Tracing { async stop(options: { path?: string } = {}) { await this._context._wrapApiCall(async (channel: channels.BrowserContextChannel) => { - await channel.tracingStop(); if (options.path) await this._doExport(channel, options.path); + await channel.tracingStop(); }); } diff --git a/src/dispatchers/browserContextDispatcher.ts b/src/dispatchers/browserContextDispatcher.ts index 74e2b32fee..535e15c6c4 100644 --- a/src/dispatchers/browserContextDispatcher.ts +++ b/src/dispatchers/browserContextDispatcher.ts @@ -184,10 +184,6 @@ export class BrowserContextDispatcher extends Dispatcher { - await this._context.tracing.reset(); - } - async tracingStop(params: channels.BrowserContextTracingStopParams): Promise { await this._context.tracing.stop(); } diff --git a/src/protocol/channels.ts b/src/protocol/channels.ts index c756d85697..4af0bbb277 100644 --- a/src/protocol/channels.ts +++ b/src/protocol/channels.ts @@ -656,7 +656,6 @@ export interface BrowserContextChannel extends EventTargetChannel { recorderSupplementEnable(params: BrowserContextRecorderSupplementEnableParams, metadata?: Metadata): Promise; newCDPSession(params: BrowserContextNewCDPSessionParams, metadata?: Metadata): Promise; tracingStart(params: BrowserContextTracingStartParams, metadata?: Metadata): Promise; - tracingReset(params?: BrowserContextTracingResetParams, metadata?: Metadata): Promise; tracingStop(params?: BrowserContextTracingStopParams, metadata?: Metadata): Promise; tracingExport(params?: BrowserContextTracingExportParams, metadata?: Metadata): Promise; } @@ -865,9 +864,6 @@ export type BrowserContextTracingStartOptions = { screenshots?: boolean, }; export type BrowserContextTracingStartResult = void; -export type BrowserContextTracingResetParams = {}; -export type BrowserContextTracingResetOptions = {}; -export type BrowserContextTracingResetResult = void; export type BrowserContextTracingStopParams = {}; export type BrowserContextTracingStopOptions = {}; export type BrowserContextTracingStopResult = void; diff --git a/src/protocol/protocol.yml b/src/protocol/protocol.yml index 72fc5f1f62..d84a70c815 100644 --- a/src/protocol/protocol.yml +++ b/src/protocol/protocol.yml @@ -639,8 +639,6 @@ BrowserContext: snapshots: boolean? screenshots: boolean? - tracingReset: - tracingStop: tracingExport: diff --git a/src/protocol/validator.ts b/src/protocol/validator.ts index df92f6a6c0..941e5da705 100644 --- a/src/protocol/validator.ts +++ b/src/protocol/validator.ts @@ -413,7 +413,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { snapshots: tOptional(tBoolean), screenshots: tOptional(tBoolean), }); - scheme.BrowserContextTracingResetParams = tOptional(tObject({})); scheme.BrowserContextTracingStopParams = tOptional(tObject({})); scheme.BrowserContextTracingExportParams = tOptional(tObject({})); scheme.PageSetDefaultNavigationTimeoutNoReplyParams = tObject({ diff --git a/src/server/snapshot/inMemorySnapshotter.ts b/src/server/snapshot/inMemorySnapshotter.ts index 4ec3d40031..a7be11213f 100644 --- a/src/server/snapshot/inMemorySnapshotter.ts +++ b/src/server/snapshot/inMemorySnapshotter.ts @@ -42,6 +42,11 @@ export class InMemorySnapshotter extends BaseSnapshotStorage implements Snapshot return await this._server.start(); } + async reset() { + await this._snapshotter.reset(); + this.clear(); + } + async dispose() { this._snapshotter.dispose(); await this._server.stop(); diff --git a/src/server/snapshot/snapshotterInjected.ts b/src/server/snapshot/snapshotterInjected.ts index 46dcd10f60..2004c556f8 100644 --- a/src/server/snapshot/snapshotterInjected.ts +++ b/src/server/snapshot/snapshotterInjected.ts @@ -177,6 +177,7 @@ export function frameSnapshotStreamer(snapshotStreamer: string) { visitNode(child); }; visitNode(document.documentElement); + visitNode(this._fakeBase); } private _sanitizeUrl(url: string): string { diff --git a/src/server/trace/recorder/tracing.ts b/src/server/trace/recorder/tracing.ts index 6e168a0fc3..60cd64e5eb 100644 --- a/src/server/trace/recorder/tracing.ts +++ b/src/server/trace/recorder/tracing.ts @@ -38,18 +38,24 @@ export type TracerOptions = { export const VERSION = 1; +type RecordingState = { + options: TracerOptions, + traceFile: string, + lastReset: number, + sha1s: Set, +}; + +const kScreencastOptions = { width: 800, height: 600, quality: 90 }; + export class Tracing implements InstrumentationListener { private _appendEventChain = Promise.resolve(); private _snapshotter: TraceSnapshotter; - private _eventListeners: RegisteredListener[] = []; + private _screencastListeners: RegisteredListener[] = []; private _pendingCalls = new Map, actionSnapshot?: Promise, afterSnapshot?: Promise }>(); private _context: BrowserContext; - private _traceFile: string | undefined; private _resourcesDir: string; - private _sha1s = new Set(); - private _recordingTraceEvents = false; + private _recording: RecordingState | undefined; private _tracesDir: string; - private _lastReset = 0; constructor(context: BrowserContext) { this._context = context; @@ -60,63 +66,77 @@ export class Tracing implements InstrumentationListener { async start(options: TracerOptions): Promise { // context + page must be the first events added, this method can't have awaits before them. - if (this._recordingTraceEvents) - throw new Error('Tracing has already been started'); - this._recordingTraceEvents = true; - // TODO: passing the same name for two contexts makes them write into a single file - // and conflict. - this._traceFile = path.join(this._tracesDir, (options.name || createGuid()) + '.trace'); - this._lastReset = 0; - this._appendEventChain = mkdirIfNeeded(this._traceFile); - const event: trace.ContextCreatedTraceEvent = { - version: VERSION, - type: 'context-options', - browserName: this._context._browser.options.name, - options: this._context._options - }; - this._appendTraceEvent(event); - for (const page of this._context.pages()) - this._onPage(options.screenshots, page); - this._eventListeners.push( - eventsHelper.addEventListener(this._context, BrowserContext.Events.Page, this._onPage.bind(this, options.screenshots)), - ); + const state = this._recording; + if (!state) { + // TODO: passing the same name for two contexts makes them write into a single file + // and conflict. + const traceFile = path.join(this._tracesDir, (options.name || createGuid()) + '.trace'); + this._recording = { options, traceFile, lastReset: 0, sha1s: new Set() }; + this._appendEventChain = mkdirIfNeeded(traceFile); + const event: trace.ContextCreatedTraceEvent = { + version: VERSION, + type: 'context-options', + browserName: this._context._browser.options.name, + options: this._context._options + }; + this._appendTraceEvent(event); + } + + if (!state?.options?.screenshots && options.screenshots) + this._startScreencast(); + else if (state?.options?.screenshots && !options.screenshots) + this._stopScreencast(); // context + page must be the first events added, no awaits above this line. await fs.promises.mkdir(this._resourcesDir, { recursive: true }); - this._context.instrumentation.addListener(this); - if (options.snapshots) - await this._snapshotter.start(); + if (!state) + this._context.instrumentation.addListener(this); + + await this._appendTraceOperation(async () => { + if (options.snapshots && state?.options?.snapshots) { + // Reset snapshots to avoid back-references. + await this._snapshotter.reset(); + } else if (options.snapshots) { + await this._snapshotter.start(); + } else if (state?.options?.snapshots) { + await this._snapshotter.stop(); + } + + if (state) { + state.lastReset++; + const markerEvent: trace.MarkerTraceEvent = { type: 'marker', resetIndex: state.lastReset }; + await fs.promises.appendFile(state.traceFile, JSON.stringify(markerEvent) + '\n'); + } + }); + + if (this._recording) + this._recording.options = options; } - async reset(): Promise { - await this._appendTraceOperation(async () => { - // Reset snapshots to avoid back-references. - await this._snapshotter.reset(); - this._lastReset++; - const markerEvent: trace.MarkerTraceEvent = { type: 'marker', resetIndex: this._lastReset }; - await fs.promises.appendFile(this._traceFile!, JSON.stringify(markerEvent) + '\n'); - }); + private _startScreencast() { + for (const page of this._context.pages()) + this._startScreencastInPage(page); + this._screencastListeners.push( + eventsHelper.addEventListener(this._context, BrowserContext.Events.Page, this._startScreencastInPage.bind(this)), + ); + } + + private _stopScreencast() { + eventsHelper.removeEventListeners(this._screencastListeners); + for (const page of this._context.pages()) + page.setScreencastOptions(null); } async stop(): Promise { - if (!this._eventListeners.length) + if (!this._recording) return; + this._recording = undefined; this._context.instrumentation.removeListener(this); - eventsHelper.removeEventListeners(this._eventListeners); - for (const { sdkObject, metadata, beforeSnapshot, actionSnapshot, afterSnapshot } of this._pendingCalls.values()) { - await Promise.all([beforeSnapshot, actionSnapshot, afterSnapshot]); - if (!afterSnapshot) - metadata.error = { error: { name: 'Error', message: 'Action was interrupted' } }; - await this.onAfterCall(sdkObject, metadata); - } - for (const page of this._context.pages()) - page.setScreencastOptions(null); + this._stopScreencast(); await this._snapshotter.stop(); - // Ensure all writes are finished. - this._recordingTraceEvents = false; await this._appendEventChain; } @@ -125,25 +145,42 @@ export class Tracing implements InstrumentationListener { } async export(): Promise { - if (!this._traceFile) + if (!this._recording) throw new Error('Must start tracing before exporting'); + + for (const { sdkObject, metadata, beforeSnapshot, actionSnapshot, afterSnapshot } of this._pendingCalls.values()) { + await Promise.all([beforeSnapshot, actionSnapshot, afterSnapshot]); + let callMetadata = metadata; + if (!afterSnapshot) { + // Note: we should not modify metadata here to avoid side-effects in any other place. + callMetadata = { + ...metadata, + error: { error: { name: 'Error', message: 'Action was interrupted' } }, + }; + } + await this.onAfterCall(sdkObject, callMetadata); + } + // Chain the export operation against write operations, // so that neither trace file nor sha1s change during the export. return await this._appendTraceOperation(async () => { + if (!this._recording) + throw new Error('Must start tracing before exporting'); + await this._snapshotter.checkpoint(); - const resetIndex = this._lastReset; - let trace = { file: this._traceFile!, sha1s: this._sha1s }; + const resetIndex = this._recording.lastReset; + let state = this._recording; // Make a filtered trace if needed. if (resetIndex) - trace = await this._filterTrace(this._traceFile!, resetIndex); + state = await this._filterTrace(this._recording, resetIndex); const zipFile = new yazl.ZipFile(); const failedPromise = new Promise((_, reject) => (zipFile as any as EventEmitter).on('error', reject)); const succeededPromise = new Promise(async fulfill => { - zipFile.addFile(trace.file, 'trace.trace'); - const zipFileName = trace.file + '.zip'; - for (const sha1 of trace.sha1s) + zipFile.addFile(state.traceFile, 'trace.trace'); + const zipFileName = state.traceFile + '.zip'; + for (const sha1 of state.sha1s) zipFile.addFile(path.join(this._resourcesDir!, sha1), path.join('resources', sha1)); zipFile.end(); await new Promise(f => { @@ -156,17 +193,17 @@ export class Tracing implements InstrumentationListener { return Promise.race([failedPromise, succeededPromise]).finally(async () => { // Remove the filtered trace. if (resetIndex) - await fs.promises.unlink(trace.file).catch(() => {}); + await fs.promises.unlink(state.traceFile).catch(() => {}); }); }); } - private async _filterTrace(traceFile: string, resetIndex: number): Promise<{ file: string, sha1s: Set }> { - const ext = path.extname(traceFile); - const traceFileCopy = traceFile.substring(0, traceFile.length - ext.length) + '-copy' + resetIndex + ext; + private async _filterTrace(state: RecordingState, sinceResetIndex: number): Promise { + const ext = path.extname(state.traceFile); + const traceFileCopy = state.traceFile.substring(0, state.traceFile.length - ext.length) + '-copy' + sinceResetIndex + ext; const sha1s = new Set(); await new Promise((resolve, reject) => { - const fileStream = fs.createReadStream(traceFile, 'utf8'); + const fileStream = fs.createReadStream(state.traceFile, 'utf8'); const rl = readline.createInterface({ input: fileStream, crlfDelay: Infinity @@ -176,10 +213,14 @@ export class Tracing implements InstrumentationListener { rl.on('line', line => { try { const event = JSON.parse(line) as trace.TraceEvent; - if (event.type === 'marker' && event.resetIndex === resetIndex) { - foundMarker = true; - } else if (event.type === 'resource-snapshot' || event.type === 'context-options' || foundMarker) { - // We keep all resources for snapshots, context options and all events after the marker. + if (event.type === 'marker') { + if (event.resetIndex === sinceResetIndex) + foundMarker = true; + } else if ((event.type === 'resource-snapshot' && state.options.snapshots) || event.type === 'context-options' || foundMarker) { + // We keep: + // - old resource events for snapshots; + // - initial context options event; + // - all events after the marker that are not markers. visitSha1s(event, sha1s); copyChain = copyChain.then(() => fs.promises.appendFile(traceFileCopy, line + '\n')); } @@ -195,7 +236,7 @@ export class Tracing implements InstrumentationListener { resolve(); }); }); - return { file: traceFileCopy, sha1s }; + return { options: state.options, lastReset: state.lastReset, sha1s, traceFile: traceFileCopy }; } async _captureSnapshot(name: 'before' | 'after' | 'action' | 'event', sdkObject: SdkObject, metadata: CallMetadata, element?: ElementHandle) { @@ -244,11 +285,9 @@ export class Tracing implements InstrumentationListener { this._appendTraceEvent(event); } - private _onPage(screenshots: boolean | undefined, page: Page) { - if (screenshots) - page.setScreencastOptions({ width: 800, height: 600, quality: 90 }); - - this._eventListeners.push( + private _startScreencastInPage(page: Page) { + page.setScreencastOptions(kScreencastOptions); + this._screencastListeners.push( eventsHelper.addEventListener(page, Page.Events.ScreencastFrame, params => { const sha1 = calculateSha1(createGuid()); // no need to compute sha1 for screenshots const event: trace.ScreencastFrameTraceEvent = { @@ -269,12 +308,12 @@ export class Tracing implements InstrumentationListener { } private _appendTraceEvent(event: any) { - if (!this._recordingTraceEvents) - return; // Serialize all writes to the trace file. this._appendTraceOperation(async () => { - visitSha1s(event, this._sha1s); - await fs.promises.appendFile(this._traceFile!, JSON.stringify(event) + '\n'); + if (!this._recording) + return; + visitSha1s(event, this._recording.sha1s); + await fs.promises.appendFile(this._recording.traceFile, JSON.stringify(event) + '\n'); }); } diff --git a/tests/snapshotter.spec.ts b/tests/snapshotter.spec.ts index 87837731d0..738c5220aa 100644 --- a/tests/snapshotter.spec.ts +++ b/tests/snapshotter.spec.ts @@ -44,6 +44,17 @@ it.describe('snapshots', () => { expect(distillSnapshot(snapshot)).toBe(''); }); + it('should preserve BASE and other content on reset', async ({ page, toImpl, snapshotter, server }) => { + await page.goto(server.EMPTY_PAGE); + const snapshot1 = await snapshotter.captureSnapshot(toImpl(page), 'snapshot1'); + const html1 = snapshot1.render().html; + expect(html1).toContain(` { await page.goto(server.EMPTY_PAGE); await page.route('**/style.css', route => { diff --git a/tests/tracing.spec.ts b/tests/tracing.spec.ts index a4ea51acf7..95c13c8954 100644 --- a/tests/tracing.spec.ts +++ b/tests/tracing.spec.ts @@ -196,16 +196,13 @@ test('should include interrupted actions', async ({ context, page, server }, tes expect(clickEvent.metadata.error.error.message).toBe('Action was interrupted'); }); -test('should reset and export', async ({ context, page, server }, testInfo) => { +test('should reset to different options', async ({ context, page, server }, testInfo) => { await context.tracing.start({ screenshots: true, snapshots: true }); await page.goto(server.PREFIX + '/frames/frame.html'); - // @ts-expect-error - await context.tracing._reset(); + await context.tracing.start({ screenshots: false, snapshots: false }); await page.setContent(''); await page.click('"Click"'); - // @ts-expect-error - await context.tracing._export({ path: testInfo.outputPath('trace.zip') }); - await context.tracing.stop(); + await context.tracing.stop({ path: testInfo.outputPath('trace.zip') }); const { events } = await parseTrace(testInfo.outputPath('trace.zip')); expect(events[0].type).toBe('context-options'); @@ -213,10 +210,43 @@ test('should reset and export', async ({ context, page, server }, testInfo) => { expect(events.find(e => e.metadata?.apiName === 'page.setContent')).toBeTruthy(); expect(events.find(e => e.metadata?.apiName === 'page.click')).toBeTruthy(); - expect(events.some(e => e.type === 'frame-snapshot')).toBeTruthy(); - expect(events.some(e => e.type === 'resource-snapshot' && e.snapshot.url.endsWith('style.css'))).toBeTruthy(); + expect(events.some(e => e.type === 'frame-snapshot')).toBeFalsy(); + expect(events.some(e => e.type === 'resource-snapshot')).toBeFalsy(); }); +test('should reset and export', async ({ context, page, server }, testInfo) => { + await context.tracing.start({ screenshots: true, snapshots: true }); + await page.goto(server.PREFIX + '/frames/frame.html'); + + await context.tracing.start({ screenshots: true, snapshots: true }); + await page.setContent(''); + await page.click('"Click"'); + page.click('"ClickNoButton"').catch(() => {}); + // @ts-expect-error + await context.tracing._export({ path: testInfo.outputPath('trace.zip') }); + + await context.tracing.start({ screenshots: true, snapshots: true }); + await page.hover('"Click"'); + await context.tracing.stop({ path: testInfo.outputPath('trace2.zip') }); + + const trace1 = await parseTrace(testInfo.outputPath('trace.zip')); + expect(trace1.events[0].type).toBe('context-options'); + expect(trace1.events.find(e => e.metadata?.apiName === 'page.goto')).toBeFalsy(); + expect(trace1.events.find(e => e.metadata?.apiName === 'page.setContent')).toBeTruthy(); + expect(trace1.events.find(e => e.metadata?.apiName === 'page.click' && !!e.metadata.error)).toBeTruthy(); + expect(trace1.events.find(e => e.metadata?.apiName === 'page.hover')).toBeFalsy(); + expect(trace1.events.find(e => e.metadata?.apiName === 'page.click' && e.metadata?.error?.error?.message === 'Action was interrupted')).toBeTruthy(); + expect(trace1.events.some(e => e.type === 'frame-snapshot')).toBeTruthy(); + expect(trace1.events.some(e => e.type === 'resource-snapshot' && e.snapshot.url.endsWith('style.css'))).toBeTruthy(); + + const trace2 = await parseTrace(testInfo.outputPath('trace2.zip')); + expect(trace2.events[0].type).toBe('context-options'); + expect(trace2.events.find(e => e.metadata?.apiName === 'page.goto')).toBeFalsy(); + expect(trace2.events.find(e => e.metadata?.apiName === 'page.setContent')).toBeFalsy(); + expect(trace2.events.find(e => e.metadata?.apiName === 'page.click')).toBeFalsy(); + expect(trace2.events.find(e => e.metadata?.apiName === 'page.hover')).toBeTruthy(); + expect(trace2.events.some(e => e.type === 'frame-snapshot')).toBeTruthy(); +}); async function parseTrace(file: string): Promise<{ events: any[], resources: Map }> { const entries = await new Promise(f => {