chore: move recorder trace to action collector (#32597)

packages/playwright-core/src/server/recorder/contextRecorder.ts

@@ -75,7 +75,7 @@ export class ContextRecorder extends EventEmitter {
       saveStorage: params.saveStorage,
     };
 
-    const collection = new RecorderCollection(params.mode === 'recording');
+    const collection = new RecorderCollection(this._pageAliases, params.mode === 'recording');
     collection.on('change', () => {
       this._recorderSources = [];
       for (const languageGenerator of this._orderedLanguages) {
@@ -163,7 +163,7 @@ export class ContextRecorder extends EventEmitter {
     // First page is called page, others are called popup1, popup2, etc.
     const frame = page.mainFrame();
     page.on('close', () => {
-      this._collection.addAction({
+      this._collection.addRecordedAction({
         frame: this._describeMainFrame(page),
         committed: true,
         action: {
@@ -185,7 +185,7 @@ export class ContextRecorder extends EventEmitter {
     if (page.opener()) {
       this._onPopup(page.opener()!, page);
     } else {
-      this._collection.addAction({
+      this._collection.addRecordedAction({
         frame: this._describeMainFrame(page),
         committed: true,
         action: {
@@ -236,14 +236,15 @@ export class ContextRecorder extends EventEmitter {
 
     await this._delegate.rewriteActionInContext?.(this._pageAliases, actionInContext);
 
-    this._collection.willPerformAction(actionInContext);
+    const callMetadata = await this._collection.willPerformAction(actionInContext);
-    const success = await performAction(this._pageAliases, actionInContext);
+    if (!callMetadata)
-    if (success) {
+      return;
-      this._collection.didPerformAction(actionInContext);
+    const error = await performAction(callMetadata, this._pageAliases, actionInContext).then(() => undefined).catch((e: Error) => e);
+    await this._collection.didPerformAction(callMetadata, actionInContext, error);
+    if (error)
+      actionInContext.committed = true;
+    else
       this._setCommittedAfterTimeout(actionInContext);
-    } else {
-      this._collection.performedActionFailed(actionInContext);
-    }
   }
 
   private async _recordAction(frame: Frame, action: actions.Action) {
@@ -260,7 +261,7 @@ export class ContextRecorder extends EventEmitter {
     await this._delegate.rewriteActionInContext?.(this._pageAliases, actionInContext);
 
     this._setCommittedAfterTimeout(actionInContext);
-    this._collection.addAction(actionInContext);
+    this._collection.addRecordedAction(actionInContext);
   }
 
   private _setCommittedAfterTimeout(actionInContext: ActionInContext) {

packages/playwright-core/src/server/recorder/recorderCollection.ts

@@ -16,18 +16,25 @@
 
 import { EventEmitter } from 'events';
 import type { Frame } from '../frames';
+import type { Page } from '../page';
 import type { Signal } from './recorderActions';
 import type { ActionInContext } from '../codegen/types';
+import type { CallMetadata } from '@protocol/callMetadata';
+import { createGuid } from '../../utils/crypto';
+import { monotonicTime } from '../../utils/time';
+import { mainFrameForAction, traceParamsForAction } from './recorderUtils';
 
 export class RecorderCollection extends EventEmitter {
   private _currentAction: ActionInContext | null = null;
   private _lastAction: ActionInContext | null = null;
   private _actions: ActionInContext[] = [];
   private _enabled: boolean;
+  private _pageAliases: Map<Page, string>;
 
-  constructor(enabled: boolean) {
+  constructor(pageAliases: Map<Page, string>, enabled: boolean) {
     super();
     this._enabled = enabled;
+    this._pageAliases = pageAliases;
     this.restart();
   }
 
@@ -46,29 +53,55 @@ export class RecorderCollection extends EventEmitter {
     this._enabled = enabled;
   }
 
-  addAction(action: ActionInContext) {
+  async willPerformAction(actionInContext: ActionInContext): Promise<CallMetadata | null> {
     if (!this._enabled)
-      return;
+      return null;
-    this.willPerformAction(action);
+    const mainFrame = mainFrameForAction(this._pageAliases, actionInContext);
-    this.didPerformAction(action);
+
+    const { action } = actionInContext;
+    const callMetadata: CallMetadata = {
+      id: `call@${createGuid()}`,
+      apiName: 'frame.' + action.name,
+      objectId: mainFrame.guid,
+      pageId: mainFrame._page.guid,
+      frameId: mainFrame.guid,
+      startTime: monotonicTime(),
+      endTime: 0,
+      type: 'Frame',
+      method: action.name,
+      params: traceParamsForAction(actionInContext),
+      log: [],
+    };
+    await mainFrame.instrumentation.onBeforeCall(mainFrame, callMetadata);
+    this._currentAction = actionInContext;
+    return callMetadata;
   }
 
-  willPerformAction(action: ActionInContext) {
+  async didPerformAction(callMetadata: CallMetadata, actionInContext: ActionInContext, error?: Error) {
     if (!this._enabled)
       return;
-    this._currentAction = action;
-  }
 
-  performedActionFailed(action: ActionInContext) {
+    if (error) {
-    if (!this._enabled)
+      // Do not clear current action on delayed error.
-      return;
+      if (this._currentAction === actionInContext)
-    if (this._currentAction === action)
+        this._currentAction = null;
+    } else {
       this._currentAction = null;
+      this._actions.push(actionInContext);
+    }
+
+    this._lastAction = actionInContext;
+    const mainFrame = mainFrameForAction(this._pageAliases, actionInContext);
+    callMetadata.endTime = monotonicTime();
+    await mainFrame.instrumentation.onAfterCall(mainFrame, callMetadata);
+
+    this.emit('change');
   }
 
-  didPerformAction(actionInContext: ActionInContext) {
+  addRecordedAction(actionInContext: ActionInContext) {
     if (!this._enabled)
       return;
+    this._currentAction = null;
     const action = actionInContext.action;
     let eraseLastAction = false;
     if (this._lastAction && this._lastAction.frame.pageAlias === actionInContext.frame.pageAlias) {
@@ -81,14 +114,12 @@ export class RecorderCollection extends EventEmitter {
       if (lastAction && action.name === 'navigate' && lastAction.name === 'navigate') {
         if (action.url === lastAction.url) {
           // Already at a target URL.
-          this._currentAction = null;
           return;
         }
       }
     }
 
     this._lastAction = actionInContext;
-    this._currentAction = null;
     if (eraseLastAction)
       this._actions.pop();
     this._actions.push(actionInContext);
@@ -125,7 +156,7 @@ export class RecorderCollection extends EventEmitter {
     }
 
     if (signal.name === 'navigation' && frame._page.mainFrame() === frame) {
-      this.addAction({
+      this.addRecordedAction({
         frame: {
           pageAlias,
           framePath: [],

packages/playwright-core/src/server/recorder/recorderRunner.ts

@@ -14,118 +14,117 @@
  * limitations under the License.
  */
 
-import { createGuid, monotonicTime, serializeExpectedTextValues } from '../../utils';
+import { serializeExpectedTextValues } from '../../utils';
 import { toKeyboardModifiers } from '../codegen/language';
 import type { ActionInContext } from '../codegen/types';
-import type { Frame } from '../frames';
 import type { CallMetadata } from '../instrumentation';
 import type { Page } from '../page';
 import type * as actions from './recorderActions';
 import type * as types from '../types';
-import { buildFullSelector } from './recorderUtils';
+import { buildFullSelector, mainFrameForAction } from './recorderUtils';
 
-async function innerPerformAction(mainFrame: Frame, action: string, params: any, cb: (callMetadata: CallMetadata) => Promise<any>): Promise<boolean> {
+export async function performAction(callMetadata: CallMetadata, pageAliases: Map<Page, string>, actionInContext: ActionInContext) {
-  const callMetadata: CallMetadata = {
+  const mainFrame = mainFrameForAction(pageAliases, actionInContext);
-    id: `call@${createGuid()}`,
-    apiName: 'frame.' + action,
-    objectId: mainFrame.guid,
-    pageId: mainFrame._page.guid,
-    frameId: mainFrame.guid,
-    startTime: monotonicTime(),
-    endTime: 0,
-    type: 'Frame',
-    method: action,
-    params,
-    log: [],
-  };
-
-  try {
-    await mainFrame.instrumentation.onBeforeCall(mainFrame, callMetadata);
-    await cb(callMetadata);
-  } catch (e) {
-    callMetadata.endTime = monotonicTime();
-    await mainFrame.instrumentation.onAfterCall(mainFrame, callMetadata);
-    return false;
-  }
-
-  callMetadata.endTime = monotonicTime();
-  await mainFrame.instrumentation.onAfterCall(mainFrame, callMetadata);
-  return true;
-}
-
-export async function performAction(pageAliases: Map<Page, string>, actionInContext: ActionInContext): Promise<boolean> {
-  const pageAlias = actionInContext.frame.pageAlias;
-  const page = [...pageAliases.entries()].find(([, alias]) => pageAlias === alias)?.[0];
-  if (!page)
-    throw new Error('Internal error: page not found');
-  const mainFrame = page.mainFrame();
   const { action } = actionInContext;
+
   const kActionTimeout = 5000;
 
-  if (action.name === 'navigate')
+  if (action.name === 'navigate') {
-    return await innerPerformAction(mainFrame, 'goto', { url: action.url }, callMetadata => mainFrame.goto(callMetadata, action.url, { timeout: kActionTimeout }));
+    await mainFrame.goto(callMetadata, action.url, { timeout: kActionTimeout });
+    return;
+  }
+
   if (action.name === 'openPage')
     throw Error('Not reached');
-  if (action.name === 'closePage')
+
-    return await innerPerformAction(mainFrame, 'close', {}, callMetadata => mainFrame._page.close(callMetadata));
+  if (action.name === 'closePage') {
+    await mainFrame._page.close(callMetadata);
+    return;
+  }
 
   const selector = buildFullSelector(actionInContext.frame.framePath, action.selector);
 
   if (action.name === 'click') {
     const options = toClickOptions(action);
-    return await innerPerformAction(mainFrame, 'click', { selector }, callMetadata => mainFrame.click(callMetadata, selector, { ...options, timeout: kActionTimeout, strict: true }));
+    await mainFrame.click(callMetadata, selector, { ...options, timeout: kActionTimeout, strict: true });
+    return;
   }
+
   if (action.name === 'press') {
     const modifiers = toKeyboardModifiers(action.modifiers);
     const shortcut = [...modifiers, action.key].join('+');
-    return await innerPerformAction(mainFrame, 'press', { selector, key: shortcut }, callMetadata => mainFrame.press(callMetadata, selector, shortcut, { timeout: kActionTimeout, strict: true }));
+    await mainFrame.press(callMetadata, selector, shortcut, { timeout: kActionTimeout, strict: true });
+    return;
   }
-  if (action.name === 'fill')
+
-    return await innerPerformAction(mainFrame, 'fill', { selector, text: action.text }, callMetadata => mainFrame.fill(callMetadata, selector, action.text, { timeout: kActionTimeout, strict: true }));
+  if (action.name === 'fill') {
-  if (action.name === 'setInputFiles')
+    await mainFrame.fill(callMetadata, selector, action.text, { timeout: kActionTimeout, strict: true });
-    return await innerPerformAction(mainFrame, 'setInputFiles', { selector, files: action.files }, callMetadata => mainFrame.setInputFiles(callMetadata, selector, { selector, payloads: [], timeout: kActionTimeout, strict: true }));
+    return;
-  if (action.name === 'check')
+  }
-    return await innerPerformAction(mainFrame, 'check', { selector }, callMetadata => mainFrame.check(callMetadata, selector, { timeout: kActionTimeout, strict: true }));
+
-  if (action.name === 'uncheck')
+  if (action.name === 'setInputFiles') {
-    return await innerPerformAction(mainFrame, 'uncheck', { selector }, callMetadata => mainFrame.uncheck(callMetadata, selector, { timeout: kActionTimeout, strict: true }));
+    await mainFrame.setInputFiles(callMetadata, selector, { selector, payloads: [], timeout: kActionTimeout, strict: true });
+    return;
+  }
+
+  if (action.name === 'check') {
+    await mainFrame.check(callMetadata, selector, { timeout: kActionTimeout, strict: true });
+    return;
+  }
+
+  if (action.name === 'uncheck') {
+    await mainFrame.uncheck(callMetadata, selector, { timeout: kActionTimeout, strict: true });
+    return;
+  }
+
   if (action.name === 'select') {
     const values = action.options.map(value => ({ value }));
-    return await innerPerformAction(mainFrame, 'selectOption', { selector, values }, callMetadata => mainFrame.selectOption(callMetadata, selector, [], values, { timeout: kActionTimeout, strict: true }));
+    await mainFrame.selectOption(callMetadata, selector, [], values, { timeout: kActionTimeout, strict: true });
+    return;
   }
+
   if (action.name === 'assertChecked') {
-    return await innerPerformAction(mainFrame, 'expect', { selector }, callMetadata => mainFrame.expect(callMetadata, selector, {
+    await mainFrame.expect(callMetadata, selector, {
       selector,
       expression: 'to.be.checked',
       isNot: !action.checked,
       timeout: kActionTimeout,
-    }));
+    });
+    return;
   }
+
   if (action.name === 'assertText') {
-    return await innerPerformAction(mainFrame, 'expect', { selector }, callMetadata => mainFrame.expect(callMetadata, selector, {
+    await mainFrame.expect(callMetadata, selector, {
       selector,
       expression: 'to.have.text',
       expectedText: serializeExpectedTextValues([action.text], { matchSubstring: true, normalizeWhiteSpace: true }),
       isNot: false,
       timeout: kActionTimeout,
-    }));
+    });
+    return;
   }
+
   if (action.name === 'assertValue') {
-    return await innerPerformAction(mainFrame, 'expect', { selector }, callMetadata => mainFrame.expect(callMetadata, selector, {
+    await mainFrame.expect(callMetadata, selector, {
       selector,
       expression: 'to.have.value',
       expectedValue: action.value,
       isNot: false,
       timeout: kActionTimeout,
-    }));
+    });
+    return;
   }
+
   if (action.name === 'assertVisible') {
-    return await innerPerformAction(mainFrame, 'expect', { selector }, callMetadata => mainFrame.expect(callMetadata, selector, {
+    await mainFrame.expect(callMetadata, selector, {
       selector,
       expression: 'to.be.visible',
       isNot: false,
       timeout: kActionTimeout,
-    }));
+    });
+    return;
   }
+
   throw new Error('Internal error: unexpected action ' + (action as any).name);
 }
 

packages/playwright-core/src/server/recorder/recorderUtils.ts

@@ -20,6 +20,8 @@ import type { Page } from '../page';
 import type { ActionInContext } from '../codegen/types';
 import type { Frame } from '../frames';
 import type * as actions from './recorderActions';
+import { toKeyboardModifiers } from '../codegen/language';
+import { serializeExpectedTextValues } from '../../utils/expectUtils';
 
 export function metadataToCallLog(metadata: CallMetadata, status: CallLogStatus): CallLog {
   let title = metadata.apiName || metadata.method;
@@ -72,3 +74,58 @@ export async function frameForAction(pageAliases: Map<Page, string>, actionInCon
     throw new Error('Internal error: frame not found');
   return result.frame;
 }
+
+export function traceParamsForAction(actionInContext: ActionInContext) {
+  const { action } = actionInContext;
+
+  switch (action.name) {
+    case 'navigate': return { url: action.url };
+    case 'openPage': return {};
+    case 'closePage': return {};
+  }
+
+  const selector = buildFullSelector(actionInContext.frame.framePath, action.selector);
+  switch (action.name) {
+    case 'click': return { selector, clickCount: action.clickCount };
+    case 'press': {
+      const modifiers = toKeyboardModifiers(action.modifiers);
+      const shortcut = [...modifiers, action.key].join('+');
+      return { selector, key: shortcut };
+    }
+    case 'fill': return { selector, text: action.text };
+    case 'setInputFiles': return { selector, files: action.files };
+    case 'check': return { selector };
+    case 'uncheck': return { selector };
+    case 'select': return { selector, values: action.options.map(value => ({ value })) };
+    case 'assertChecked': {
+      return {
+        selector,
+        expression: 'to.be.checked',
+        isNot: !action.checked,
+      };
+    }
+    case 'assertText': {
+      return {
+        selector,
+        expression: 'to.have.text',
+        expectedText: serializeExpectedTextValues([action.text], { matchSubstring: true, normalizeWhiteSpace: true }),
+        isNot: false,
+      };
+    }
+    case 'assertValue': {
+      return {
+        selector,
+        expression: 'to.have.value',
+        expectedValue: action.value,
+        isNot: false,
+      };
+    }
+    case 'assertVisible': {
+      return {
+        selector,
+        expression: 'to.be.visible',
+        isNot: false,
+      };
+    }
+  }
+}