Skip to content

Commit

Permalink
chore: remove dependency from library on expectZone, straighten csi h…
Browse files Browse the repository at this point in the history 
…andling (#34211)
  • Loading branch information
@dgozman
dgozman authored Jan 7, 2025
1 parent 527505e commit 63329a3
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 74 deletions.
75 changes: 32 additions & 43 deletions packages/playwright-core/src/client/channelOwner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { EventEmitter } from './eventEmitter';
import type * as channels from '@protocol/channels';
import { maybeFindValidator, ValidationError, type ValidatorContext } from '../protocol/validator';
import { debugLogger } from '../utils/debugLogger';
import type { ExpectZone } from '../utils/stackTrace';
import { captureLibraryStackTrace, stringifyStackFrames } from '../utils/stackTrace';
import { isUnderTest } from '../utils';
import { zones } from '../utils/zones';
Expand Down Expand Up @@ -148,15 +147,18 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
if (validator) {
return async (params: any) => {
return await this._wrapApiCall(async apiZone => {
const { apiName, frames, csi, callCookie, stepId } = apiZone.reported ? { apiName: undefined, csi: undefined, callCookie: undefined, frames: [], stepId: undefined } : apiZone;
apiZone.reported = true;
let currentStepId = stepId;
if (csi && apiName) {
const out: { stepId?: string } = {};
csi.onApiCallBegin(apiName, params, frames, callCookie, out);
currentStepId = out.stepId;
const validatedParams = validator(params, '', { tChannelImpl: tChannelImplToWire, binary: this._connection.rawBuffers() ? 'buffer' : 'toBase64' });
if (!apiZone.isInternal && !apiZone.reported) {
// Reporting/tracing/logging this api call for the first time.
apiZone.params = params;
apiZone.reported = true;
this._instrumentation.onApiCallBegin(apiZone);
logApiCall(this._logger, `=> ${apiZone.apiName} started`);
return await this._connection.sendMessageToServer(this, prop, validatedParams, apiZone.apiName, apiZone.frames, apiZone.stepId);
}
return await this._connection.sendMessageToServer(this, prop, validator(params, '', { tChannelImpl: tChannelImplToWire, binary: this._connection.rawBuffers() ? 'buffer' : 'toBase64' }), apiName, frames, currentStepId);
// Since this api call is either internal, or has already been reported/traced once,
// passing undefined apiName will avoid an extra unneeded tracing entry.
return await this._connection.sendMessageToServer(this, prop, validatedParams, undefined, [], undefined);
});
};
}
Expand All @@ -170,48 +172,36 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel

async _wrapApiCall<R>(func: (apiZone: ApiZone) => Promise<R>, isInternal?: boolean): Promise<R> {
const logger = this._logger;
const apiZone = zones.zoneData<ApiZone>('apiZone');
if (apiZone)
return await func(apiZone);

const stackTrace = captureLibraryStackTrace();
let apiName: string | undefined = stackTrace.apiName;
const frames: channels.StackFrame[] = stackTrace.frames;
const existingApiZone = zones.zoneData<ApiZone>('apiZone');
if (existingApiZone)
return await func(existingApiZone);

if (isInternal === undefined)
isInternal = this._isInternalType;
if (isInternal)
apiName = undefined;

// Enclosing zone could have provided the apiName and wallTime.
const expectZone = zones.zoneData<ExpectZone>('expectZone');
const stepId = expectZone?.stepId;
if (!isInternal && expectZone)
apiName = expectZone.title;

// If we are coming from the expectZone, there is no need to generate a new
// step for the API call, since it will be generated by the expect itself.
const csi = isInternal || expectZone ? undefined : this._instrumentation;
const callCookie: any = {};
const stackTrace = captureLibraryStackTrace();
const apiZone: ApiZone = { apiName: stackTrace.apiName, frames: stackTrace.frames, isInternal, reported: false, userData: undefined, stepId: undefined };

try {
logApiCall(logger, `=> ${apiName} started`, isInternal);
const apiZone: ApiZone = { apiName, frames, isInternal, reported: false, csi, callCookie, stepId };
const result = await zones.run('apiZone', apiZone, async () => await func(apiZone));
csi?.onApiCallEnd(callCookie);
logApiCall(logger, `<= ${apiName} succeeded`, isInternal);
if (!isInternal) {
logApiCall(logger, `<= ${apiZone.apiName} succeeded`);
this._instrumentation.onApiCallEnd(apiZone);
}
return result;
} catch (e) {
const innerError = ((process.env.PWDEBUGIMPL || isUnderTest()) && e.stack) ? '\n<inner error>\n' + e.stack : '';
if (apiName && !apiName.includes('<anonymous>'))
e.message = apiName + ': ' + e.message;
if (apiZone.apiName && !apiZone.apiName.includes('<anonymous>'))
e.message = apiZone.apiName + ': ' + e.message;
const stackFrames = '\n' + stringifyStackFrames(stackTrace.frames).join('\n') + innerError;
if (stackFrames.trim())
e.stack = e.message + stackFrames;
else
e.stack = '';
csi?.onApiCallEnd(callCookie, e);
logApiCall(logger, `<= ${apiName} failed`, isInternal);
if (!isInternal) {
apiZone.error = e;
logApiCall(logger, `<= ${apiZone.apiName} failed`);
this._instrumentation.onApiCallEnd(apiZone);
}
throw e;
}
}
Expand All @@ -232,9 +222,7 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
}
}

function logApiCall(logger: Logger | undefined, message: string, isNested: boolean) {
if (isNested)
return;
function logApiCall(logger: Logger | undefined, message: string) {
if (logger && logger.isEnabled('api', 'info'))
logger.log('api', 'info', message, [], { color: 'cyan' });
debugLogger.log('api', message);
Expand All @@ -247,11 +235,12 @@ function tChannelImplToWire(names: '*' | string[], arg: any, path: string, conte
}

type ApiZone = {
apiName: string | undefined;
apiName: string;
params?: Record<string, any>;
frames: channels.StackFrame[];
isInternal: boolean;
reported: boolean;
csi: ClientInstrumentation | undefined;
callCookie: any;
userData: any;
stepId?: string;
error?: Error;
};
18 changes: 14 additions & 4 deletions packages/playwright-core/src/client/clientInstrumentation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,22 @@ import type { StackFrame } from '@protocol/channels';
import type { BrowserContext } from './browserContext';
import type { APIRequestContext } from './fetch';

// Instrumentation can mutate the data, for example change apiName or stepId.
export interface ApiCallData {
apiName: string;
params?: Record<string, any>;
frames: StackFrame[];
userData: any;
stepId?: string;
error?: Error;
}

export interface ClientInstrumentation {
addListener(listener: ClientInstrumentationListener): void;
removeListener(listener: ClientInstrumentationListener): void;
removeAllListeners(): void;
onApiCallBegin(apiCall: string, params: Record<string, any>, frames: StackFrame[], userData: any, out: { stepId?: string }): void;
onApiCallEnd(userData: any, error?: Error): void;
onApiCallBegin(apiCall: ApiCallData): void;
onApiCallEnd(apiCal: ApiCallData): void;
onWillPause(options: { keepTestTimeout: boolean }): void;

runAfterCreateBrowserContext(context: BrowserContext): Promise<void>;
Expand All @@ -33,8 +43,8 @@ export interface ClientInstrumentation {
}

export interface ClientInstrumentationListener {
onApiCallBegin?(apiName: string, params: Record<string, any>, frames: StackFrame[], userData: any, out: { stepId?: string }): void;
onApiCallEnd?(userData: any, error?: Error): void;
onApiCallBegin?(apiCall: ApiCallData): void;
onApiCallEnd?(apiCall: ApiCallData): void;
onWillPause?(options: { keepTestTimeout: boolean }): void;

runAfterCreateBrowserContext?(context: BrowserContext): Promise<void>;
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/client/connection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ export class Connection extends EventEmitter {

constructor(localUtils: LocalUtils | undefined, instrumentation: ClientInstrumentation | undefined) {
super();
this._rootObject = new Root(this);
this._localUtils = localUtils;
this._instrumentation = instrumentation || createInstrumentation();
this._localUtils = localUtils;
this._rootObject = new Root(this);
}

markAsRemote() {
Expand Down
53 changes: 28 additions & 25 deletions packages/playwright/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ import * as fs from 'fs';
import * as path from 'path';
import type { APIRequestContext, BrowserContext, Browser, BrowserContextOptions, LaunchOptions, Page, Tracing, Video } from 'playwright-core';
import * as playwrightLibrary from 'playwright-core';
import { createGuid, debugMode, addInternalStackPrefix, isString, asLocator, jsonStringifyForceASCII } from 'playwright-core/lib/utils';
import { createGuid, debugMode, addInternalStackPrefix, isString, asLocator, jsonStringifyForceASCII, zones } from 'playwright-core/lib/utils';
import type { ExpectZone } from 'playwright-core/lib/utils';
import type { Fixtures, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, ScreenshotMode, TestInfo, TestType, VideoMode } from '../types/test';
import type { TestInfoImpl, TestStepInternal } 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 { ApiCallData, ClientInstrumentation, ClientInstrumentationListener } from '../../playwright-core/src/client/clientInstrumentation';
import { currentTestInfo } from './common/globals';
export { expect } from './matchers/expect';
export const _baseTest: TestType<{}, {}> = rootTestType.test;
Expand Down Expand Up @@ -258,34 +259,43 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({

const tracingGroupSteps: TestStepInternal[] = [];
const csiListener: ClientInstrumentationListener = {
onApiCallBegin: (apiName: string, params: Record<string, any>, frames: StackFrame[], userData: any, out: { stepId?: string }) => {
userData.apiName = apiName;
onApiCallBegin: (data: ApiCallData) => {
const testInfo = currentTestInfo();
if (!testInfo || apiName.includes('setTestIdAttribute') || apiName === 'tracing.groupEnd')
// Some special calls do not get into steps.
if (!testInfo || data.apiName.includes('setTestIdAttribute') || data.apiName === 'tracing.groupEnd')
return;
const expectZone = zones.zoneData<ExpectZone>('expectZone');
if (expectZone) {
// Display the internal locator._expect call under the name of the enclosing expect call,
// and connect it to the existing expect step.
data.apiName = expectZone.title;
data.stepId = expectZone.stepId;
return;
}
// In the general case, create a step for each api call and connect them through the stepId.
const step = testInfo._addStep({
location: frames[0] as any,
location: data.frames[0],
category: 'pw:api',
title: renderApiCall(apiName, params),
apiName,
params,
title: renderApiCall(data.apiName, data.params),
apiName: data.apiName,
params: data.params,
}, tracingGroupSteps[tracingGroupSteps.length - 1]);
userData.step = step;
out.stepId = step.stepId;
if (apiName === 'tracing.group')
data.userData = step;
data.stepId = step.stepId;
if (data.apiName === 'tracing.group')
tracingGroupSteps.push(step);
},
onApiCallEnd: (userData: any, error?: Error) => {
onApiCallEnd: (data: ApiCallData) => {
// "tracing.group" step will end later, when "tracing.groupEnd" finishes.
if (userData.apiName === 'tracing.group')
if (data.apiName === 'tracing.group')
return;
if (userData.apiName === 'tracing.groupEnd') {
if (data.apiName === 'tracing.groupEnd') {
const step = tracingGroupSteps.pop();
step?.complete({ error });
step?.complete({ error: data.error });
return;
}
const step = userData.step;
step?.complete({ error });
const step = data.userData;
step?.complete({ error: data.error });
},
onWillPause: ({ keepTestTimeout }) => {
if (!keepTestTimeout)
Expand Down Expand Up @@ -441,13 +451,6 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
},
});

type StackFrame = {
file: string,
line?: number,
column?: number,
function?: string,
};

type ScreenshotOption = PlaywrightWorkerOptions['screenshot'] | undefined;
type Playwright = PlaywrightWorkerArgs['playwright'];

Expand Down

0 comments on commit 63329a3

Please sign in to comment.