From 55a0a7d89297f84bea163fe797bd26b8821d354a Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Thu, 26 Sep 2024 16:40:01 +0200 Subject: [PATCH] feat(pwt): serialize Error.cause as multiple errors from Worker process --- packages/playwright/src/util.ts | 16 ++++++--- packages/playwright/src/worker/testInfo.ts | 8 ++--- packages/playwright/src/worker/testTracing.ts | 18 +++++----- packages/playwright/src/worker/workerMain.ts | 6 ++-- tests/playwright-test/reporter-base.spec.ts | 35 ++++++++++++++++++- 5 files changed, 62 insertions(+), 21 deletions(-) diff --git a/packages/playwright/src/util.ts b/packages/playwright/src/util.ts index 460b3de07e..605f62a997 100644 --- a/packages/playwright/src/util.ts +++ b/packages/playwright/src/util.ts @@ -62,12 +62,18 @@ export function filteredStackTrace(rawStack: RawStack): StackFrame[] { return frames; } -export function serializeError(error: Error | any): TestInfoError { - if (error instanceof Error) - return filterStackTrace(error); - return { +export function serializeError(error: Error | any): TestInfoError[] { + if (error instanceof Error) { + const errors = [filterStackTrace(error)]; + while (error.cause) { + error = error.cause; + errors.push(filterStackTrace(error)); + } + return errors; + } + return [{ value: util.inspect(error) - }; + }]; } export type Matcher = (value: string) => boolean; diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts index 378b32524f..ab87092bba 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/packages/playwright/src/worker/testInfo.ts @@ -272,7 +272,7 @@ export class TestInfoImpl implements TestInfo { if (result.error) { if (typeof result.error === 'object' && !(result.error as any)?.[stepSymbol]) (result.error as any)[stepSymbol] = step; - const error = serializeError(result.error); + const error = serializeError(result.error)[0]; if (data.boxedStack) error.stack = `${error.message}\n${stringifyStackFrames(data.boxedStack).join('\n')}`; step.error = error; @@ -333,9 +333,9 @@ export class TestInfoImpl implements TestInfo { const serialized = serializeError(error); const step: TestStepInternal | undefined = typeof error === 'object' ? (error as any)?.[stepSymbol] : undefined; if (step && step.boxedStack) - serialized.stack = `${(error as Error).name}: ${(error as Error).message}\n${stringifyStackFrames(step.boxedStack).join('\n')}`; - this.errors.push(serialized); - this._tracing.appendForError(serialized); + serialized[0].stack = `${(error as Error).name}: ${(error as Error).message}\n${stringifyStackFrames(step.boxedStack).join('\n')}`; + this.errors.push(...serialized); + this._tracing.appendForError(...serialized); } async _runAsStage(stage: TestStage, cb: () => Promise) { diff --git a/packages/playwright/src/worker/testTracing.ts b/packages/playwright/src/worker/testTracing.ts index fed7fdde7e..fefa90a3db 100644 --- a/packages/playwright/src/worker/testTracing.ts +++ b/packages/playwright/src/worker/testTracing.ts @@ -219,14 +219,16 @@ export class TestTracing { this._testInfo.attachments.push({ name: 'trace', path: tracePath, contentType: 'application/zip' }); } - 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, - }); + appendForError(...errors: TestInfoError[]) { + for (const error of errors) { + 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) { diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index f180f3d08b..7217d504d2 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -112,7 +112,7 @@ export class WorkerMain extends ProcessRunner { await fakeTestInfo._runAsStage({ title: 'worker cleanup', runnable }, () => gracefullyCloseAll()).catch(() => {}); this._fatalErrors.push(...fakeTestInfo.errors); } catch (e) { - this._fatalErrors.push(serializeError(e)); + this._fatalErrors.push(...serializeError(e)); } if (this._fatalErrors.length) { @@ -153,7 +153,7 @@ export class WorkerMain extends ProcessRunner { // No current test - fatal error. if (!this._currentTest) { if (!this._fatalErrors.length) - this._fatalErrors.push(serializeError(error)); + this._fatalErrors.push(...serializeError(error)); void this._stop(); return; } @@ -224,7 +224,7 @@ export class WorkerMain extends ProcessRunner { // In theory, we should run above code without any errors. // However, in the case we screwed up, or loadTestFile failed in the worker // but not in the runner, let's do a fatal error. - this._fatalErrors.push(serializeError(e)); + this._fatalErrors.push(...serializeError(e)); void this._stop(); } finally { const donePayload: DonePayload = { diff --git a/tests/playwright-test/reporter-base.spec.ts b/tests/playwright-test/reporter-base.spec.ts index 6d6e499605..7638ba7244 100644 --- a/tests/playwright-test/reporter-base.spec.ts +++ b/tests/playwright-test/reporter-base.spec.ts @@ -17,7 +17,7 @@ import { test, expect } from './playwright-test-fixtures'; import * as path from 'path'; -for (const useIntermediateMergeReport of [false, true] as const) { +for (const useIntermediateMergeReport of [false] as const) { test.describe(`${useIntermediateMergeReport ? 'merged' : 'created'}`, () => { test.use({ useIntermediateMergeReport }); @@ -118,6 +118,39 @@ for (const useIntermediateMergeReport of [false, true] as const) { expect(output).toContain(`a.spec.ts:5:13`); }); + test('should print error with a nested cause', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('foobar', async ({}) => { + try { + try { + throw new Error('my-message'); + } catch (e) { + try { + throw new Error('inner-message', { cause: e }); + } catch (e) { + throw new Error('outer-message', { cause: e }); + } + } + } catch (e) { + throw new Error('wrapper-message', { cause: e }); + } + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + const testFile = path.join(result.report.config.rootDir, result.report.suites[0].specs[0].file); + expect(result.output).toContain(` at fn (${testFile}:15:21)`); + expect(result.output).toContain(` Error: outer-message`); + expect(result.output).toContain(` at fn (${testFile}:11:25)`); + expect(result.output).toContain(` Error: inner-message`); + expect(result.output).toContain(` at fn (${testFile}:9:25)`); + expect(result.output).toContain(` Error: my-message`); + expect(result.output).toContain(` at fn (${testFile}:6:23)`); + }); + test('should print codeframe from a helper', async ({ runInlineTest }) => { const result = await runInlineTest({ 'helper.ts': `