diff --git a/packages/playwright-core/src/client/channelOwner.ts b/packages/playwright-core/src/client/channelOwner.ts index 140389c596ec9..d48ba439f078c 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'; @@ -150,12 +149,15 @@ export abstract class ChannelOwner { 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`); - apiZone.csi?.onApiCallBegin(apiZone); return await this._connection.sendMessageToServer(this, prop, validatedParams, apiZone.apiName, apiZone.frames, apiZone.stepId); } + // 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); }); }; @@ -174,42 +176,32 @@ export abstract class ChannelOwner('expectZone'); - const stepId = expectZone?.stepId; - if (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 apiZone: ApiZone = { apiName, frames, isInternal, reported: false, csi, userData: undefined, stepId }; + const stackTrace = captureLibraryStackTrace(); + const apiZone: ApiZone = { apiName: stackTrace.apiName, frames: stackTrace.frames, isInternal, reported: false, userData: undefined, stepId: undefined }; try { const result = await zones.run('apiZone', apiZone, async () => await func(apiZone)); - csi?.onApiCallEnd(apiZone); - if (!isInternal) - logApiCall(logger, `<= ${apiName} succeeded`); + 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(apiZone); - if (!isInternal) - logApiCall(logger, `<= ${apiName} failed`); + if (!isInternal) { + apiZone.error = e; + logApiCall(logger, `<= ${apiZone.apiName} failed`); + this._instrumentation.onApiCallEnd(apiZone); + } throw e; } } @@ -248,7 +240,6 @@ type ApiZone = { frames: channels.StackFrame[]; isInternal: boolean; reported: boolean; - csi: ClientInstrumentation | undefined; userData: any; stepId?: string; error?: Error; diff --git a/packages/playwright-core/src/client/connection.ts b/packages/playwright-core/src/client/connection.ts index 75d1878da3ea4..f5907a7fec551 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 c117c695b60be..29fabb22e8791 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -18,7 +18,8 @@ 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'; @@ -260,8 +261,18 @@ const playwrightFixtures: Fixtures = ({ const csiListener: ClientInstrumentationListener = { onApiCallBegin: (data: ApiCallData) => { const testInfo = currentTestInfo(); + // Some special calls do not get into steps. if (!testInfo || data.apiName.includes('setTestIdAttribute') || data.apiName === 'tracing.groupEnd') return; + 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; + } + // 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', @@ -440,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'];