chore: remove parsed stack trace (#27496)

This commit is contained in:
Pavel Feldman 2023-10-09 17:04:16 -07:00 committed by GitHub
parent 40ba5ebc1d
commit 11a4b3f7f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 63 additions and 75 deletions

View File

@ -18,8 +18,8 @@ import { EventEmitter } from 'events';
import type * as channels from '@protocol/channels';
import { maybeFindValidator, ValidationError, type ValidatorContext } from '../protocol/validator';
import { debugLogger } from '../common/debugLogger';
import type { ExpectZone, ParsedStackTrace } from '../utils/stackTrace';
import { captureRawStack, captureLibraryStackTrace } from '../utils/stackTrace';
import type { ExpectZone } from '../utils/stackTrace';
import { captureRawStack, captureLibraryStackTrace, stringifyStackFrames } from '../utils/stackTrace';
import { isUnderTest } from '../utils';
import { zones } from '../utils/zones';
import type { ClientInstrumentation } from './clientInstrumentation';
@ -143,11 +143,11 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
if (validator) {
return (params: any) => {
return this._wrapApiCall(apiZone => {
const { stackTrace, csi, callCookie, wallTime } = apiZone.reported ? { csi: undefined, callCookie: undefined, stackTrace: null, wallTime: undefined } : apiZone;
const { apiName, frames, csi, callCookie, wallTime } = apiZone.reported ? { apiName: undefined, csi: undefined, callCookie: undefined, frames: [], wallTime: undefined } : apiZone;
apiZone.reported = true;
if (csi && stackTrace && stackTrace.apiName)
csi.onApiCallBegin(stackTrace.apiName, params, stackTrace, wallTime, callCookie);
return this._connection.sendMessageToServer(this, prop, validator(params, '', { tChannelImpl: tChannelImplToWire, binary: this._connection.isRemote() ? 'toBase64' : 'buffer' }), stackTrace, wallTime);
if (csi && apiName)
csi.onApiCallBegin(apiName, params, frames, wallTime, callCookie);
return this._connection.sendMessageToServer(this, prop, validator(params, '', { tChannelImpl: tChannelImplToWire, binary: this._connection.isRemote() ? 'toBase64' : 'buffer' }), apiName, frames, wallTime);
});
};
}
@ -167,23 +167,25 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
return func(apiZone);
const stackTrace = captureLibraryStackTrace(stack);
let apiName: string | undefined = stackTrace.apiName;
const frames: channels.StackFrame[] = stackTrace.frames;
isInternal = isInternal || this._type === 'LocalUtils';
if (isInternal)
delete stackTrace.apiName;
apiName = undefined;
// Enclosing zone could have provided the apiName and wallTime.
const expectZone = zones.zoneData<ExpectZone>('expectZone', stack);
const wallTime = expectZone ? expectZone.wallTime : Date.now();
if (!isInternal && expectZone)
stackTrace.apiName = expectZone.title;
apiName = expectZone.title;
const csi = isInternal ? undefined : this._instrumentation;
const callCookie: any = {};
const { apiName, frameTexts } = stackTrace;
try {
logApiCall(logger, `=> ${apiName} started`, isInternal);
const apiZone = { stackTrace, isInternal, reported: false, csi, callCookie, wallTime };
const apiZone: ApiZone = { apiName, frames, isInternal, reported: false, csi, callCookie, wallTime };
const result = await zones.run<ApiZone, Promise<R>>('apiZone', apiZone, async () => {
return await func(apiZone);
});
@ -194,7 +196,7 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
const innerError = ((process.env.PWDEBUGIMPL || isUnderTest()) && e.stack) ? '\n<inner error>\n' + e.stack : '';
if (apiName && !apiName.includes('<anonymous>'))
e.message = apiName + ': ' + e.message;
const stackFrames = '\n' + frameTexts.join('\n') + innerError;
const stackFrames = '\n' + stringifyStackFrames(stackTrace.frames).join('\n') + innerError;
if (stackFrames.trim())
e.stack = e.message + stackFrames;
else
@ -236,7 +238,8 @@ function tChannelImplToWire(names: '*' | string[], arg: any, path: string, conte
}
type ApiZone = {
stackTrace: ParsedStackTrace;
apiName: string | undefined;
frames: channels.StackFrame[];
isInternal: boolean;
reported: boolean;
csi: ClientInstrumentation | undefined;

View File

@ -14,7 +14,7 @@
* limitations under the License.
*/
import type { ParsedStackTrace } from '../utils/stackTrace';
import type { StackFrame } from '@protocol/channels';
import type { BrowserContext } from './browserContext';
import type { APIRequestContext } from './fetch';
@ -22,7 +22,7 @@ export interface ClientInstrumentation {
addListener(listener: ClientInstrumentationListener): void;
removeListener(listener: ClientInstrumentationListener): void;
removeAllListeners(): void;
onApiCallBegin(apiCall: string, params: Record<string, any>, stackTrace: ParsedStackTrace | null, wallTime: number, userData: any): void;
onApiCallBegin(apiCall: string, params: Record<string, any>, frames: StackFrame[], wallTime: number, userData: any): void;
onApiCallEnd(userData: any, error?: Error): void;
onDidCreateBrowserContext(context: BrowserContext): Promise<void>;
onDidCreateRequestContext(context: APIRequestContext): Promise<void>;
@ -32,7 +32,7 @@ export interface ClientInstrumentation {
}
export interface ClientInstrumentationListener {
onApiCallBegin?(apiCall: string, params: Record<string, any>, stackTrace: ParsedStackTrace | null, wallTime: number, userData: any): void;
onApiCallBegin?(apiName: string, params: Record<string, any>, frames: StackFrame[], wallTime: number, userData: any): void;
onApiCallEnd?(userData: any, error?: Error): void;
onDidCreateBrowserContext?(context: BrowserContext): Promise<void>;
onDidCreateRequestContext?(context: APIRequestContext): Promise<void>;

View File

@ -35,7 +35,7 @@ import { WritableStream } from './writableStream';
import { debugLogger } from '../common/debugLogger';
import { SelectorsOwner } from './selectors';
import { Android, AndroidSocket, AndroidDevice } from './android';
import { captureLibraryStackTrace, type ParsedStackTrace } from '../utils/stackTrace';
import { captureLibraryStackText, stringifyStackFrames } from '../utils/stackTrace';
import { Artifact } from './artifact';
import { EventEmitter } from 'events';
import { JsonPipe } from './jsonPipe';
@ -65,7 +65,7 @@ export class Connection extends EventEmitter {
readonly _objects = new Map<string, ChannelOwner>();
onmessage = (message: object): void => {};
private _lastId = 0;
private _callbacks = new Map<number, { resolve: (a: any) => void, reject: (a: Error) => void, stackTrace: ParsedStackTrace | null, type: string, method: string }>();
private _callbacks = new Map<number, { resolve: (a: any) => void, reject: (a: Error) => void, apiName: string | undefined, frames: channels.StackFrame[], type: string, method: string }>();
private _rootObject: Root;
private _closedErrorMessage: string | undefined;
private _isRemote = false;
@ -98,8 +98,14 @@ export class Connection extends EventEmitter {
return await this._rootObject.initialize();
}
pendingProtocolCalls(): ParsedStackTrace[] {
return Array.from(this._callbacks.values()).map(callback => callback.stackTrace).filter(Boolean) as ParsedStackTrace[];
pendingProtocolCalls(): String {
const lines: string[] = [];
for (const call of this._callbacks.values()) {
if (!call.apiName)
continue;
lines.push(` - ${call.apiName}\n${stringifyStackFrames(call.frames)}\n`);
}
return lines.length ? 'Pending operations:\n' + lines.join('\n') : '';
}
getObjectWithKnownName(guid: string): any {
@ -113,13 +119,12 @@ export class Connection extends EventEmitter {
this._tracingCount--;
}
async sendMessageToServer(object: ChannelOwner, method: string, params: any, stackTrace: ParsedStackTrace | null, wallTime: number | undefined): Promise<any> {
async sendMessageToServer(object: ChannelOwner, method: string, params: any, apiName: string | undefined, frames: channels.StackFrame[], wallTime: number | undefined): Promise<any> {
if (this._closedErrorMessage)
throw new Error(this._closedErrorMessage);
if (object._wasCollected)
throw new Error('The object has been collected to prevent unbounded heap growth.');
const { apiName, frames } = stackTrace || { apiName: '', frames: [] };
const guid = object._guid;
const type = object._type;
const id = ++this._lastId;
@ -133,7 +138,7 @@ export class Connection extends EventEmitter {
if (this._tracingCount && frames && type !== 'LocalUtils')
this._localUtils?._channel.addStackToTracingNoReply({ callData: { stack: frames, id } }).catch(() => {});
this.onmessage({ ...message, metadata });
return await new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject, stackTrace, type, method }));
return await new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject, apiName, frames, type, method }));
}
dispatch(message: object) {
@ -186,7 +191,7 @@ export class Connection extends EventEmitter {
}
close(errorMessage: string = 'Connection closed') {
const stack = captureLibraryStackTrace().frameTexts.join('\n');
const stack = captureLibraryStackText();
if (stack)
errorMessage += '\n ==== Closed by ====\n' + stack + '\n';
this._closedErrorMessage = errorMessage;

View File

@ -36,13 +36,6 @@ const internalStackPrefixes = [
];
export const addInternalStackPrefix = (prefix: string) => internalStackPrefixes.push(prefix);
export type ParsedStackTrace = {
allFrames: StackFrame[];
frames: StackFrame[];
frameTexts: string[];
apiName: string | undefined;
};
export type RawStack = string[];
export function captureRawStack(): RawStack {
@ -54,7 +47,7 @@ export function captureRawStack(): RawStack {
return stack.split('\n');
}
export function captureLibraryStackTrace(rawStack?: RawStack): ParsedStackTrace {
export function captureLibraryStackTrace(rawStack?: RawStack): { frames: StackFrame[], apiName: string } {
const stack = rawStack || captureRawStack();
const isTesting = isUnderTest();
@ -79,7 +72,6 @@ export function captureLibraryStackTrace(rawStack?: RawStack): ParsedStackTrace
}).filter(Boolean) as ParsedFrame[];
let apiName = '';
const allFrames = parsedFrames;
// Deepest transition between non-client code calling into client
// code is the api entry.
@ -110,13 +102,27 @@ export function captureLibraryStackTrace(rawStack?: RawStack): ParsedStackTrace
});
return {
allFrames: allFrames.map(p => p.frame),
frames: parsedFrames.map(p => p.frame),
frameTexts: parsedFrames.map(p => p.frameText),
apiName
};
}
export function stringifyStackFrames(frames: StackFrame[]): string[] {
const stackLines: string[] = [];
for (const frame of frames) {
if (frame.function)
stackLines.push(` at ${frame.function} (${frame.file}:${frame.line}:${frame.column})`);
else
stackLines.push(` at ${frame.file}:${frame.line}:${frame.column}`);
}
return stackLines;
}
export function captureLibraryStackText() {
const parsed = captureLibraryStackTrace();
return stringifyStackFrames(parsed.frames).join('\n');
}
export function splitErrorMessage(message: string): { name: string, message: string } {
const separationIdx = message.indexOf(':');
return {

View File

@ -24,7 +24,6 @@ import type { TestInfoImpl } from './worker/testInfo';
import { rootTestType } from './common/testType';
import type { ContextReuseMode } from './common/config';
import type { ClientInstrumentation, ClientInstrumentationListener } from '../../playwright-core/src/client/clientInstrumentation';
import type { ParsedStackTrace } from '../../playwright-core/src/utils/stackTrace';
import { currentTestInfo } from './common/globals';
import { mergeTraceFiles } from './worker/testTracing';
export { expect } from './matchers/expect';
@ -253,12 +252,12 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
const artifactsRecorder = new ArtifactsRecorder(playwright, _artifactsDir, trace, screenshot);
await artifactsRecorder.willStartTest(testInfo as TestInfoImpl);
const csiListener: ClientInstrumentationListener = {
onApiCallBegin: (apiName: string, params: Record<string, any>, stackTrace: ParsedStackTrace | null, wallTime: number, userData: any) => {
onApiCallBegin: (apiName: string, params: Record<string, any>, frames: StackFrame[], wallTime: number, userData: any) => {
const testInfo = currentTestInfo();
if (!testInfo || apiName.startsWith('expect.') || apiName.includes('setTestIdAttribute'))
return { userObject: null };
const step = testInfo._addStep({
location: stackTrace?.frames[0] as any,
location: frames[0] as any,
category: 'pw:api',
title: renderApiCall(apiName, params),
apiName,
@ -331,8 +330,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
return context;
});
const prependToError = testInfoImpl._didTimeout ?
formatPendingCalls((browser as any)._connection.pendingProtocolCalls()) : '';
const prependToError = testInfoImpl._didTimeout ? (browser as any)._connection.pendingProtocolCalls() : '';
let counter = 0;
await Promise.all([...contexts.keys()].map(async context => {
@ -403,22 +401,6 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
},
});
function formatPendingCalls(calls: ParsedStackTrace[]) {
calls = calls.filter(call => !!call.apiName);
if (!calls.length)
return '';
return 'Pending operations:\n' + calls.map(call => {
const frame = call.frames && call.frames[0] ? ' at ' + formatStackFrame(call.frames[0]) : '';
return ` - ${call.apiName}${frame}\n`;
}).join('');
}
function formatStackFrame(frame: StackFrame) {
const file = path.relative(process.cwd(), frame.file) || path.basename(frame.file);
return `${file}:${frame.line || 1}:${frame.column || 1}`;
}
function hookType(testInfo: TestInfoImpl): 'beforeAll' | 'afterAll' | undefined {
const type = testInfo._timeoutManager.currentRunnableType();
if (type === 'beforeAll' || type === 'afterAll')

View File

@ -17,8 +17,8 @@
import { colors } from 'playwright-core/lib/utilsBundle';
import type { ExpectMatcherContext } from './expect';
import type { Locator } from 'playwright-core';
import { stringifyStackFrames } from '../util';
import type { StackFrame } from '@protocol/channels';
import { stringifyStackFrames } from 'playwright-core/lib/utils';
export function matcherHint(state: ExpectMatcherContext, locator: Locator | undefined, matcherName: string, expression: any, actual: any, matcherOptions: any, timeout?: number) {
let header = state.utils.matcherHint(matcherName, expression, actual, matcherOptions).replace(/ \/\/ deep equality/, '') + '\n\n';

View File

@ -23,7 +23,7 @@ import url from 'url';
import { colors, debug, minimatch, parseStackTraceLine } from 'playwright-core/lib/utilsBundle';
import type { TestInfoError } from './../types/test';
import type { Location } from './../types/testReporter';
import { calculateSha1, isRegExp, isString, sanitizeForFilePath } from 'playwright-core/lib/utils';
import { calculateSha1, isRegExp, isString, sanitizeForFilePath, stringifyStackFrames } from 'playwright-core/lib/utils';
import type { RawStack } from 'playwright-core/lib/utils';
const PLAYWRIGHT_TEST_PATH = path.join(__dirname, '..');
@ -61,17 +61,6 @@ export function filteredStackTrace(rawStack: RawStack): StackFrame[] {
return frames;
}
export function stringifyStackFrames(frames: StackFrame[]): string[] {
const stackLines: string[] = [];
for (const frame of frames) {
if (frame.function)
stackLines.push(` at ${frame.function} (${frame.file}:${frame.line}:${frame.column})`);
else
stackLines.push(` at ${frame.file}:${frame.line}:${frame.column}`);
}
return stackLines;
}
export function serializeError(error: Error | any): TestInfoError {
if (error instanceof Error)
return filterStackTrace(error);

View File

@ -16,7 +16,7 @@
import fs from 'fs';
import path from 'path';
import { MaxTime, captureRawStack, monotonicTime, zones, sanitizeForFilePath } from 'playwright-core/lib/utils';
import { MaxTime, captureRawStack, monotonicTime, zones, sanitizeForFilePath, stringifyStackFrames } from 'playwright-core/lib/utils';
import type { TestInfoError, TestInfo, TestStatus, FullProject, FullConfig } from '../../types/test';
import type { AttachmentPayload, StepBeginPayload, StepEndPayload, WorkerInitParams } from '../common/ipc';
import type { TestCase } from '../common/test';
@ -24,7 +24,7 @@ import { TimeoutManager } from './timeoutManager';
import type { RunnableType, TimeSlot } from './timeoutManager';
import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config';
import type { Location } from '../../types/testReporter';
import { filteredStackTrace, getContainedPath, normalizeAndSaveAttachment, serializeError, stringifyStackFrames, trimLongString } from '../util';
import { filteredStackTrace, getContainedPath, normalizeAndSaveAttachment, serializeError, trimLongString } from '../util';
import { TestTracing } from './testTracing';
import type { Attachment } from './testTracing';

View File

@ -227,7 +227,7 @@ test('should write missing expectations locally twice and continue', async ({ ru
expect(result.output).toContain('Here we are!');
const stackLines = result.output.split('\n').filter(line => line.includes(' at ')).filter(line => !line.includes(testInfo.outputPath()));
const stackLines = result.output.split('\n').filter(line => line.includes(' at ')).filter(line => !line.includes('a.spec.js'));
expect(result.output).toContain('a.spec.js:4');
expect(stackLines.length).toBe(0);
});

View File

@ -340,8 +340,10 @@ test('should report error and pending operations on timeout', async ({ runInline
expect(result.passed).toBe(0);
expect(result.failed).toBe(1);
expect(result.output).toContain('Pending operations:');
expect(result.output).toContain('- locator.click at a.test.ts:6:37');
expect(result.output).toContain('- locator.textContent at a.test.ts:7:42');
expect(result.output).toContain('- locator.click');
expect(result.output).toContain('a.test.ts:6:37');
expect(result.output).toContain('- locator.textContent');
expect(result.output).toContain('a.test.ts:7:42');
expect(result.output).toContain('waiting for');
expect(result.output).toContain(`7 | page.getByText('More missing').textContent(),`);
});
@ -410,7 +412,8 @@ test('should not report waitForEventInfo as pending', async ({ runInlineTest })
expect(result.passed).toBe(0);
expect(result.failed).toBe(1);
expect(result.output).toContain('Pending operations:');
expect(result.output).toContain('- page.click at a.test.ts:6:20');
expect(result.output).toContain('- page.click');
expect(result.output).toContain('a.test.ts:6:20');
expect(result.output).not.toContain('- page.waitForLoadState');
});

View File

@ -598,7 +598,7 @@ test('should write missing expectations locally twice and attach them', async ({
expect(result.output).toContain('Here we are!');
const stackLines = result.output.split('\n').filter(line => line.includes(' at ')).filter(line => !line.includes(testInfo.outputPath()));
const stackLines = result.output.split('\n').filter(line => line.includes(' at ')).filter(line => !line.includes('a.spec.js'));
expect(result.output).toContain('a.spec.js:5');
expect(stackLines.length).toBe(0);