chore: render testInfo errors in the Errors tab (#28179)

Fixes https://github.com/microsoft/playwright/issues/28056

packages/playwright/src/worker/testTracing.ts

@@ -21,7 +21,8 @@ import fs from 'fs';
 import path from 'path';
 import { ManualPromise, calculateSha1, monotonicTime } from 'playwright-core/lib/utils';
 import { yauzl, yazl } from 'playwright-core/lib/zipBundle';
-import type { TestInfo } from '../../types/test';
+import type { TestInfo, TestInfoError } from '../../types/test';
+import { filteredStackTrace } from '../util';
 
 
 export type Attachment = TestInfo['attachments'][0];
@@ -101,6 +102,16 @@ export class TestTracing {
     });
   }
 
+  appendForError(error: TestInfoError) {
+    const rawStack = error.stack?.split('\n') || [];
+    const stack = rawStack ? filteredStackTrace(rawStack) : [];
+    this._appendTraceEvent({
+      type: 'error',
+      message: error.message || String(error.value),
+      stack,
+    });
+  }
+
   appendStdioToTrace(type: 'stdout' | 'stderr', chunk: string | Buffer) {
     this._appendTraceEvent({
       type,

packages/playwright/src/worker/workerMain.ts

@@ -22,7 +22,7 @@ import { ConfigLoader } from '../common/configLoader';
 import type { Suite, TestCase } from '../common/test';
 import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config';
 import { FixtureRunner } from './fixtureRunner';
-import { ManualPromise, captureLibraryStackTrace, gracefullyCloseAll, removeFolders } from 'playwright-core/lib/utils';
+import { ManualPromise, gracefullyCloseAll, removeFolders } from 'playwright-core/lib/utils';
 import { TestInfoImpl } from './testInfo';
 import { TimeoutManager, type TimeSlot } from './timeoutManager';
 import { ProcessRunner } from '../common/process';
@@ -372,7 +372,7 @@ export class WorkerMain extends ProcessRunner {
         return;
       }
 
-      const error = await testInfo._runAndFailOnError(async () => {
+      await testInfo._runAndFailOnError(async () => {
         // Now run the test itself.
         debugTest(`test function started`);
         const fn = test.fn; // Extract a variable to get a better stack trace ("myTest" vs "TestCase.myTest [as fn]").
@@ -380,17 +380,8 @@ export class WorkerMain extends ProcessRunner {
         debugTest(`test function finished`);
       }, 'allowSkips');
 
-      // If there are no steps with errors in the test, but the test has an error - append artificial trace entry.
-      if (error && !testInfo._steps.some(s => !!s.error)) {
-        const frames = error.stack ? captureLibraryStackTrace(error.stack.split('\n')).frames : [];
-        const step = testInfo._addStep({
-          wallTime: Date.now(),
-          title: error.message || 'error',
-          category: 'hook',
-          location: frames[0],
-        });
-        step.complete({ error });
-      }
+      for (const error of testInfo.errors)
+        testInfo._tracing.appendForError(error);
     });
 
     if (didFailBeforeAllForSuite) {

packages/trace-viewer/src/entries.ts

@@ -36,6 +36,7 @@ export type ContextEntry = {
   actions: ActionEntry[];
   events: (trace.EventTraceEvent | trace.ConsoleMessageTraceEvent)[];
   stdio: trace.StdioTraceEvent[];
+  errors: trace.ErrorTraceEvent[];
   hasSource: boolean;
 };
 
@@ -68,6 +69,7 @@ export function createEmptyContext(): ContextEntry {
     resources: [],
     actions: [],
     events: [],
+    errors: [],
     stdio: [],
     hasSource: false
   };

packages/trace-viewer/src/traceModel.ts

@@ -223,6 +223,10 @@ export class TraceModel {
         contextEntry!.stdio.push(event);
         break;
       }
+      case 'error': {
+        contextEntry!.errors.push(event);
+        break;
+      }
       case 'console': {
         contextEntry!.events.push(event);
         break;

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

@@ -20,20 +20,50 @@ import type * as modelUtil from './modelUtil';
 import { PlaceholderPanel } from './placeholderPanel';
 import { renderAction } from './actionList';
 import type { Language } from '@isomorphic/locatorGenerators';
+import type { StackFrame } from '@protocol/channels';
+
+type ErrorDescription = {
+  action?: modelUtil.ActionTraceEventInContext;
+  stack?: StackFrame[];
+};
 
 type ErrorsTabModel = {
-  errors: Map<string, modelUtil.ActionTraceEventInContext>;
+  errors: Map<string, ErrorDescription>;
 };
 
+function errorsFromActions(model: modelUtil.MultiTraceModel): Map<string, ErrorDescription> {
+  const errors = new Map<string, ErrorDescription>();
+  for (const action of model.actions || []) {
+    // Overwrite errors with the last one.
+    if (!action.error?.message || errors.has(action.error.message))
+      continue;
+    errors.set(action.error.message, {
+      action,
+      stack: action.stack,
+    });
+  }
+  return errors;
+}
+
+function errorsFromTestRunner(model: modelUtil.MultiTraceModel): Map<string, ErrorDescription> {
+  const actionErrors = errorsFromActions(model);
+  const errors = new Map<string, ErrorDescription>();
+  for (const error of model.errors || []) {
+    if (!error.message || errors.has(error.message))
+      continue;
+    errors.set(error.message, actionErrors.get(error.message) || error);
+  }
+  return errors;
+}
+
 export function useErrorsTabModel(model: modelUtil.MultiTraceModel | undefined): ErrorsTabModel {
   return React.useMemo(() => {
-    const errors = new Map<string, modelUtil.ActionTraceEventInContext>();
-    for (const action of model?.actions || []) {
-      // Overwrite errors with the last one.
-      if (action.error?.message)
-        errors.set(action.error.message, action);
-    }
-    return { errors };
+    if (!model)
+      return { errors: new Map() };
+    // Feature detection: if there is test runner info, pick errors from the 'error' trace events.
+    // If there are no test errors, but there are action errors - render those instead.
+    const testHasErrors = !!model.errors.length;
+    return { errors: testHasErrors ? errorsFromTestRunner(model) : errorsFromActions(model) };
   }, [model]);
 }
 
@@ -46,13 +76,14 @@ export const ErrorsTab: React.FunctionComponent<{
     return <PlaceholderPanel text='No errors' />;
 
   return <div className='fill' style={{ overflow: 'auto' }}>
-    {[...errorsModel.errors.entries()].map(([message, action]) => {
+    {[...errorsModel.errors.entries()].map(([message, error]) => {
       let location: string | undefined;
       let longLocation: string | undefined;
-      if (action.stack?.[0]) {
-        const file = action.stack[0].file.replace(/.*\/(.*)/, '$1');
-        location = file + ':' + action.stack[0].line;
-        longLocation = action.stack[0].file + ':' + action.stack[0].line;
+      const stackFrame = error.stack?.[0];
+      if (stackFrame) {
+        const file = stackFrame.file.replace(/.*\/(.*)/, '$1');
+        location = file + ':' + stackFrame.line;
+        longLocation = stackFrame.file + ':' + stackFrame.line;
       }
       return <div key={message}>
         <div className='hbox' style={{
@@ -62,9 +93,9 @@ export const ErrorsTab: React.FunctionComponent<{
           fontWeight: 'bold',
           color: 'var(--vscode-errorForeground)',
         }}>
-          {renderAction(action, { sdkLanguage })}
+          {error.action && renderAction(error.action, { sdkLanguage })}
           {location && <div className='action-location'>
-            @ <span title={longLocation} onClick={() => revealInSource(action)}>{location}</span>
+            @ <span title={longLocation} onClick={() => error.action && revealInSource(error.action)}>{location}</span>
           </div>}
         </div>
         <ErrorMessage error={message} />

packages/trace-viewer/src/ui/modelUtil.ts

@@ -61,6 +61,7 @@ export class MultiTraceModel {
   readonly actions: ActionTraceEventInContext[];
   readonly events: (trace.EventTraceEvent | trace.ConsoleMessageTraceEvent)[];
   readonly stdio: trace.StdioTraceEvent[];
+  readonly errors: trace.ErrorTraceEvent[];
   readonly hasSource: boolean;
   readonly sdkLanguage: Language | undefined;
   readonly testIdAttributeName: string | undefined;
@@ -86,6 +87,7 @@ export class MultiTraceModel {
     this.actions = mergeActions(contexts);
     this.events = ([] as (trace.EventTraceEvent | trace.ConsoleMessageTraceEvent)[]).concat(...contexts.map(c => c.events));
     this.stdio = ([] as trace.StdioTraceEvent[]).concat(...contexts.map(c => c.stdio));
+    this.errors = ([] as trace.ErrorTraceEvent[]).concat(...contexts.map(c => c.errors));
     this.hasSource = contexts.some(c => c.hasSource);
     this.resources = [...contexts.map(c => c.resources)].flat();
 

packages/trace/src/trace.ts

@@ -145,6 +145,12 @@ export type StdioTraceEvent = {
   base64?: string;
 };
 
+export type ErrorTraceEvent = {
+  type: 'error';
+  message: string;
+  stack?: StackFrame[];
+};
+
 export type TraceEvent =
     ContextCreatedTraceEvent |
     ScreencastFrameTraceEvent |
@@ -157,4 +163,5 @@ export type TraceEvent =
     ConsoleMessageTraceEvent |
     ResourceSnapshotTraceEvent |
     FrameSnapshotTraceEvent |
-    StdioTraceEvent;
+    StdioTraceEvent |
+    ErrorTraceEvent;

tests/config/utils.ts

@@ -156,7 +156,7 @@ export async function parseTraceRaw(file: string): Promise<{ events: any[], reso
   };
 }
 
-export async function parseTrace(file: string): Promise<{ resources: Map<string, Buffer>, events: (EventTraceEvent | ConsoleMessageTraceEvent)[], actions: ActionTraceEvent[], apiNames: string[], traceModel: TraceModel, model: MultiTraceModel, actionTree: string[] }> {
+export async function parseTrace(file: string): Promise<{ resources: Map<string, Buffer>, events: (EventTraceEvent | ConsoleMessageTraceEvent)[], actions: ActionTraceEvent[], apiNames: string[], traceModel: TraceModel, model: MultiTraceModel, actionTree: string[], errors: string[] }> {
   const backend = new TraceBackend(file);
   const traceModel = new TraceModel();
   await traceModel.load(backend, () => {});
@@ -174,6 +174,7 @@ export async function parseTrace(file: string): Promise<{ resources: Map<string,
     resources: backend.entries,
     actions: model.actions,
     events: model.events,
+    errors: model.errors.map(e => e.message),
     model,
     traceModel,
     actionTree,

tests/playwright-test/playwright.trace.spec.ts

@@ -323,7 +323,6 @@ test('should not override trace file in afterAll', async ({ runInlineTest, serve
     '  fixture: page',
     '    browserContext.newPage',
     'page.goto',
-    'error',
     'After Hooks',
     '  fixture: page',
     '  fixture: context',
@@ -335,6 +334,7 @@ test('should not override trace file in afterAll', async ({ runInlineTest, serve
     '  fixture: request',
     '    apiRequestContext.dispose',
   ]);
+  expect(trace1.errors).toEqual([`'oh no!'`]);
 
   const error = await parseTrace(testInfo.outputPath('test-results', 'a-test-2', 'trace.zip')).catch(e => e);
   expect(error).toBeTruthy();
@@ -668,11 +668,11 @@ test('should show non-expect error in trace', async ({ runInlineTest }, testInfo
     '  fixture: page',
     '    browserContext.newPage',
     'expect.toBe',
-    'ReferenceError: undefinedVariable1 is not defined',
     'After Hooks',
     '  fixture: page',
     '  fixture: context',
   ]);
+  expect(trace.errors).toEqual(['ReferenceError: undefinedVariable1 is not defined']);
 });
 
 test('should not throw when attachment is missing', async ({ runInlineTest }, testInfo) => {

tests/playwright-test/reporter-html.spec.ts

@@ -820,7 +820,7 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       await showReport();
 
       await page.locator('text=sample').first().click();
-      await expect(page.locator('text=ouch')).toHaveCount(2);
+      await expect(page.locator('text=ouch')).toHaveCount(1);
       await page.locator('text=All').first().click();
 
       await page.locator('text=sample').nth(1).click();