From 99d7d196c5a347eb1d7962ed16d38504ccf09012 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 2 Jul 2021 14:33:38 -0700 Subject: [PATCH] feat(trace-viewer): render call info w/ params, result (#7438) --- src/dispatchers/dispatcher.ts | 15 +++-- src/server/instrumentation.ts | 4 +- .../supplements/recorder/recorderTypes.ts | 3 +- src/server/trace/common/traceEvents.ts | 1 + src/server/trace/recorder/tracing.ts | 7 +- src/server/trace/viewer/traceModel.ts | 34 ++++++++-- src/server/trace/viewer/traceViewer.ts | 13 ++-- src/web/recorder/callLog.tsx | 5 +- .../ui/{logsTab.css => callTab.css} | 42 +++++++++--- src/web/traceViewer/ui/callTab.tsx | 66 +++++++++++++++++++ src/web/traceViewer/ui/logsTab.tsx | 37 ----------- src/web/traceViewer/ui/modelUtil.ts | 9 ++- src/web/traceViewer/ui/tabbedPane.css | 9 ++- src/web/traceViewer/ui/tabbedPane.tsx | 3 + src/web/traceViewer/ui/workbench.tsx | 14 ++-- tests/trace-viewer/trace-viewer.spec.ts | 6 +- tests/tracing.spec.ts | 2 +- utils/check_deps.js | 3 +- 18 files changed, 186 insertions(+), 87 deletions(-) rename src/web/traceViewer/ui/{logsTab.css => callTab.css} (61%) create mode 100644 src/web/traceViewer/ui/callTab.tsx delete mode 100644 src/web/traceViewer/ui/logsTab.tsx diff --git a/src/dispatchers/dispatcher.ts b/src/dispatchers/dispatcher.ts index 35d88313c6..94cf9e5d48 100644 --- a/src/dispatchers/dispatcher.ts +++ b/src/dispatchers/dispatcher.ts @@ -246,7 +246,7 @@ export class DispatcherConnection { } case 'after': { const originalMetadata = this._waitOperations.get(info.waitId)!; originalMetadata.endTime = monotonicTime(); - originalMetadata.error = info.error; + originalMetadata.error = info.error ? { error: { name: 'Error', message: info.error } } : undefined; this._waitOperations.delete(info.waitId); await sdkObject.instrumentation.onAfterCall(sdkObject, originalMetadata); return; @@ -255,14 +255,15 @@ export class DispatcherConnection { } - let result: any; let error: any; await sdkObject?.instrumentation.onBeforeCall(sdkObject, callMetadata); try { - result = await (dispatcher as any)[method](validParams, callMetadata); + const result = await (dispatcher as any)[method](validParams, callMetadata); + callMetadata.result = this._replaceDispatchersWithGuids(result); } catch (e) { // Dispatching error - callMetadata.error = e.message; + // We want original, unmodified error in metadata. + callMetadata.error = serializeError(e); if (callMetadata.log.length) rewriteErrorMessage(e, e.message + formatLogRecording(callMetadata.log)); error = serializeError(e); @@ -271,10 +272,10 @@ export class DispatcherConnection { await sdkObject?.instrumentation.onAfterCall(sdkObject, callMetadata); } - if (error) - this.onmessage({ id, error }); + if (callMetadata.error) + this.onmessage({ id, error: error }); else - this.onmessage({ id, result: this._replaceDispatchersWithGuids(result) }); + this.onmessage({ id, result: callMetadata.result }); } private _replaceDispatchersWithGuids(payload: any): any { diff --git a/src/server/instrumentation.ts b/src/server/instrumentation.ts index 63c77f9670..e0d23b7ff2 100644 --- a/src/server/instrumentation.ts +++ b/src/server/instrumentation.ts @@ -16,6 +16,7 @@ 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'; @@ -46,7 +47,8 @@ export type CallMetadata = { stack?: StackFrame[]; log: string[]; snapshots: { title: string, snapshotName: string }[]; - error?: string; + error?: SerializedError; + result?: any; point?: Point; objectId?: string; pageId?: string; diff --git a/src/server/supplements/recorder/recorderTypes.ts b/src/server/supplements/recorder/recorderTypes.ts index 1dcd9fedc2..6ee15e65b8 100644 --- a/src/server/supplements/recorder/recorderTypes.ts +++ b/src/server/supplements/recorder/recorderTypes.ts @@ -15,6 +15,7 @@ */ import { Point } from '../../../common/types'; +import { SerializedError } from '../../../protocol/channels'; export type Mode = 'inspecting' | 'recording' | 'none'; @@ -36,7 +37,7 @@ export type CallLog = { title: string; messages: string[]; status: CallLogStatus; - error?: string; + error?: SerializedError; reveal?: boolean; duration?: number; params: { diff --git a/src/server/trace/common/traceEvents.ts b/src/server/trace/common/traceEvents.ts index bb7ee284e0..8bb3a21365 100644 --- a/src/server/trace/common/traceEvents.ts +++ b/src/server/trace/common/traceEvents.ts @@ -19,6 +19,7 @@ import { FrameSnapshot, ResourceSnapshot } from '../../snapshot/snapshotTypes'; import { BrowserContextOptions } from '../../types'; export type ContextCreatedTraceEvent = { + version: number, type: 'context-options', browserName: string, options: BrowserContextOptions diff --git a/src/server/trace/recorder/tracing.ts b/src/server/trace/recorder/tracing.ts index 3fd7b7ea48..3a1879cef2 100644 --- a/src/server/trace/recorder/tracing.ts +++ b/src/server/trace/recorder/tracing.ts @@ -35,6 +35,8 @@ export type TracerOptions = { screenshots?: boolean; }; +export const VERSION = 1; + export class Tracing implements InstrumentationListener { private _appendEventChain = Promise.resolve(); private _snapshotter: TraceSnapshotter; @@ -63,6 +65,7 @@ export class Tracing implements InstrumentationListener { this._appendEventChain = mkdirIfNeeded(this._traceFile); const event: trace.ContextCreatedTraceEvent = { + version: VERSION, type: 'context-options', browserName: this._context._browser.options.name, options: this._context._options @@ -90,7 +93,7 @@ export class Tracing implements InstrumentationListener { for (const { sdkObject, metadata, beforeSnapshot, actionSnapshot, afterSnapshot } of this._pendingCalls.values()) { await Promise.all([beforeSnapshot, actionSnapshot, afterSnapshot]); if (!afterSnapshot) - metadata.error = 'Action was interrupted'; + metadata.error = { error: { name: 'Error', message: 'Action was interrupted' } }; await this.onAfterCall(sdkObject, metadata); } for (const page of this._context.pages()) @@ -227,6 +230,6 @@ export class Tracing implements InstrumentationListener { } } -function shouldCaptureSnapshot(metadata: CallMetadata): boolean { +export function shouldCaptureSnapshot(metadata: CallMetadata): boolean { return commandsWithTracingSnapshots.has(metadata.type + '.' + metadata.method); } diff --git a/src/server/trace/viewer/traceModel.ts b/src/server/trace/viewer/traceModel.ts index 6407764c76..46c9397a9d 100644 --- a/src/server/trace/viewer/traceModel.ts +++ b/src/server/trace/viewer/traceModel.ts @@ -18,8 +18,9 @@ import fs from 'fs'; import path from 'path'; import * as trace from '../common/traceEvents'; import { ContextResources, ResourceSnapshot } from '../../snapshot/snapshotTypes'; -import { BaseSnapshotStorage, SnapshotStorage } from '../../snapshot/snapshotStorage'; +import { BaseSnapshotStorage } from '../../snapshot/snapshotStorage'; import { BrowserContextOptions } from '../../types'; +import { shouldCaptureSnapshot, VERSION } from '../recorder/tracing'; export * as trace from '../common/traceEvents'; export class TraceModel { @@ -27,6 +28,7 @@ export class TraceModel { pageEntries = new Map(); contextResources = new Map(); private _snapshotStorage: PersistentSnapshotStorage; + private _version: number | undefined; constructor(snapshotStorage: PersistentSnapshotStorage) { this._snapshotStorage = snapshotStorage; @@ -40,12 +42,10 @@ export class TraceModel { }; } - appendEvents(events: trace.TraceEvent[], snapshotStorage: SnapshotStorage) { - for (const event of events) - this.appendEvent(event); + build() { for (const page of this.contextEntry!.pages) page.actions.sort((a1, a2) => a1.metadata.startTime - a2.metadata.startTime); - this.contextEntry!.resources = snapshotStorage.resources(); + this.contextEntry!.resources = this._snapshotStorage.resources(); } private _pageEntry(pageId: string): PageEntry { @@ -63,9 +63,11 @@ export class TraceModel { return pageEntry; } - appendEvent(event: trace.TraceEvent) { + appendEvent(line: string) { + const event = this._modernize(JSON.parse(line)); switch (event.type) { case 'context-options': { + this._version = event.version || 0; this.contextEntry.browserName = event.browserName; this.contextEntry.options = event.options; break; @@ -76,7 +78,7 @@ export class TraceModel { } case 'action': { const metadata = event.metadata; - const include = typeof event.hasSnapshot !== 'boolean' || event.hasSnapshot; + const include = event.hasSnapshot; if (include && metadata.pageId) this._pageEntry(metadata.pageId).actions.push(event); break; @@ -103,6 +105,24 @@ export class TraceModel { this.contextEntry!.endTime = Math.max(this.contextEntry!.endTime, event.metadata.endTime); } } + + private _modernize(event: any): trace.TraceEvent { + if (this._version === undefined) + return event; + for (let version = this._version; version < VERSION; ++version) + event = (this as any)[`_modernize_${version}_to_${version + 1}`].call(this, event); + return event; + } + + _modernize_0_to_1(event: any): any { + if (event.type === 'action') { + if (typeof event.metadata.error === 'string') + event.metadata.error = { error: { name: 'Error', message: event.metadata.error } }; + if (event.metadata && typeof event.hasSnapshot !== 'boolean') + event.hasSnapshot = shouldCaptureSnapshot(event.metadata); + } + return event; + } } export type ContextEntry = { diff --git a/src/server/trace/viewer/traceViewer.ts b/src/server/trace/viewer/traceViewer.ts index 25f9e61e93..c12119f1d1 100644 --- a/src/server/trace/viewer/traceViewer.ts +++ b/src/server/trace/viewer/traceViewer.ts @@ -16,12 +16,12 @@ import extract from 'extract-zip'; import fs from 'fs'; +import readline from 'readline'; import os from 'os'; import path from 'path'; import rimraf from 'rimraf'; import { createPlaywright } from '../../playwright'; import { PersistentSnapshotStorage, TraceModel } from './traceModel'; -import { TraceEvent } from '../common/traceEvents'; import { ServerRouteHandler, HttpServer } from '../../../utils/httpServer'; import { SnapshotServer } from '../../snapshot/snapshotServer'; import * as consoleApiSource from '../../../generated/consoleApiSource'; @@ -80,10 +80,15 @@ export class TraceViewer { response.statusCode = 200; response.setHeader('Content-Type', 'application/json'); (async () => { - const traceContent = await fs.promises.readFile(tracePrefix + '.trace', 'utf8'); - const events = traceContent.split('\n').map(line => line.trim()).filter(line => !!line).map(line => JSON.parse(line)) as TraceEvent[]; + const fileStream = fs.createReadStream(tracePrefix + '.trace', 'utf8'); + const rl = readline.createInterface({ + input: fileStream, + crlfDelay: Infinity + }); const model = new TraceModel(snapshotStorage); - model.appendEvents(events, snapshotStorage); + for await (const line of rl as any) + model.appendEvent(line); + model.build(); response.end(JSON.stringify(model.contextEntry)); })().catch(e => console.error(e)); return true; diff --git a/src/web/recorder/callLog.tsx b/src/web/recorder/callLog.tsx index c1286c6d12..228a3edc0d 100644 --- a/src/web/recorder/callLog.tsx +++ b/src/web/recorder/callLog.tsx @@ -32,7 +32,6 @@ export const CallLogView: React.FC = ({ if (log.find(callLog => callLog.reveal)) messagesEndRef.current?.scrollIntoView({ block: 'center', inline: 'nearest' }); }, [messagesEndRef]); - return
{log.map(callLog => { const expandOverride = expandOverrides.get(callLog.id); @@ -55,9 +54,7 @@ export const CallLogView: React.FC = ({ { message.trim() }
; })} - { callLog.error ? : undefined } + { !!callLog.error && } })}
diff --git a/src/web/traceViewer/ui/logsTab.css b/src/web/traceViewer/ui/callTab.css similarity index 61% rename from src/web/traceViewer/ui/logsTab.css rename to src/web/traceViewer/ui/callTab.css index 67bf64f505..2912238f3c 100644 --- a/src/web/traceViewer/ui/logsTab.css +++ b/src/web/traceViewer/ui/callTab.css @@ -14,30 +14,52 @@ limitations under the License. */ -.logs-tab { +.call-tab { flex: auto; - line-height: 20px; + line-height: 24px; white-space: pre; overflow: auto; - padding-top: 3px; + padding: 6px 0; user-select: text; } -.logs-error { +.call-error { border-bottom: 1px solid var(--background); padding: 3px 0 3px 12px; } -.logs-error .codicon { +.call-error .codicon { color: red; position: relative; top: 2px; margin-right: 2px; } -.log-line { - flex: none; - padding: 3px 0 3px 12px; - display: flex; - align-items: center; +.call-section { + padding-left: 6px; + border-top: 1px solid #ddd; + font-weight: bold; + text-transform: uppercase; + font-size: 10px; + border-bottom: 1px solid #ddd; + background-color: #efefef; + line-height: 18px; +} + +.call-line { + padding: 0 0 2px 6px; + flex: none; + align-items: center; + text-overflow: ellipsis; + overflow: hidden; +} + +.call-line .string { + color: var(--orange); +} + +.call-line .number, +.call-line .boolean, +.call-line .object { + color: var(--blue); } diff --git a/src/web/traceViewer/ui/callTab.tsx b/src/web/traceViewer/ui/callTab.tsx new file mode 100644 index 0000000000..c3124f0808 --- /dev/null +++ b/src/web/traceViewer/ui/callTab.tsx @@ -0,0 +1,66 @@ +/** + * 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 * as React from 'react'; +import './callTab.css'; +import { ActionTraceEvent } from '../../../server/trace/common/traceEvents'; + +export const CallTab: React.FunctionComponent<{ + action: ActionTraceEvent | undefined, +}> = ({ action }) => { + if (!action) + return null; + const logs = action.metadata.log; + const error = action.metadata.error?.error?.message; + const params = { ...action.metadata.params }; + delete params.info; + const paramKeys = Object.keys(params); + return
+ ; +}; + +function renderValue(value: any) { + const type = typeof value; + if (type !== 'object') + return String(value); + if (value.guid) + return ''; +} diff --git a/src/web/traceViewer/ui/logsTab.tsx b/src/web/traceViewer/ui/logsTab.tsx deleted file mode 100644 index 91b13464c3..0000000000 --- a/src/web/traceViewer/ui/logsTab.tsx +++ /dev/null @@ -1,37 +0,0 @@ -/** - * 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 * as React from 'react'; -import './logsTab.css'; -import { ActionTraceEvent } from '../../../server/trace/common/traceEvents'; - -export const LogsTab: React.FunctionComponent<{ - action: ActionTraceEvent | undefined, -}> = ({ action }) => { - const logs = action?.metadata.log || []; - const error = action?.metadata.error; - return
{ - ; -}; diff --git a/src/web/traceViewer/ui/modelUtil.ts b/src/web/traceViewer/ui/modelUtil.ts index aa6ed86f9f..2153985199 100644 --- a/src/web/traceViewer/ui/modelUtil.ts +++ b/src/web/traceViewer/ui/modelUtil.ts @@ -22,6 +22,7 @@ const contextSymbol = Symbol('context'); const pageSymbol = Symbol('context'); const nextSymbol = Symbol('next'); const eventsSymbol = Symbol('events'); +const resourcesSymbol = Symbol('resources'); export function indexModel(context: ContextEntry) { for (const page of context.pages) { @@ -84,8 +85,14 @@ export function eventsForAction(action: ActionTraceEvent): ActionTraceEvent[] { } export function resourcesForAction(action: ActionTraceEvent): ResourceSnapshot[] { + let result: ResourceSnapshot[] = (action as any)[resourcesSymbol]; + if (result) + return result; + const nextAction = next(action); - return context(action).resources.filter(resource => { + result = context(action).resources.filter(resource => { return resource.timestamp > action.metadata.startTime && (!nextAction || resource.timestamp < nextAction.metadata.startTime); }); + (action as any)[resourcesSymbol] = result; + return result; } diff --git a/src/web/traceViewer/ui/tabbedPane.css b/src/web/traceViewer/ui/tabbedPane.css index bd8f5edd10..6611b5015b 100644 --- a/src/web/traceViewer/ui/tabbedPane.css +++ b/src/web/traceViewer/ui/tabbedPane.css @@ -44,7 +44,7 @@ } .tab-element { - padding: 2px 12px 0 12px; + padding: 2px 10px 0 10px; margin-right: 4px; cursor: pointer; display: flex; @@ -65,6 +65,13 @@ display: inline-block; } +.tab-count { + font-size: 10px; + display: flex; + align-self: flex-start; + width: 0px; +} + .tab-element.selected { border-bottom-color: #666; } diff --git a/src/web/traceViewer/ui/tabbedPane.tsx b/src/web/traceViewer/ui/tabbedPane.tsx index dd3bdf29d8..1e1e66efda 100644 --- a/src/web/traceViewer/ui/tabbedPane.tsx +++ b/src/web/traceViewer/ui/tabbedPane.tsx @@ -16,10 +16,12 @@ import './tabbedPane.css'; import * as React from 'react'; +import { count } from 'console'; export interface TabbedPaneTab { id: string; title: string; + count: number; render: () => React.ReactElement; } @@ -37,6 +39,7 @@ export const TabbedPane: React.FunctionComponent<{ onClick={() => setSelectedTab(tab.id)} key={tab.id}>
{tab.title}
+
{tab.count || ''}
}) }
diff --git a/src/web/traceViewer/ui/workbench.tsx b/src/web/traceViewer/ui/workbench.tsx index d127dadbdd..3980427b39 100644 --- a/src/web/traceViewer/ui/workbench.tsx +++ b/src/web/traceViewer/ui/workbench.tsx @@ -25,7 +25,7 @@ import { ContextSelector } from './contextSelector'; import { NetworkTab } from './networkTab'; import { SourceTab } from './sourceTab'; import { SnapshotTab } from './snapshotTab'; -import { LogsTab } from './logsTab'; +import { CallTab } from './callTab'; import { SplitView } from '../../components/splitView'; import { useAsyncMemo } from './helpers'; import { ConsoleTab } from './consoleTab'; @@ -59,7 +59,9 @@ export const Workbench: React.FunctionComponent<{ // Leave some nice free space on the right hand side. boundaries.maximum += (boundaries.maximum - boundaries.minimum) / 20; - + const { errors, warnings } = selectedAction ? modelUtil.stats(selectedAction) : { errors: 0, warnings: 0 }; + const consoleCount = errors + warnings; + const networkCount = selectedAction ? modelUtil.resourcesForAction(selectedAction).length : 0; return
@@ -89,10 +91,10 @@ export const Workbench: React.FunctionComponent<{ }, - { id: 'console', title: 'Console', render: () => }, - { id: 'source', title: 'Source', render: () => }, - { id: 'network', title: 'Network', render: () => }, + { id: 'logs', title: 'Call', count: 0, render: () => }, + { id: 'console', title: 'Console', count: consoleCount, render: () => }, + { id: 'network', title: 'Network', count: networkCount, render: () => }, + { id: 'source', title: 'Source', count: 0, render: () => }, ]} selectedTab={selectedTab} setSelectedTab={setSelectedTab}/> ee.map(e => e.textContent)); + await this.page.waitForSelector('.call-line:visible'); + return await this.page.$$eval('.call-line:visible', ee => ee.map(e => e.textContent)); } async eventBars() { @@ -130,7 +130,7 @@ test('should open simple trace viewer', async ({ showTraceViewer }) => { ]); }); -test('should contain action log', async ({ showTraceViewer }) => { +test('should contain action info', async ({ showTraceViewer }) => { const traceViewer = await showTraceViewer(traceFile); await traceViewer.selectAction('page.click'); const logLines = await traceViewer.logLines(); diff --git a/tests/tracing.spec.ts b/tests/tracing.spec.ts index 2c8fcb4be3..6d4fcf0395 100644 --- a/tests/tracing.spec.ts +++ b/tests/tracing.spec.ts @@ -192,7 +192,7 @@ test('should include interrupted actions', async ({ context, page, server }, tes const { events } = await parseTrace(testInfo.outputPath('trace.zip')); const clickEvent = events.find(e => e.metadata?.apiName === 'page.click'); expect(clickEvent).toBeTruthy(); - expect(clickEvent.metadata.error).toBe('Action was interrupted'); + expect(clickEvent.metadata.error.error.message).toBe('Action was interrupted'); }); diff --git a/utils/check_deps.js b/utils/check_deps.js index 9f78cdc13c..db94900938 100644 --- a/utils/check_deps.js +++ b/utils/check_deps.js @@ -161,8 +161,7 @@ DEPS['src/utils/'] = ['src/common/']; // Trace viewer DEPS['src/server/trace/common/'] = ['src/server/snapshot/', ...DEPS['src/server/']]; DEPS['src/server/trace/recorder/'] = ['src/server/trace/common/', ...DEPS['src/server/trace/common/']]; -DEPS['src/server/trace/viewer/'] = ['src/server/trace/common/', 'src/server/chromium/', ...DEPS['src/server/trace/common/']]; - +DEPS['src/server/trace/viewer/'] = ['src/server/trace/common/', 'src/server/trace/recorder/', 'src/server/chromium/', ...DEPS['src/server/trace/common/']]; DEPS['src/test/'] = ['src/test/**', 'src/utils/utils.ts']; checkDeps().catch(e => {