feat(trace-viewer): add nicer params rendering (#7448)

src/common/types.ts

@@ -20,10 +20,3 @@ 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/dispatchers/dispatcher.ts

@@ -22,7 +22,6 @@ import { assert, debugAssert, isUnderTest, monotonicTime } from '../utils/utils'
 import { tOptional } from '../protocol/validatorPrimitives';
 import { kBrowserOrContextClosedError } from '../utils/errors';
 import { CallMetadata, SdkObject } from '../server/instrumentation';
-import { StackFrame } from '../common/types';
 import { rewriteErrorMessage } from '../utils/stackTrace';
 
 export const dispatcherSymbol = Symbol('dispatcher');
@@ -133,7 +132,7 @@ export class DispatcherConnection {
   private _rootDispatcher: Root;
   onmessage = (message: object) => {};
   private _validateParams: (type: string, method: string, params: any) => any;
-  private _validateMetadata: (metadata: any) => { stack?: StackFrame[] };
+  private _validateMetadata: (metadata: any) => { stack?: channels.StackFrame[] };
   private _waitOperations = new Map<string, CallMetadata>();
 
   sendMessageToClient(guid: string, type: string, method: string, params: any, sdkObject?: SdkObject) {

src/protocol/callMetadata.ts

@@ -0,0 +1,38 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * 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.
+ */
+
+import { Point, StackFrame, SerializedError } from './channels';
+
+export type CallMetadata = {
+  id: string;
+  startTime: number;
+  endTime: number;
+  pauseStartTime?: number;
+  pauseEndTime?: number;
+  type: string;
+  method: string;
+  params: any;
+  apiName?: string;
+  stack?: StackFrame[];
+  log: string[];
+  snapshots: { title: string, snapshotName: string }[];
+  error?: SerializedError;
+  result?: any;
+  point?: Point;
+  objectId?: string;
+  pageId?: string;
+  frameId?: string;
+};

src/server/instrumentation.ts

@@ -15,8 +15,6 @@
  */
 
 import { EventEmitter } from 'events';
-import { Point, StackFrame } from '../common/types';
-import { SerializedError } from '../protocol/channels';
 import { createGuid } from '../utils/utils';
 import type { Browser } from './browser';
 import type { BrowserContext } from './browserContext';
@@ -34,26 +32,8 @@ export type Attribution = {
   frame?: Frame;
 };
 
-export type CallMetadata = {
+import { CallMetadata } from '../protocol/callMetadata';
-  id: string;
+export { CallMetadata } from '../protocol/callMetadata';
-  startTime: number;
-  endTime: number;
-  pauseStartTime?: number;
-  pauseEndTime?: number;
-  type: string;
-  method: string;
-  params: any;
-  apiName?: string;
-  stack?: StackFrame[];
-  log: string[];
-  snapshots: { title: string, snapshotName: string }[];
-  error?: SerializedError;
-  result?: any;
-  point?: Point;
-  objectId?: string;
-  pageId?: string;
-  frameId?: string;
-};
 
 export class SdkObject extends EventEmitter {
   guid: string;

src/utils/stackTrace.ts

@@ -15,7 +15,7 @@
  */
 
 import path from 'path';
-import { StackFrame } from '../common/types';
+import { StackFrame } from '../protocol/channels';
 import StackUtils from 'stack-utils';
 import { isUnderTest } from './utils';
 

src/web/traceViewer/ui/callTab.tsx

@@ -16,7 +16,9 @@
 
 import * as React from 'react';
 import './callTab.css';
-import { ActionTraceEvent } from '../../../server/trace/common/traceEvents';
+import type { ActionTraceEvent } from '../../../server/trace/common/traceEvents';
+import { CallMetadata } from '../../../protocol/callMetadata';
+import { parseSerializedValue } from '../../../protocol/serializers';
 
 export const CallTab: React.FunctionComponent<{
   action: ActionTraceEvent | undefined,
@@ -26,6 +28,7 @@ export const CallTab: React.FunctionComponent<{
   const logs = action.metadata.log;
   const error = action.metadata.error?.error?.message;
   const params = { ...action.metadata.params };
+  // Strip down the waitForEventInfo data, we never need it.
   delete params.info;
   const paramKeys = Object.keys(params);
   return <div className='call-tab'>
@@ -36,14 +39,12 @@ export const CallTab: React.FunctionComponent<{
       <div className='call-line'>{action.metadata.apiName}</div>
       { !!paramKeys.length && <div className='call-section'>Parameters</div> }
       {
-        !!paramKeys.length && paramKeys.map(name =>
+        !!paramKeys.length && paramKeys.map(name => renderLine(action.metadata, name, params[name]))
-          <div className='call-line'>{name}: <span className={typeof params[name]}>{renderValue(params[name])}</span></div>
-        )
       }
       { !!action.metadata.result && <div className='call-section'>Return value</div> }
       {
         !!action.metadata.result && Object.keys(action.metadata.result).map(name =>
-          <div className='call-line'>{name}: <span className={typeof action.metadata.result[name]}>{renderValue(action.metadata.result[name])}</span></div>
+          renderLine(action.metadata, name, action.metadata.result[name])
         )
       }
       <div className='call-section'>Log</div>
@@ -57,10 +58,31 @@ export const CallTab: React.FunctionComponent<{
     </div>;
 };
 
-function renderValue(value: any) {
+function renderLine(metadata: CallMetadata, name: string, value: any) {
+  const { title, type } = toString(metadata, name, value);
+  let text = trimRight(title.replace(/\n/g, '↵'), 80);
+  if (type === 'string')
+    text = `"${text}"`;
+  return <div className='call-line'>{name}: <span className={type} title={title}>{text}</span></div>
+}
+
+function toString(metadata: CallMetadata, name: string, value: any): { title: string, type: string } {
+  if (metadata.method.includes('eval')) {
+    if (name === 'arg')
+      value = parseSerializedValue(value.value, new Array(10).fill({ handle: '<handle>' }));
+    if (name === 'value')
+      value = parseSerializedValue(value, new Array(10).fill({ handle: '<handle>' }));
+  }
   const type = typeof value;
   if (type !== 'object')
-    return String(value);
+    return { title: String(value), type };
   if (value.guid)
-    return '<handle>';
+    return { title: '<handle>', type: 'handle' };
+  return { title: JSON.stringify(value), type: 'object' };
+}
+
+function trimRight(text: string, max: number): string {
+  if (text.length > max)
+    return text.substr(0, max) + '\u2026';
+  return text;
 }

src/web/traceViewer/ui/sourceTab.tsx

@@ -18,11 +18,11 @@ import * as React from 'react';
 import { useAsyncMemo } from './helpers';
 import './sourceTab.css';
 import '../../../third_party/highlightjs/highlightjs/tomorrow.css';
-import { StackFrame } from '../../../common/types';
 import { Source as SourceView } from '../../components/source';
 import { StackTraceView } from './stackTrace';
 import { SplitView } from '../../components/splitView';
 import { ActionTraceEvent } from '../../../server/trace/common/traceEvents';
+import { StackFrame } from '../../../protocol/channels';
 
 type StackInfo = string | {
   frames: StackFrame[];

tests/trace-viewer/trace-viewer.spec.ts

@@ -42,7 +42,8 @@ class TraceViewerPage {
     await this.page.click(`.action-title:has-text("${title}")`);
   }
 
-  async logLines() {
+
+  async callLines() {
     await this.page.waitForSelector('.call-line:visible');
     return await this.page.$$eval('.call-line:visible', ee => ee.map(e => e.textContent));
   }
@@ -97,12 +98,13 @@ test.beforeAll(async ({ browser, browserName }, workerInfo) => {
   const page = await context.newPage();
   await page.goto('data:text/html,<html>Hello world</html>');
   await page.setContent('<button>Click</button>');
-  await page.evaluate(() => {
+  await page.evaluate(({ a }) => {
     console.log('Info');
     console.warn('Warning');
     console.error('Error');
     setTimeout(() => { throw new Error('Unhandled exception'); }, 0);
-  });
+    return 'return ' + a;
+  }, { a: 'paramA', b: 4 });
   await page.click('"Click"');
   await Promise.all([
     page.waitForNavigation(),
@@ -133,7 +135,7 @@ test('should open simple trace viewer', async ({ showTraceViewer }) => {
 test('should contain action info', async ({ showTraceViewer }) => {
   const traceViewer = await showTraceViewer(traceFile);
   await traceViewer.selectAction('page.click');
-  const logLines = await traceViewer.logLines();
+  const logLines = await traceViewer.callLines();
   expect(logLines.length).toBeGreaterThan(10);
   expect(logLines).toContain('attempting click action');
   expect(logLines).toContain('  click action done');
@@ -168,3 +170,15 @@ test('should open console errors on click', async ({ showTraceViewer, browserNam
   await (await traceViewer.actionIcons('page.evaluate')).click();
   expect(await traceViewer.page.waitForSelector('.console-tab'));
 });
+
+test('should show params and return value', async ({ showTraceViewer, browserName }) => {
+  const traceViewer = await showTraceViewer(traceFile);
+  expect(await traceViewer.selectAction('page.evaluate'));
+  expect(await traceViewer.callLines()).toEqual([
+    'page.evaluate',
+    'expression: "({↵    a↵  }) => {↵    console.log(\'Info\');↵    console.warn(\'Warning\');↵    con…"',
+    'isFunction: true',
+    'arg: {"a":"paramA","b":4}',
+    'value: "return paramA"'
+  ]);
+});

utils/check_deps.js

@@ -147,7 +147,7 @@ DEPS['src/cli/driver.ts'] = DEPS['src/inprocess.ts'] = DEPS['src/browserServerIm
 // Tracing is a client/server plugin, nothing should depend on it.
 DEPS['src/web/recorder/'] = ['src/common/', 'src/web/', 'src/web/components/', 'src/server/supplements/recorder/recorderTypes.ts'];
 DEPS['src/web/traceViewer/'] = ['src/common/', 'src/web/'];
-DEPS['src/web/traceViewer/ui/'] = ['src/common/', 'src/web/traceViewer/', 'src/web/', 'src/server/trace/viewer/', 'src/server/trace/', 'src/server/trace/common/', 'src/server/snapshot/snapshotTypes.ts', 'src/protocol/channels.ts'];
+DEPS['src/web/traceViewer/ui/'] = ['src/common/', 'src/protocol/', 'src/web/traceViewer/', 'src/web/', 'src/server/trace/viewer/', 'src/server/trace/', 'src/server/trace/common/', 'src/server/snapshot/snapshotTypes.ts', 'src/protocol/channels.ts'];
 // 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/server/trace/'];
 
@@ -156,7 +156,7 @@ DEPS['src/cli/'] = ['src/cli/**', 'src/client/**', 'src/install/**', 'src/genera
 
 DEPS['src/server/supplements/recorder/recorderApp.ts'] = ['src/common/', 'src/utils/', 'src/server/', 'src/server/chromium/'];
 DEPS['src/server/supplements/recorderSupplement.ts'] = ['src/server/snapshot/', ...DEPS['src/server/']];
-DEPS['src/utils/'] = ['src/common/'];
+DEPS['src/utils/'] = ['src/common/', 'src/protocol/'];
 
 // Trace viewer
 DEPS['src/server/trace/common/'] = ['src/server/snapshot/', ...DEPS['src/server/']];