diff --git a/package-lock.json b/package-lock.json index 81e20911a743f..ef651146385c9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13822,7 +13822,6 @@ "version": "2.0.3", "resolved": "https://registry.npmjs.org/stack-utils/-/stack-utils-2.0.3.tgz", "integrity": "sha512-gL//fkxfWUsIlFL2Tl42Cl6+HFALEaB1FU76I/Fy+oZjRreP7OPMXFlGbxM7NQsI0ZpUfw76sHnv0WNYuTb7Iw==", - "dev": true, "requires": { "escape-string-regexp": "^2.0.0" }, @@ -13830,8 +13829,7 @@ "escape-string-regexp": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-2.0.0.tgz", - "integrity": "sha512-UpzcLCXolUWcNu5HtVMHYdXJjArjsF9C0aNnquZYY4uW/Vu0miy5YoWvbV345HauVvcAUnpRuhMMcqTcGOY2+w==", - "dev": true + "integrity": "sha512-UpzcLCXolUWcNu5HtVMHYdXJjArjsF9C0aNnquZYY4uW/Vu0miy5YoWvbV345HauVvcAUnpRuhMMcqTcGOY2+w==" } } }, diff --git a/package.json b/package.json index 7fc6f939cde86..f2cb228e9eeb4 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "proper-lockfile": "^4.1.1", "proxy-from-env": "^1.1.0", "rimraf": "^3.0.2", + "stack-utils": "^2.0.3", "ws": "^7.3.1" }, "devDependencies": { diff --git a/src/client/connection.ts b/src/client/connection.ts index 73d587dde2c30..7cb99100c40df 100644 --- a/src/client/connection.ts +++ b/src/client/connection.ts @@ -41,6 +41,7 @@ import { debugLogger } from '../utils/debugLogger'; import { SelectorsOwner } from './selectors'; import { isUnderTest } from '../utils/utils'; import { Android, AndroidSocket, AndroidDevice } from './android'; +import { captureStackTrace } from '../utils/stackTrace'; class Root extends ChannelOwner { constructor(connection: Connection) { @@ -71,14 +72,12 @@ export class Connection { } async sendMessageToServer(guid: string, method: string, params: any): Promise { - const stackObject: any = {}; - Error.captureStackTrace(stackObject); - const stack = stackObject.stack.startsWith('Error') ? stackObject.stack.substring(5) : stackObject.stack; + const { stack, frames } = captureStackTrace(); const id = ++this._lastId; const converted = { id, guid, method, params }; // Do not include metadata in debug logs to avoid noise. debugLogger.log('channel:command', converted); - this.onmessage({ ...converted, metadata: { stack } }); + this.onmessage({ ...converted, metadata: { stack: frames } }); try { return await new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject })); } catch (e) { diff --git a/src/client/types.ts b/src/client/types.ts index d393c3a278cbd..b84ec772f4573 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -23,14 +23,12 @@ export interface Logger { log(name: string, severity: LoggerSeverity, message: string | Error, args: any[], hints: { color?: string }): void; } -export type Size = { width: number, height: number }; -export type Point = { x: number, y: number }; -export type Rect = Size & Point; +import { Size } from '../common/types'; +export { Size, Point, Rect, Quad, URLMatch, TimeoutOptions } from '../common/types'; + export type Headers = { [key: string]: string }; export type Env = { [key: string]: string | number | boolean | undefined }; -export type URLMatch = string | RegExp | ((url: URL) => boolean); -export type TimeoutOptions = { timeout?: number }; export type WaitForEventOptions = Function | { predicate?: Function, timeout?: number }; export type WaitForFunctionOptions = { timeout?: number, polling?: 'raf' | number }; diff --git a/src/common/types.ts b/src/common/types.ts new file mode 100644 index 0000000000000..e48e687551951 --- /dev/null +++ b/src/common/types.ts @@ -0,0 +1,29 @@ +/** + * Copyright (c) Microsoft Corporation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export type Size = { width: number, height: number }; +export type Point = { x: number, y: number }; +export type Rect = Size & Point; +export type Quad = [ Point, Point, Point, Point ]; +export type URLMatch = string | RegExp | ((url: URL) => boolean); +export type TimeoutOptions = { timeout?: number }; + +export type StackFrame = { + file: string, + line?: number, + column?: number, + function?: string, +}; diff --git a/src/protocol/channels.ts b/src/protocol/channels.ts index b76e58c5047f9..1c667b976797d 100644 --- a/src/protocol/channels.ts +++ b/src/protocol/channels.ts @@ -24,7 +24,12 @@ export interface Channel extends EventEmitter { } export type Metadata = { - stack?: string, + stack?: { + file: string, + line?: number, + column?: number, + function?: string, + }[], }; export type Point = { diff --git a/src/protocol/protocol.yml b/src/protocol/protocol.yml index da41f56579e9d..c2cf47f2915f9 100644 --- a/src/protocol/protocol.yml +++ b/src/protocol/protocol.yml @@ -17,7 +17,15 @@ Metadata: type: object properties: - stack: string? + stack: + type: array? + items: + type: object + properties: + file: string + line: number? + column: number? + function: string? Point: diff --git a/src/protocol/validator.ts b/src/protocol/validator.ts index 7699dfe123f1c..37e95bf7e7ad2 100644 --- a/src/protocol/validator.ts +++ b/src/protocol/validator.ts @@ -34,7 +34,12 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { }; scheme.Metadata = tObject({ - stack: tOptional(tString), + stack: tOptional(tArray(tObject({ + file: tString, + line: tOptional(tNumber), + column: tOptional(tNumber), + function: tOptional(tString), + }))), }); scheme.Point = tObject({ x: tNumber, diff --git a/src/server/instrumentation.ts b/src/server/instrumentation.ts index a3009aeaeb059..12dd0776c4d07 100644 --- a/src/server/instrumentation.ts +++ b/src/server/instrumentation.ts @@ -15,6 +15,7 @@ */ import { EventEmitter } from 'events'; +import { StackFrame } from '../common/types'; import type { Browser } from './browser'; import type { BrowserContext } from './browserContext'; import type { BrowserType } from './browserType'; @@ -33,7 +34,7 @@ export type CallMetadata = { type: string; method: string; params: any; - stack: string; + stack?: StackFrame[]; }; export class SdkObject extends EventEmitter { @@ -109,6 +110,5 @@ export function internalCallMetadata(): CallMetadata { type: 'Internal', method: '', params: {}, - stack: '' }; } diff --git a/src/server/types.ts b/src/server/types.ts index cd6493a786431..1e2486990a69c 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -15,12 +15,8 @@ * limitations under the License. */ -export type Size = { width: number, height: number }; -export type Point = { x: number, y: number }; -export type Rect = Size & Point; -export type Quad = [ Point, Point, Point, Point ]; - -export type TimeoutOptions = { timeout?: number }; +import { Size, Point, Rect, TimeoutOptions } from '../common/types'; +export { Size, Point, Rect, Quad, URLMatch, TimeoutOptions } from '../common/types'; export type WaitForElementOptions = TimeoutOptions & { state?: 'attached' | 'detached' | 'visible' | 'hidden' }; @@ -58,8 +54,6 @@ export type PageScreencastOptions = { outputFile: string, }; -export type URLMatch = string | RegExp | ((url: URL) => boolean); - export type Credentials = { username: string; password: string; diff --git a/src/trace/traceTypes.ts b/src/trace/traceTypes.ts index 47914bee6e7ad..916c4d826ef4f 100644 --- a/src/trace/traceTypes.ts +++ b/src/trace/traceTypes.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { StackFrame } from '../common/types'; import { NodeSnapshot } from './snapshotterInjected'; export { NodeSnapshot } from './snapshotterInjected'; @@ -81,7 +82,7 @@ export type ActionTraceEvent = { objectType: string, method: string, params: any, - stack?: string, + stack?: StackFrame[], pageId?: string, startTime: number, endTime: number, diff --git a/src/utils/stackTrace.ts b/src/utils/stackTrace.ts index 993a8d7468643..58757dc983cb8 100644 --- a/src/utils/stackTrace.ts +++ b/src/utils/stackTrace.ts @@ -15,49 +15,14 @@ */ import * as path from 'path'; +import { StackFrame } from '../common/types'; +const StackUtils = require('stack-utils'); -// NOTE: update this to point to playwright/lib when moving this file. -const PLAYWRIGHT_LIB_PATH = path.normalize(path.join(__dirname, '..')); +const stackUtils = new StackUtils(); -type ParsedStackFrame = { filePath: string, functionName: string }; - -function parseStackFrame(frame: string): ParsedStackFrame | null { - frame = frame.trim(); - if (!frame.startsWith('at ')) - return null; - frame = frame.substring('at '.length); - if (frame.startsWith('async ')) - frame = frame.substring('async '.length); - let location: string; - let functionName: string; - if (frame.endsWith(')')) { - const from = frame.indexOf('('); - location = frame.substring(from + 1, frame.length - 1); - functionName = frame.substring(0, from).trim(); - } else { - location = frame; - functionName = ''; - } - const match = location.match(/^(?:async )?([^(]*):(\d+):(\d+)$/); - if (!match) - return null; - const filePath = match[1]; - return { filePath, functionName }; -} - -export function getCallerFilePath(ignorePrefix = PLAYWRIGHT_LIB_PATH): string | null { - const error = new Error(); - const stackFrames = (error.stack || '').split('\n').slice(2); - // Find first stackframe that doesn't point to ignorePrefix. - for (const frame of stackFrames) { - const parsed = parseStackFrame(frame); - if (!parsed) - return null; - if (parsed.filePath.startsWith(ignorePrefix)) - continue; - return parsed.filePath; - } - return null; +export function getCallerFilePath(ignorePrefix: string): string | null { + const frame = captureStackTrace().frames.find(f => !f.file.startsWith(ignorePrefix)); + return frame ? frame.file : null; } export function rewriteErrorMessage(e: Error, newMessage: string): Error { @@ -70,3 +35,27 @@ export function rewriteErrorMessage(e: Error, newMessage: string): Error { return e; } +export function captureStackTrace(): { stack: string, frames: StackFrame[] } { + const stack = new Error().stack!; + const frames: StackFrame[] = []; + for (const line of stack.split('\n')) { + const frame = stackUtils.parseLine(line); + if (!frame || !frame.file) + continue; + if (frame.file.startsWith('internal')) + continue; + const fileName = path.resolve(process.cwd(), frame.file); + if (fileName.includes(path.join('playwright', 'lib'))) + continue; + // for tests. + if (fileName.includes(path.join('playwright', 'src'))) + continue; + frames.push({ + file: fileName, + line: frame.line, + column: frame.column, + function: frame.function, + }); + } + return { stack, frames }; +} diff --git a/src/web/traceViewer/ui/sourceTab.tsx b/src/web/traceViewer/ui/sourceTab.tsx index 0f47f90b04def..96277b620449e 100644 --- a/src/web/traceViewer/ui/sourceTab.tsx +++ b/src/web/traceViewer/ui/sourceTab.tsx @@ -20,14 +20,10 @@ import { useAsyncMemo } from './helpers'; import './sourceTab.css'; import '../../../third_party/highlightjs/highlightjs/tomorrow.css'; import * as highlightjs from '../../../third_party/highlightjs/highlightjs'; +import { StackFrame } from '../../../common/types'; type StackInfo = string | { - frames: { - filePath: string, - fileName: string, - lineNumber: number, - functionName: string, - }[]; + frames: StackFrame[]; fileContent: Map; }; @@ -50,41 +46,11 @@ export const SourceTab: React.FunctionComponent<{ const { action } = actionEntry; if (!action.stack) return ''; - let frames = action.stack.split('\n').slice(1); - frames = frames.filter(frame => !frame.includes('playwright/lib/') && !frame.includes('playwright/src/')); - const info: StackInfo = { - frames: [], + const frames = action.stack; + return { + frames, fileContent: new Map(), }; - for (const frame of frames) { - let filePath: string; - let lineNumber: number; - let functionName: string; - const match1 = frame.match(/at ([^(]+)\(([^:]+):(\d+):\d+\)/); - const match2 = frame.match(/at ([^:^(]+):(\d+):\d+/); - if (match1) { - functionName = match1[1]; - filePath = match1[2]; - lineNumber = parseInt(match1[3], 10); - } else if (match2) { - functionName = ''; - filePath = match2[1]; - lineNumber = parseInt(match2[2], 10); - } else { - continue; - } - const pathSep = navigator.platform.includes('Win') ? '\\' : '/'; - const fileName = filePath.substring(filePath.lastIndexOf(pathSep) + 1); - info.frames.push({ - filePath, - fileName, - lineNumber, - functionName: functionName || '(anonymous)', - }); - } - if (!info.frames.length) - return action.stack; - return info; }, [actionEntry]); const content = useAsyncMemo(async () => { @@ -92,7 +58,7 @@ export const SourceTab: React.FunctionComponent<{ if (typeof stackInfo === 'string') { value = stackInfo; } else { - const filePath = stackInfo.frames[selectedFrame].filePath; + const filePath = stackInfo.frames[selectedFrame].file; if (!stackInfo.fileContent.has(filePath)) stackInfo.fileContent.set(filePath, await fetch(`/file?${filePath}`).then(response => response.text()).catch(e => ``)); value = stackInfo.fileContent.get(filePath)!; @@ -107,7 +73,7 @@ export const SourceTab: React.FunctionComponent<{ return result; }, [stackInfo, selectedFrame], []); - const targetLine = typeof stackInfo === 'string' ? -1 : stackInfo.frames[selectedFrame].lineNumber; + const targetLine = typeof stackInfo === 'string' ? -1 : stackInfo.frames[selectedFrame].line; const targetLineRef = React.createRef(); React.useLayoutEffect(() => { @@ -142,13 +108,13 @@ export const SourceTab: React.FunctionComponent<{ }} > - {frame.functionName} + {frame.function || '(anonymous)'} - {frame.fileName} + {frame.file} - {':' + frame.lineNumber} + {':' + frame.line} ; }) diff --git a/utils/check_deps.js b/utils/check_deps.js index d405c91dff329..63cba0215ad4c 100644 --- a/utils/check_deps.js +++ b/utils/check_deps.js @@ -112,12 +112,13 @@ DEPS['src/protocol/'] = ['src/utils/']; DEPS['src/install/'] = ['src/utils/']; // Client depends on chromium protocol for types. -DEPS['src/client/'] = ['src/utils/', 'src/protocol/', 'src/server/chromium/protocol.ts']; +DEPS['src/client/'] = ['src/common/', 'src/utils/', 'src/protocol/', 'src/server/chromium/protocol.ts']; DEPS['src/dispatchers/'] = ['src/utils/', 'src/protocol/', 'src/server/**']; // Generic dependencies for server-side code. DEPS['src/server/'] = [ + 'src/common/', 'src/utils/', 'src/generated/', // Can depend on files directly in the server directory. @@ -142,11 +143,10 @@ DEPS['src/server/playwright.ts'] = [...DEPS['src/server/'], 'src/trace/', 'src/s DEPS['src/cli/driver.ts'] = DEPS['src/inprocess.ts'] = DEPS['src/browserServerImpl.ts'] = ['src/**']; // Tracing is a client/server plugin, nothing should depend on it. -DEPS['src/trace/'] = ['src/utils/', 'src/client/**', 'src/server/**']; -DEPS['src/web/'] = []; -DEPS['src/web/recorder/'] = ['src/web/', 'src/web/components/']; -DEPS['src/web/traceViewer/'] = ['src/web/', 'src/cli/traceViewer/']; -DEPS['src/web/traceViewer/ui/'] = ['src/web/traceViewer/', 'src/web/', 'src/cli/traceViewer/', 'src/trace/']; +DEPS['src/trace/'] = ['src/common/', 'src/utils/', 'src/client/**', 'src/server/**']; +DEPS['src/web/recorder/'] = ['src/common/', 'src/web/', 'src/web/components/']; +DEPS['src/web/traceViewer/'] = ['src/common/', 'src/web/', 'src/cli/traceViewer/']; +DEPS['src/web/traceViewer/ui/'] = ['src/common/', 'src/web/traceViewer/', 'src/web/', 'src/cli/traceViewer/', 'src/trace/']; // The service is a cross-cutting feature, and so it depends on a bunch of things. DEPS['src/remote/'] = ['src/client/', 'src/debug/', 'src/dispatchers/', 'src/server/', 'src/server/supplements/', 'src/server/electron/', 'src/trace/']; DEPS['src/service.ts'] = ['src/remote/']; @@ -154,7 +154,8 @@ DEPS['src/service.ts'] = ['src/remote/']; // CLI should only use client-side features. DEPS['src/cli/'] = ['src/cli/**', 'src/client/**', 'src/install/**', 'src/generated/', 'src/server/injected/', 'src/debug/injected/', 'src/trace/**', 'src/utils/**']; -DEPS['src/server/supplements/recorder/recorderApp.ts'] = ['src/server/', 'src/server/chromium/'] +DEPS['src/server/supplements/recorder/recorderApp.ts'] = ['src/server/', 'src/server/chromium/']; +DEPS['src/utils/'] = ['src/common/']; checkDeps().catch(e => { console.error(e && e.stack ? e.stack : e);