chore(trace): use a single id for calls and test runner steps

packages/isomorphic/trace/traceLoader.ts

@@ -14,8 +14,6 @@
  * limitations under the License.
  */
 
-import { parseClientSideCallMetadata } from './traceUtils';
-
 import { SnapshotStorage } from './snapshotStorage';
 import { TraceModernizer } from './traceModernizer';
 
@@ -72,6 +70,11 @@ export class TraceLoader {
       modernizer.appendTrace(network);
       unzipProgress?.(++done, total);
 
+      const stacks = await this._backend.readText(prefix + '.stacks');
+      if (stacks)
+        modernizer.appendStacks(stacks);
+      unzipProgress?.(++done, total);
+
       contextEntry.actions = modernizer.actions().sort((a1, a2) => a1.startTime - a2.startTime);
 
       if (!backend.isLive()) {
@@ -88,14 +91,6 @@ export class TraceLoader {
         }
       }
 
-      const stacks = await this._backend.readText(prefix + '.stacks');
-      if (stacks) {
-        const callMetadata = parseClientSideCallMetadata(JSON.parse(stacks));
-        for (const action of contextEntry.actions)
-          action.stack = action.stack || callMetadata.get(action.callId);
-      }
-      unzipProgress?.(++done, total);
-
       for (const resource of contextEntry.resources) {
         if (resource.request.postData?._file)
           this._resourceToContentType.set(resource.request.postData._file, stripEncodingFromContentType(resource.request.postData.mimeType));

packages/isomorphic/trace/traceModel.ts

@@ -288,8 +288,6 @@ function mergeActionsAndUpdateTiming(contexts: ContextEntry[]) {
   return result;
 }
 
-let lastTmpStepId = 0;
-
 function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]): ActionEntry[] {
   const map = new Map<string, ActionEntry>();
 
@@ -311,37 +309,31 @@ function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]): ActionE
   }
 
   for (const context of libraryContexts) {
-    for (const action of context.actions) {
-      // Never merge stepless events.
-      map.set(action.stepId || `tmp-step@${++lastTmpStepId}`, { ...action });
-    }
+    for (const action of context.actions)
+      map.set(action.callId, { ...action });
   }
 
-  const nonPrimaryIdToPrimaryId = new Map<string, string>();
   for (const context of testRunnerContexts) {
     for (const action of context.actions) {
-      const existing = action.stepId && map.get(action.stepId);
-      if (existing) {
-        nonPrimaryIdToPrimaryId.set(action.callId, existing.callId);
-        if (action.error)
-          existing.error = action.error;
-        if (action.attachments)
-          existing.attachments = action.attachments;
-        if (action.annotations)
-          existing.annotations = action.annotations;
-        if (action.parentId)
-          existing.parentId = nonPrimaryIdToPrimaryId.get(action.parentId) ?? action.parentId;
-        if (action.group)
-          existing.group = action.group;
-        // For the events that are present in the test runner context, always take
-        // their time from the test runner context to preserve client side order.
-        existing.startTime = action.startTime;
-        existing.endTime = action.endTime;
+      const existing = map.get(action.callId);
+      if (!existing) {
+        map.set(action.callId, { ...action });
         continue;
       }
+      if (action.error)
+        existing.error = action.error;
+      if (action.attachments)
+        existing.attachments = action.attachments;
+      if (action.annotations)
+        existing.annotations = action.annotations;
       if (action.parentId)
-        action.parentId = nonPrimaryIdToPrimaryId.get(action.parentId) ?? action.parentId;
-      map.set(action.stepId || `tmp-step@${++lastTmpStepId}`, { ...action });
+        existing.parentId = action.parentId;
+      if (action.group)
+        existing.group = action.group;
+      // For the events that are present in the test runner context, always take
+      // their time from the test runner context to preserve client side order.
+      existing.startTime = action.startTime;
+      existing.endTime = action.endTime;
     }
   }
   return [...map.values()];

packages/isomorphic/trace/traceModernizer.ts

@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+import { defaultCallId, parseClientSideCallMetadata } from './traceUtils';
+
+import type { SerializedStack } from './traceUtils';
 import type * as trace from './trace';
 import type * as traceV3 from './versions/traceV3';
 import type * as traceV4 from './versions/traceV4';
@@ -46,6 +49,7 @@ export class TraceModernizer {
   private _consoleObjects = new Map<string, { type: string, text: string, location: { url: string, lineNumber: number, columnNumber: number }, args?: { preview: string, value: string }[] }>();
   private _apiRequestRef: string | undefined;
   private _snapshotPhases = new Map<string, trace.ActionPhase>();
+  private _legacyCallIdToStepId = new Map<string, string>();
 
   constructor(contextEntry: ContextEntry, snapshotStorage: SnapshotStorage) {
     this._contextEntry = contextEntry;
@@ -57,6 +61,18 @@ export class TraceModernizer {
       this._appendEvent(line);
   }
 
+  appendStacks(stacks: string) {
+    const data = JSON.parse(stacks);
+    const normalized: SerializedStack[] = data.stacks.map(([id, ...rest]: any) => {
+      // Transform legacy numeric call ids into string ids.
+      const callId = typeof id === 'number' ? defaultCallId(id) : id;
+      return [this._legacyCallIdToStepId.get(callId) ?? callId, ...rest];
+    });
+    const callMetadata = parseClientSideCallMetadata({ files: data.files, stacks: normalized });
+    for (const action of this._actionMap.values())
+      action.stack = action.stack || callMetadata.get(action.callId);
+  }
+
   actions(): ActionEntry[] {
     return [...this._actionMap.values()];
   }
@@ -469,6 +485,21 @@ export class TraceModernizer {
 
   _modernize_8_to_9(events: traceV8.TraceEvent[]): trace.TraceEvent[] {
     for (const event of events) {
+      // The library and the test runner used to mint their own id for the same call and reconcile
+      // them through a `stepId` side-channel. Now they share a single id - adopt the step id as the
+      // call id, remembering the mapping for the ids that `appendStacks` will see.
+      if (event.type === 'before' || event.type === 'action') {
+        if (event.stepId && event.stepId !== event.callId)
+          this._legacyCallIdToStepId.set(event.callId, event.stepId);
+        delete event.stepId;
+        if (event.parentId)
+          event.parentId = this._legacyCallIdToStepId.get(event.parentId) ?? event.parentId;
+      }
+      if (event.type === 'before' || event.type === 'input' || event.type === 'after' || event.type === 'action' || event.type === 'log')
+        event.callId = this._legacyCallIdToStepId.get(event.callId) ?? event.callId;
+      if (event.type === 'frame-snapshot')
+        event.snapshot.callId = this._legacyCallIdToStepId.get(event.snapshot.callId) ?? event.snapshot.callId;
+
       // Actions used to point at their snapshots by name, now snapshots know their own phase.
       if (event.type === 'before' || event.type === 'input' || event.type === 'after' || event.type === 'action') {
         const action = event as traceV8.ActionTraceEvent;

packages/isomorphic/trace/traceUtils.ts

@@ -18,19 +18,23 @@ import type { StackFrame } from './trace';
 import type { ClientSideCallMetadata } from '@protocol/structs';
 
 export type SerializedStackFrame = [number, number, number, string];
-export type SerializedStack = [number, SerializedStackFrame[]];
+export type SerializedStack = [string, SerializedStackFrame[]];
 
 export type SerializedClientSideCallMetadata = {
   files: string[];
   stacks: SerializedStack[];
 };
 
+export function defaultCallId(ordinal: number): string {
+  return `call@${ordinal}`;
+}
+
 export function parseClientSideCallMetadata(data: SerializedClientSideCallMetadata): Map<string, StackFrame[]> {
   const result = new Map<string, StackFrame[]>();
   const { files, stacks } = data;
   for (const s of stacks) {
     const [id, ff] = s;
-    result.set(`call@${id}`, ff.map(f => ({ file: files[f[0]], line: f[1], column: f[2], function: f[3] })));
+    result.set(id, ff.map(f => ({ file: files[f[0]], line: f[1], column: f[2], function: f[3] })));
   }
   return result;
 }

packages/isomorphic/trace/versions/traceV9.ts

@@ -148,7 +148,6 @@ export type BeforeActionTraceEvent = {
   class: string;
   method: string;
   params: Record<string, any>;
-  stepId?: string;
   stack?: StackFrame[];
   parentId?: string;
   group?: string;

packages/playwright-core/src/client/channelOwner.ts

@@ -196,7 +196,7 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
     let apiName = stackTrace.apiName;
     if (apiName.startsWith('_') || apiName.includes('._'))
       apiName = options?.title ?? apiName;
-    const apiZone: ApiZone = { title: options?.title, apiName, frames: stackTrace.frames, internal: options?.internal ?? false, reported: false, userData: undefined, stepId: undefined };
+    const apiZone: ApiZone = { title: options?.title, apiName, frames: stackTrace.frames, internal: options?.internal ?? false, reported: false, userData: undefined, callId: undefined };
 
     try {
       const result = await currentZone().with('apiZone', apiZone).run(async () => await func(apiZone));
@@ -254,6 +254,6 @@ type ApiZone = {
   internal?: boolean;
   reported: boolean;
   userData: any;
-  stepId?: string;
+  callId?: string;
   error?: Error;
 };

packages/playwright-core/src/client/clientInstrumentation.ts

@@ -20,12 +20,12 @@ import type { StackFrame } from './channels';
 import type { Page } from './page';
 import type { BrowserContextOptions } from './types';
 
-// Instrumentation can mutate the data, for example change the stepId.
+// Instrumentation can mutate the data, for example assign the callId.
 export interface ApiCallData {
   title?: string;
   frames: StackFrame[];
   userData: any;
-  stepId?: string;
+  callId?: string;
   error?: Error;
 }
 

packages/playwright-core/src/client/connection.ts

@@ -20,6 +20,7 @@ import { isUnderTest } from '@utils/debug';
 import { debugLogger } from '@utils/debugLogger';
 import { emptyZone } from '@utils/zones';
 import { ValidationError, findValidator, maybeFindValidator } from '@protocol/validator';
+import { defaultCallId } from '@isomorphic/trace/traceUtils';
 import { EventEmitter } from './eventEmitter';
 import { Android, AndroidDevice, AndroidSocket } from './android';
 import { Artifact } from './artifact';
@@ -73,8 +74,8 @@ export type ChannelOwnerFactory = (parent: ChannelOwner, type: string, guid: str
 export class Connection extends EventEmitter {
   readonly _objects = new Map<string, ChannelOwner>();
   onmessage = (message: object): void => {};
-  private _lastId = 0;
-  private _callbacks = new Map<number, { resolve: (a: any) => void, reject: (a: Error) => void, signal: AbortSignal | undefined, title: string | undefined, type: string, method: string }>();
+  private _lastOrdinal = 0;
+  private _callbacks = new Map<string, { resolve: (a: any) => void, reject: (a: Error) => void, signal: AbortSignal | undefined, title: string | undefined, type: string, method: string }>();
   private _rootObject: Root;
   private _closedError: Error | undefined;
   private _isRemote = false;
@@ -175,7 +176,7 @@ export class Connection extends EventEmitter {
       this._tracingCount--;
   }
 
-  async sendMessageToServer(object: ChannelOwner, method: string, params: any, options: { apiName?: string, title?: string, internal?: boolean, frames?: channels.StackFrame[], stepId?: string, signal?: AbortSignal, timeout: number }): Promise<any> {
+  async sendMessageToServer(object: ChannelOwner, method: string, params: any, options: { apiName?: string, title?: string, internal?: boolean, frames?: channels.StackFrame[], callId?: string, signal?: AbortSignal, timeout: number }): Promise<any> {
     // Fire-and-forget: server intentionally never replies to __waitInfo__,
     // so silently drop it after the connection is closed or the object was collected.
     if (method === '__waitInfo__' && (this._closedError || object._wasCollected))
@@ -191,14 +192,14 @@ export class Connection extends EventEmitter {
 
     const guid = object._guid;
     const type = object._type;
-    const id = ++this._lastId;
+    const id = options.callId ?? defaultCallId(++this._lastOrdinal);
     const message = { id, guid, method, params };
     if (debugLogger.isEnabled('channel')) {
       // Do not include metadata in debug logs to avoid noise.
       debugLogger.log('channel', 'SEND> ' + JSON.stringify(message));
     }
     const location = options.frames?.[0] ? { file: options.frames[0].file, line: options.frames[0].line, column: options.frames[0].column } : undefined;
-    const metadata: channels.Metadata = { title: options.title, location, internal: options.internal, stepId: options.stepId, timeout: options.timeout };
+    const metadata: channels.Metadata = { title: options.title, location, internal: options.internal, timeout: options.timeout };
     if (this._tracingCount && options.frames && type !== 'LocalUtils')
       this._localUtils?.addStackToTracingNoReply({ callData: { stack: options.frames ?? [], id } }).catch(() => {});
     // We need to exit zones before calling into the server, otherwise

packages/playwright-core/src/server/dispatchers/dispatcher.ts

@@ -297,7 +297,7 @@ export class DispatcherConnection {
   }
 
   async dispatch(message: object) {
-    const { id, guid, method, params, metadata } = message as any;
+    const { id, guid, method, params, metadata } = message as { id: string, guid: string, method: string, params: any, metadata: any };
     const dispatcher = this._dispatcherByGuid.get(guid);
     if (method === '__waitInfo__') {
       // Fire-and-forget: silently drop if the target is gone.
@@ -306,7 +306,7 @@ export class DispatcherConnection {
       return;
     }
     if (method === '__abort__') {
-      const entry = this._activeProgressControllers.get(`call@${params.id}`);
+      const entry = this._activeProgressControllers.get(params.id);
       if (!entry)
         return;
       entry.abortError = new AbortError(params.reason);
@@ -342,11 +342,10 @@ export class DispatcherConnection {
 
     const sdkObject = dispatcher._object;
     const callMetadata: CallMetadata = {
-      id: `call@${id}`,
+      id,
       location: validMetadata.location,
       title: validMetadata.title,
       internal: validMetadata.internal,
-      stepId: validMetadata.stepId,
       objectId: sdkObject.guid,
       startTime: monotonicTime(),
       endTime: 0,
@@ -358,7 +357,7 @@ export class DispatcherConnection {
     };
 
     const abortControllerEntry: { controller?: ProgressController, abortError?: Error } = {};
-    this._activeProgressControllers.set(callMetadata.id, abortControllerEntry);
+    this._activeProgressControllers.set(id, abortControllerEntry);
     const swapProgressController = () => {
       const controller = dispatcher.createProgressController(callMetadata, abortControllerEntry.abortError);
       abortControllerEntry.controller = controller;
@@ -403,7 +402,7 @@ export class DispatcherConnection {
       await afterController.run(progress => sdkObject.instrumentation.onAfterCall(progress, sdkObject), 3000).catch(() => {});
       if (metainfo?.slowMo)
         await this._doSlowMo(sdkObject);
-      this._activeProgressControllers.delete(callMetadata.id);
+      this._activeProgressControllers.delete(id);
     }
 
     if (response.error)
@@ -417,7 +416,7 @@ export class DispatcherConnection {
       await new Promise(f => setTimeout(f, slowMo));
   }
 
-  private async _dispatchWaitInfo(id: number, dispatcher: DispatcherScope, params: any, metadata: any) {
+  private async _dispatchWaitInfo(id: string, dispatcher: DispatcherScope, params: any, metadata: any) {
     // Fire-and-forget notification: never reply, never throw to the caller.
     let info: channels.WaitInfo;
     let validMetadata: channels.Metadata;
@@ -432,11 +431,10 @@ export class DispatcherConnection {
     const sdkObject = dispatcher._object;
     if (info.phase === 'before') {
       const callMetadata: CallMetadata = {
-        id: `call@${id}`,
+        id,
         location: validMetadata.location,
         title: validMetadata.title,
         internal: validMetadata.internal,
-        stepId: validMetadata.stepId,
         objectId: sdkObject.guid,
         startTime: monotonicTime(),
         endTime: 0,

packages/playwright-core/src/server/instrumentation.ts

@@ -96,8 +96,6 @@ export type CallMetadata = {
   // Client is making an internal call that should not show up in
   // the inspector or trace.
   internal?: boolean;
-  // Test runner step id.
-  stepId?: string;
   location?: { file: string, line?: number, column?: number };
   log: string[];
   error?: SerializedError;

packages/playwright-core/src/server/trace/recorder/tracing.ts

@@ -263,7 +263,6 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
       class: 'Tracing',
       method: 'tracingGroup',
       params: { },
-      stepId: metadata.stepId,
       stack: stackFrames,
     };
     if (this._currentGroupId())
@@ -782,7 +781,6 @@ function createBeforeActionTraceEvent(metadata: CallMetadata, parentId?: string)
     class: metadata.type,
     method: metadata.method,
     params: metadata.timeout ? { ...metadata.params, timeout: metadata.timeout } : metadata.params,
-    stepId: metadata.stepId,
   };
   if (parentId)
     event.parentId = parentId;

packages/playwright/src/index.ts

@@ -106,11 +106,11 @@ const utilityFixtures: Fixtures<UtilityTestFixtures, UtilityWorkerFixtures> = {
         const zone = currentZone().data<TestStepInternal>('stepZone');
         const isExpectCall = (channel.type === 'Frame' && channel.method === 'expect') || (channel.type === 'Page' && channel.method === 'expectScreenshot');
         if (zone && zone.category === 'expect' && isExpectCall) {
-          data.stepId = zone.stepId;
+          data.callId = zone.stepId;
           return;
         }
 
-        // In the general case, create a step for each api call and connect them through the stepId.
+        // In the general case, create a step for each api call, use it as a callId.
         const params = renderParamsForCall({ type: channel.type, method: channel.method, params: channel.params });
         const step = testInfo._addStep({
           location: data.frames[0],
@@ -120,7 +120,7 @@ const utilityFixtures: Fixtures<UtilityTestFixtures, UtilityWorkerFixtures> = {
           params,
           group: getActionGroup({ type: channel.type, method: channel.method }),
         }, tracingGroupSteps[tracingGroupSteps.length - 1]);
-        data.stepId = step.stepId;
+        data.callId = step.stepId;
         if (channel.type === 'Tracing' && channel.method === 'tracingGroup') {
           // The step will end later, when the corresponding "tracing.groupEnd" call finishes.
           tracingGroupSteps.push(step);