chore(server): drop fixupMetadataError

packages/isomorphic/trace/traceModel.ts

@@ -167,12 +167,13 @@ export class TraceModel {
   private _errorDescriptorsFromActions(): ErrorDescription[] {
     const errors: ErrorDescription[] = [];
     for (const action of this.actions || []) {
-      if (!action.error?.message)
+      const message = actionErrorMessage(action);
+      if (!message)
         continue;
       errors.push({
         action,
         stack: action.stack,
-        message: action.error.message,
+        message,
       });
     }
     return errors;
@@ -387,6 +388,17 @@ export function nextActionByStartTime(action: ActionTraceEvent): ActionTraceEven
   return (action as any)[nextByStartTimeSymbol];
 }
 
+// In library mode, `expect` returns a result instead of throwing, so its
+// "failed" outcome never lands in `action.error`. Recognize that case here so
+// the action shows up as an error in the trace viewer.
+export function actionErrorMessage(action: ActionTraceEvent): string | undefined {
+  if (action.error?.message)
+    return action.error.message;
+  if (action.method === 'expect' && action.result && action.params && action.result.matches === action.params.isNot)
+    return 'Expect failed';
+  return undefined;
+}
+
 export function stats(action: ActionTraceEvent): { errors: number, warnings: number } {
   let errors = 0;
   let warnings = 0;

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

@@ -1455,11 +1455,6 @@ export class Frame extends SdkObject<FrameEventMap> {
   async expect(progress: Progress, selector: string | undefined, options: FrameExpectParams): Promise<ExpectResult> {
     progress.log(`${renderTitleForCall(progress.metadata)}${options.timeoutForLogs ? ` with timeout ${options.timeoutForLogs}ms` : ''}`);
     const lastIntermediateResult: { received?: ExpectReceived, isSet: boolean, errorMessage?: string } = { isSet: false };
-    const fixupMetadataError = (result: ExpectResult) => {
-      // Library mode special case for the expect errors which are return values, not exceptions.
-      if (result.matches === options.isNot)
-        progress.metadata.error = { error: { name: 'Expect', message: 'Expect failed' } };
-    };
     try {
       // Step 1: perform locator handlers checkpoint with a specified timeout.
       if (selector)
@@ -1493,7 +1488,6 @@ export class Frame extends SdkObject<FrameEventMap> {
         }
         return { matches, received };
       });
-      fixupMetadataError(result);
       return result;
     } catch (e) {
       // Q: Why not throw upon isNonRetriableError(e) as in other places?
@@ -1509,7 +1503,6 @@ export class Frame extends SdkObject<FrameEventMap> {
       }
       if (e instanceof TimeoutError)
         result.timedOut = true;
-      fixupMetadataError(result);
       return result;
     }
   }

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

@@ -25,7 +25,7 @@ import { eventsHelper } from '@utils/eventsHelper';
 import { monotonicTime } from '@isomorphic/time';
 import { BrowserContext } from './browserContext';
 import { Debugger } from './debugger';
-import { buildFullSelector, generateFrameSelector, metadataToCallLog } from './recorder/recorderUtils';
+import { buildFullSelector, generateFrameSelector, metadataError, metadataToCallLog } from './recorder/recorderUtils';
 import { nullProgress, ProgressController } from './progress';
 
 import { RecorderSignalProcessor } from './recorder/recorderSignalProcessor';
@@ -417,7 +417,7 @@ export class Recorder extends EventEmitter<RecorderEventMap> implements Instrume
   async onAfterCall(sdkObject: SdkObject, metadata: CallMetadata) {
     if (this._omitCallTracking || this._isRecording())
       return;
-    if (!metadata.error)
+    if (!metadataError(metadata))
       this._currentCallsMetadata.delete(metadata);
     this._updateUserSources();
     this._updateCallLog([metadata]);
@@ -442,7 +442,7 @@ export class Recorder extends EventEmitter<RecorderEventMap> implements Instrume
       }
       if (line) {
         const paused = this._debugger.isPaused(metadata);
-        source.highlight.push({ line, type: metadata.error ? 'error' : (paused ? 'paused' : 'running') });
+        source.highlight.push({ line, type: metadataError(metadata) ? 'error' : (paused ? 'paused' : 'running') });
         source.revealLine = line;
       }
     }

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

@@ -30,9 +30,21 @@ export function buildFullSelector(framePath: string[], selector: string) {
   return [...framePath, selector].join(' >> internal:control=enter-frame >> ');
 }
 
+// In library mode, `expect` returns a result instead of throwing, so its
+// "failed" outcome never lands in `metadata.error`. Recognize that case here so
+// the action shows up as an error in the recorder UI.
+export function metadataError(metadata: CallMetadata): string | undefined {
+  if (metadata.error?.error?.message)
+    return metadata.error.error.message;
+  if (metadata.method === 'expect' && metadata.result && metadata.params && metadata.result.matches === metadata.params.isNot)
+    return 'Expect failed';
+  return undefined;
+}
+
 export function metadataToCallLog(metadata: CallMetadata, status: CallLogStatus): CallLog {
   const title = renderTitleForCall(metadata);
-  if (metadata.error)
+  const error = metadataError(metadata);
+  if (error)
     status = 'error';
   const params = {
     url: metadata.params?.url,
@@ -48,7 +60,7 @@ export function metadataToCallLog(metadata: CallMetadata, status: CallLogStatus)
     messages: metadata.log,
     title: title ?? '',
     status,
-    error: metadata.error?.error?.message,
+    error,
     params,
     duration,
   };

packages/trace-viewer/src/ui/actionList.tsx

@@ -19,7 +19,7 @@ import { clsx } from '@web/uiUtils';
 import { msToString } from '@isomorphic/formatUtils';
 import * as React from 'react';
 import './actionList.css';
-import { stats, buildActionTree } from '@isomorphic/trace/traceModel';
+import { actionErrorMessage, stats, buildActionTree } from '@isomorphic/trace/traceModel';
 import { asLocatorDescription, type Language } from '@isomorphic/locatorGenerators';
 import type { TreeState } from '@web/components/treeView';
 import { TreeView } from '@web/components/treeView';
@@ -71,7 +71,7 @@ export const ActionList: React.FC<ActionListProps> = ({
   }, [itemMap, selectedAction]);
 
   const isError = React.useCallback((item: ActionTreeItem) => {
-    return !!item.action.error?.message;
+    return !!actionErrorMessage(item.action);
   }, []);
 
   const onAccepted = React.useCallback((item: ActionTreeItem) => {

tests/library/inspector/pause.spec.ts

@@ -395,6 +395,27 @@ it.describe('pause', () => {
     expect(error.message).toContain('Not a checkbox or radio button');
   });
 
+  it('should populate log with expect failure', async ({ page, recorderPageGetter }) => {
+    await page.setContent('<button>Submit</button>');
+    const scriptPromise = (async () => {
+      // @ts-ignore
+      await page.pause({ __testHookKeepTestTimeout: true });
+      await expect(page.getByRole('button')).toHaveText('Other', { timeout: 1 });
+    })().catch(e => e);
+    const recorderPage = await recorderPageGetter();
+    await recorderPage.click('[title="Resume (F8)"]');
+    await recorderPage.waitForSelector('.source-line-error-underline');
+    expect(await sanitizeLog(recorderPage)).toEqual([
+      'Pause- XXms',
+      'Expect "toHaveText"(page.getByRole(\'button\'))- XXms',
+      'Expect "toHaveText" with timeout 1ms',
+      'waiting for getByRole(\'button\')',
+      'error: Expect failed',
+    ]);
+    const error = await scriptPromise;
+    expect(error.message).toContain('toHaveText');
+  });
+
   it('should populate log with error in waitForEvent', async ({ page, recorderPageGetter }) => {
     await page.setContent('<button>Submit</button>');
     const scriptPromise = (async () => {