diff --git a/packages/playwright-core/src/client/channelOwner.ts b/packages/playwright-core/src/client/channelOwner.ts index a5d753507b..d48ba439f0 100644 --- a/packages/playwright-core/src/client/channelOwner.ts +++ b/packages/playwright-core/src/client/channelOwner.ts @@ -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'; @@ -148,15 +147,18 @@ export abstract class ChannelOwner { 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); }); }; } @@ -170,48 +172,36 @@ export abstract class ChannelOwner(func: (apiZone: ApiZone) => Promise, isInternal?: boolean): Promise { const logger = this._logger; - const apiZone = zones.zoneData('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'); + 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'); - 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\n' + e.stack : ''; - if (apiName && !apiName.includes('')) - e.message = apiName + ': ' + e.message; + if (apiZone.apiName && !apiZone.apiName.includes('')) + 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; } } @@ -232,9 +222,7 @@ export abstract class ChannelOwner; frames: channels.StackFrame[]; isInternal: boolean; reported: boolean; - csi: ClientInstrumentation | undefined; - callCookie: any; + userData: any; stepId?: string; + error?: Error; }; diff --git a/packages/playwright-core/src/client/clientInstrumentation.ts b/packages/playwright-core/src/client/clientInstrumentation.ts index 55c787df05..e2e8c6678e 100644 --- a/packages/playwright-core/src/client/clientInstrumentation.ts +++ b/packages/playwright-core/src/client/clientInstrumentation.ts @@ -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; + 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, 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; @@ -33,8 +43,8 @@ export interface ClientInstrumentation { } export interface ClientInstrumentationListener { - onApiCallBegin?(apiName: string, params: Record, 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; diff --git a/packages/playwright-core/src/client/connection.ts b/packages/playwright-core/src/client/connection.ts index 75d1878da3..f5907a7fec 100644 --- a/packages/playwright-core/src/client/connection.ts +++ b/packages/playwright-core/src/client/connection.ts @@ -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() { diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index 27f1e9320b..29fabb22e8 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -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; @@ -258,34 +259,43 @@ const playwrightFixtures: Fixtures = ({ const tracingGroupSteps: TestStepInternal[] = []; const csiListener: ClientInstrumentationListener = { - onApiCallBegin: (apiName: string, params: Record, 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 step = testInfo._addStep({ - location: frames[0] as any, - category: 'pw:api', - title: renderApiCall(apiName, params), - apiName, - params, - }, tracingGroupSteps[tracingGroupSteps.length - 1]); - userData.step = step; - out.stepId = step.stepId; - if (apiName === 'tracing.group') - tracingGroupSteps.push(step); - }, - onApiCallEnd: (userData: any, error?: Error) => { - // "tracing.group" step will end later, when "tracing.groupEnd" finishes. - if (userData.apiName === 'tracing.group') - return; - if (userData.apiName === 'tracing.groupEnd') { - const step = tracingGroupSteps.pop(); - step?.complete({ error }); + const expectZone = zones.zoneData('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; } - const step = userData.step; - step?.complete({ error }); + // In the general case, create a step for each api call and connect them through the stepId. + const step = testInfo._addStep({ + location: data.frames[0], + category: 'pw:api', + title: renderApiCall(data.apiName, data.params), + apiName: data.apiName, + params: data.params, + }, tracingGroupSteps[tracingGroupSteps.length - 1]); + data.userData = step; + data.stepId = step.stepId; + if (data.apiName === 'tracing.group') + tracingGroupSteps.push(step); + }, + onApiCallEnd: (data: ApiCallData) => { + // "tracing.group" step will end later, when "tracing.groupEnd" finishes. + if (data.apiName === 'tracing.group') + return; + if (data.apiName === 'tracing.groupEnd') { + const step = tracingGroupSteps.pop(); + step?.complete({ error: data.error }); + return; + } + const step = data.userData; + step?.complete({ error: data.error }); }, onWillPause: ({ keepTestTimeout }) => { if (!keepTestTimeout) @@ -441,13 +451,6 @@ const playwrightFixtures: Fixtures = ({ }, }); -type StackFrame = { - file: string, - line?: number, - column?: number, - function?: string, -}; - type ScreenshotOption = PlaywrightWorkerOptions['screenshot'] | undefined; type Playwright = PlaywrightWorkerArgs['playwright'];