chore: pass parsed stack in metainfo (#5407)

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=="
         }
       }
     },

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": {

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<channels.Channel, {}> {
   constructor(connection: Connection) {
@@ -71,14 +72,12 @@ export class Connection {
   }
 
   async sendMessageToServer(guid: string, method: string, params: any): Promise<any> {
-    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) {

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 };
 

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,
+};

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 = {

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:

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,

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: ''
   };
 }

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;

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,

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 };
+}

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<string, string>;
 };
 
@@ -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<string[]>(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 => `<Unable to read "${filePath}">`));
       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<HTMLDivElement>();
   React.useLayoutEffect(() => {
@@ -142,13 +108,13 @@ export const SourceTab: React.FunctionComponent<{
           }}
         >
           <span className='source-stack-frame-function'>
-            {frame.functionName}
+            {frame.function || '(anonymous)'}
           </span>
           <span className='source-stack-frame-location'>
-            {frame.fileName}
+            {frame.file}
           </span>
           <span className='source-stack-frame-line'>
-            {':' + frame.lineNumber}
+            {':' + frame.line}
           </span>
         </div>;
       })

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);