diff --git a/packages/trace-viewer/src/ui/errorsTab.tsx b/packages/trace-viewer/src/ui/errorsTab.tsx index eb1b529c26..e600bb4be2 100644 --- a/packages/trace-viewer/src/ui/errorsTab.tsx +++ b/packages/trace-viewer/src/ui/errorsTab.tsx @@ -31,39 +31,14 @@ type ErrorsTabModel = { errors: Map; }; -function errorsFromActions(model: modelUtil.MultiTraceModel): Map { - const errors = new Map(); - 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 { - const actionErrors = errorsFromActions(model); - const errors = new Map(); - 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(() => { 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) }; + const errors = new Map(); + for (const error of model.errorDescriptors) + errors.set(error.message, error); + return { errors }; }, [model]); } diff --git a/packages/trace-viewer/src/ui/modelUtil.ts b/packages/trace-viewer/src/ui/modelUtil.ts index f29b4f7616..1b6b7cdff4 100644 --- a/packages/trace-viewer/src/ui/modelUtil.ts +++ b/packages/trace-viewer/src/ui/modelUtil.ts @@ -19,6 +19,7 @@ import type { ResourceSnapshot } from '@trace/snapshot'; import type * as trace from '@trace/trace'; import type { ActionTraceEvent } from '@trace/trace'; import type { ContextEntry, PageEntry } from '../entries'; +import type { StackFrame } from '@protocol/channels'; const contextSymbol = Symbol('context'); const nextInContextSymbol = Symbol('next'); @@ -48,6 +49,12 @@ export type ActionTreeItem = { action?: ActionTraceEventInContext; }; +type ErrorDescription = { + action?: ActionTraceEventInContext; + stack?: StackFrame[]; + message: string; +}; + export class MultiTraceModel { readonly startTime: number; readonly endTime: number; @@ -62,7 +69,9 @@ export class MultiTraceModel { readonly events: (trace.EventTraceEvent | trace.ConsoleMessageTraceEvent)[]; readonly stdio: trace.StdioTraceEvent[]; readonly errors: trace.ErrorTraceEvent[]; + readonly errorDescriptors: ErrorDescription[]; readonly hasSource: boolean; + readonly hasStepData: boolean; readonly sdkLanguage: Language | undefined; readonly testIdAttributeName: string | undefined; readonly sources: Map; @@ -89,17 +98,46 @@ export class MultiTraceModel { 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.hasStepData = contexts.some(context => !context.isPrimary); this.resources = [...contexts.map(c => c.resources)].flat(); this.events.sort((a1, a2) => a1.time - a2.time); this.resources.sort((a1, a2) => a1._monotonicTime! - a2._monotonicTime!); - this.sources = collectSources(this.actions); + this.errorDescriptors = this.hasStepData ? this._errorDescriptorsFromTestRunner() : this._errorDescriptorsFromActions(); + this.sources = collectSources(this.actions, this.errorDescriptors); } failedAction() { // This find innermost action for nested ones. return this.actions.findLast(a => a.error); } + + private _errorDescriptorsFromActions(): ErrorDescription[] { + const errors: ErrorDescription[] = []; + for (const action of this.actions || []) { + if (!action.error?.message) + continue; + errors.push({ + action, + stack: action.stack, + message: action.error.message, + }); + } + return errors; + } + + private _errorDescriptorsFromTestRunner(): ErrorDescription[] { + const errors: ErrorDescription[] = []; + for (const error of this.errors || []) { + if (!error.message) + continue; + errors.push({ + stack: error.stack, + message: error.message + }); + } + return errors; + } } function indexModel(context: ContextEntry) { @@ -248,7 +286,7 @@ export function eventsForAction(action: ActionTraceEvent): (trace.EventTraceEven return result; } -function collectSources(actions: trace.ActionTraceEvent[]): Map { +function collectSources(actions: trace.ActionTraceEvent[], errorDescriptors: ErrorDescription[]): Map { const result = new Map(); for (const action of actions) { for (const frame of action.stack || []) { @@ -258,8 +296,16 @@ function collectSources(actions: trace.ActionTraceEvent[]): Map { /After Hooks/, ]); }); + +test('should not show caught errors in the errors tab', async ({ runUITest }, testInfo) => { + const { page } = await runUITest({ + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('pass', async ({ page }, testInfo) => { + await page.setContent(""); + await expect(page.locator('input')).toBeChecked({ timeout: 1 }).catch(() => {}); + }); + `, + }); + + await page.getByText('pass').dblclick(); + const listItem = page.getByTestId('actions-tree').getByRole('listitem'); + + await expect( + listItem, + 'action list' + ).toHaveText([ + /Before Hooks[\d.]+m?s/, + /page.setContent/, + /expect.toBeCheckedlocator.*[\d.]+m?s/, + /After Hooks/, + ]); + + await page.getByText('Source', { exact: true }).click(); + await expect(page.locator('.source-line-running')).toContainText('toBeChecked'); + await expect(page.locator('.CodeMirror-linewidget')).toHaveCount(0); + + await page.getByText('Errors', { exact: true }).click(); + await expect(page.locator('.tab-errors')).toHaveText('No errors'); +});