feat(tracing): allow calling tracing.start multiple times (#7992)

Subsequent invocations reset the trace. This removes the `tracing._reset` method.

Also fixed a bug where BASE element was not reset properly.
Also fixed a bug where tracing would affect the result of protocol call
by setting `callMetadata.error` property.

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();
     });
   }
 

src/dispatchers/browserContextDispatcher.ts

@@ -184,10 +184,6 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
     await this._context.tracing.start(params);
   }
 
-  async tracingReset(params: channels.BrowserContextTracingResetParams): Promise<channels.BrowserContextTracingResetResult> {
-    await this._context.tracing.reset();
-  }
-
   async tracingStop(params: channels.BrowserContextTracingStopParams): Promise<channels.BrowserContextTracingStopResult> {
     await this._context.tracing.stop();
   }

src/protocol/channels.ts

@@ -656,7 +656,6 @@ export interface BrowserContextChannel extends EventTargetChannel {
   recorderSupplementEnable(params: BrowserContextRecorderSupplementEnableParams, metadata?: Metadata): Promise<BrowserContextRecorderSupplementEnableResult>;
   newCDPSession(params: BrowserContextNewCDPSessionParams, metadata?: Metadata): Promise<BrowserContextNewCDPSessionResult>;
   tracingStart(params: BrowserContextTracingStartParams, metadata?: Metadata): Promise<BrowserContextTracingStartResult>;
-  tracingReset(params?: BrowserContextTracingResetParams, metadata?: Metadata): Promise<BrowserContextTracingResetResult>;
   tracingStop(params?: BrowserContextTracingStopParams, metadata?: Metadata): Promise<BrowserContextTracingStopResult>;
   tracingExport(params?: BrowserContextTracingExportParams, metadata?: Metadata): Promise<BrowserContextTracingExportResult>;
 }
@@ -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;

src/protocol/protocol.yml

@@ -639,8 +639,6 @@ BrowserContext:
         snapshots: boolean?
         screenshots: boolean?
 
-    tracingReset:
-
     tracingStop:
 
     tracingExport:

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({

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();

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 {

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<string>,
+};
+
+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<string, { sdkObject: SdkObject, metadata: CallMetadata, beforeSnapshot: Promise<void>, actionSnapshot?: Promise<void>, afterSnapshot?: Promise<void> }>();
   private _context: BrowserContext;
-  private _traceFile: string | undefined;
   private _resourcesDir: string;
-  private _sha1s = new Set<string>();
+  private _recording: RecordingState | undefined;
-  private _recordingTraceEvents = false;
   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<void> {
     // 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 state = this._recording;
-    const event: trace.ContextCreatedTraceEvent = {
+    if (!state) {
-      version: VERSION,
+      // TODO: passing the same name for two contexts makes them write into a single file
-      type: 'context-options',
+      // and conflict.
-      browserName: this._context._browser.options.name,
+      const traceFile = path.join(this._tracesDir, (options.name || createGuid()) + '.trace');
-      options: this._context._options
+      this._recording = { options, traceFile, lastReset: 0, sha1s: new Set() };
-    };
+      this._appendEventChain = mkdirIfNeeded(traceFile);
-    this._appendTraceEvent(event);
+      const event: trace.ContextCreatedTraceEvent = {
-    for (const page of this._context.pages())
+        version: VERSION,
-      this._onPage(options.screenshots, page);
+        type: 'context-options',
-    this._eventListeners.push(
+        browserName: this._context._browser.options.name,
-        eventsHelper.addEventListener(this._context, BrowserContext.Events.Page, this._onPage.bind(this, options.screenshots)),
+        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 (!state)
-    if (options.snapshots)
+      this._context.instrumentation.addListener(this);
-      await this._snapshotter.start();
+
+    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<void> {
+  private _startScreencast() {
-    await this._appendTraceOperation(async () => {
+    for (const page of this._context.pages())
-      // Reset snapshots to avoid back-references.
+      this._startScreencastInPage(page);
-      await this._snapshotter.reset();
+    this._screencastListeners.push(
-      this._lastReset++;
+        eventsHelper.addEventListener(this._context, BrowserContext.Events.Page, this._startScreencastInPage.bind(this)),
-      const markerEvent: trace.MarkerTraceEvent = { type: 'marker', resetIndex: this._lastReset };
+    );
-      await fs.promises.appendFile(this._traceFile!, JSON.stringify(markerEvent) + '\n');
+  }
-    });
+
+  private _stopScreencast() {
+    eventsHelper.removeEventListeners(this._screencastListeners);
+    for (const page of this._context.pages())
+      page.setScreencastOptions(null);
   }
 
   async stop(): Promise<void> {
-    if (!this._eventListeners.length)
+    if (!this._recording)
       return;
+    this._recording = undefined;
     this._context.instrumentation.removeListener(this);
-    eventsHelper.removeEventListeners(this._eventListeners);
+    this._stopScreencast();
-    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);
     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<Artifact> {
-    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;
+      const resetIndex = this._recording.lastReset;
-      let trace = { file: this._traceFile!, sha1s: this._sha1s };
+      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<Artifact>((_, reject) => (zipFile as any as EventEmitter).on('error', reject));
       const succeededPromise = new Promise<Artifact>(async fulfill => {
-        zipFile.addFile(trace.file, 'trace.trace');
+        zipFile.addFile(state.traceFile, 'trace.trace');
-        const zipFileName = trace.file + '.zip';
+        const zipFileName = state.traceFile + '.zip';
-        for (const sha1 of trace.sha1s)
+        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<string> }> {
+  private async _filterTrace(state: RecordingState, sinceResetIndex: number): Promise<RecordingState> {
-    const ext = path.extname(traceFile);
+    const ext = path.extname(state.traceFile);
-    const traceFileCopy = traceFile.substring(0, traceFile.length - ext.length) + '-copy' + resetIndex + ext;
+    const traceFileCopy = state.traceFile.substring(0, state.traceFile.length - ext.length) + '-copy' + sinceResetIndex + ext;
     const sha1s = new Set<string>();
     await new Promise<void>((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) {
+          if (event.type === 'marker') {
-            foundMarker = true;
+            if (event.resetIndex === sinceResetIndex)
-          } else if (event.type === 'resource-snapshot' || event.type === 'context-options' || foundMarker) {
+              foundMarker = true;
-            // We keep all resources for snapshots, context options and all events after the marker.
+          } 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) {
+  private _startScreencastInPage(page: Page) {
-    if (screenshots)
+    page.setScreencastOptions(kScreencastOptions);
-      page.setScreencastOptions({ width: 800, height: 600, quality: 90 });
+    this._screencastListeners.push(
-
-    this._eventListeners.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);
+      if (!this._recording)
-      await fs.promises.appendFile(this._traceFile!, JSON.stringify(event) + '\n');
+        return;
+      visitSha1s(event, this._recording.sha1s);
+      await fs.promises.appendFile(this._recording.traceFile, JSON.stringify(event) + '\n');
     });
   }
 

tests/snapshotter.spec.ts

@@ -44,6 +44,17 @@ it.describe('snapshots', () => {
     expect(distillSnapshot(snapshot)).toBe('<BUTTON>Hello</BUTTON>');
   });
 
+  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(`<BASE href="${server.EMPTY_PAGE}"`);
+    await snapshotter.reset();
+    const snapshot2 = await snapshotter.captureSnapshot(toImpl(page), 'snapshot2');
+    const html2 = snapshot2.render().html;
+    expect(html2.replace(`"snapshot2"`, `"snapshot1"`)).toEqual(html1);
+  });
+
   it('should capture resources', async ({ page, toImpl, server, snapshotter }) => {
     await page.goto(server.EMPTY_PAGE);
     await page.route('**/style.css', route => {

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.start({ screenshots: false, snapshots: false });
-  await context.tracing._reset();
   await page.setContent('<button>Click</button>');
   await page.click('"Click"');
-  // @ts-expect-error
+  await context.tracing.stop({ path: testInfo.outputPath('trace.zip') });
-  await context.tracing._export({ path: testInfo.outputPath('trace.zip') });
-  await context.tracing.stop();
 
   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 === 'frame-snapshot')).toBeFalsy();
-  expect(events.some(e => e.type === 'resource-snapshot' && e.snapshot.url.endsWith('style.css'))).toBeTruthy();
+  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('<button>Click</button>');
+  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<string, Buffer> }> {
   const entries = await new Promise<any[]>(f => {