feat(inspector): render errors (#5459)

index.js

@@ -14,5 +14,4 @@
  * limitations under the License.
  */
 
-const { setUnderTest } = require('./lib/utils/utils');
 module.exports = require('./lib/inprocess');

src/dispatchers/dispatcher.ts

@@ -214,6 +214,7 @@ export class DispatcherConnection {
       this.onmessage({ id, result: this._replaceDispatchersWithGuids(result) });
     } catch (e) {
       // Dispatching error
+      callMetadata.error = e.message;
       if (callMetadata.log.length)
         rewriteErrorMessage(e, e.message + formatLogRecording(callMetadata.log) + kLoggingNote);
       this.onmessage({ id, error: serializeError(e) });

src/server/browserContext.ts

@@ -345,8 +345,8 @@ export abstract class BrowserContext extends SdkObject {
     }
   }
 
-  async extendInjectedScript(source: string) {
-    const installInFrame = (frame: frames.Frame) => frame.extendInjectedScript(source).catch(e => {});
+  async extendInjectedScript(source: string, arg?: any) {
+    const installInFrame = (frame: frames.Frame) => frame.extendInjectedScript(source, arg).catch(() => {});
     const installInPage = (page: Page) => {
       page.on(Page.Events.InternalFrameNavigatedToNewDocument, installInFrame);
       return Promise.all(page.frames().map(installInFrame));

src/server/instrumentation.ts

@@ -39,7 +39,7 @@ export type CallMetadata = {
   params: any;
   stack?: StackFrame[];
   log: string[];
-  error?: Error;
+  error?: string;
   point?: Point;
 };
 

src/server/supplements/injected/recorder.ts

@@ -51,8 +51,10 @@ export class Recorder {
   private _actionPointElement: HTMLElement;
   private _actionPoint: Point | undefined;
   private _actionSelector: string | undefined;
+  private _params: { isUnderTest: boolean; };
 
-  constructor(injectedScript: InjectedScript) {
+  constructor(injectedScript: InjectedScript, params: { isUnderTest: boolean }) {
+    this._params = params;
     this._injectedScript = injectedScript;
     this._outerGlassPaneElement = html`
       <x-pw-glass style="
@@ -76,7 +78,7 @@ export class Recorder {
       </x-pw-glass-inner>`;
 
     // Use a closed shadow root to prevent selectors matching our internal previews.
-    this._glassPaneShadow = this._outerGlassPaneElement.attachShadow({ mode: 'closed' });
+    this._glassPaneShadow = this._outerGlassPaneElement.attachShadow({ mode: this._params.isUnderTest ? 'open' : 'closed' });
     this._glassPaneShadow.appendChild(this._innerGlassPaneElement);
     this._glassPaneShadow.appendChild(this._actionPointElement);
     this._glassPaneShadow.appendChild(html`

src/server/supplements/recorder/recorderApp.ts

@@ -26,6 +26,7 @@ import { internalCallMetadata } from '../../instrumentation';
 import type { CallLog, EventData, Mode, Source } from './recorderTypes';
 import { BrowserContext } from '../../browserContext';
 import { isUnderTest } from '../../../utils/utils';
+import { RecentLogsCollector } from '../../../utils/debugLogger';
 
 const readFileAsync = util.promisify(fs.readFile);
 
@@ -41,11 +42,13 @@ declare global {
 
 export class RecorderApp extends EventEmitter {
   private _page: Page;
+  readonly wsEndpoint: string | undefined;
 
-  constructor(page: Page) {
+  constructor(page: Page, wsEndpoint: string | undefined) {
     super();
     this.setMaxListeners(0);
     this._page = page;
+    this.wsEndpoint = wsEndpoint;
   }
 
   async close() {
@@ -90,28 +93,45 @@ export class RecorderApp extends EventEmitter {
 
   static async open(inspectedContext: BrowserContext): Promise<RecorderApp> {
     const recorderPlaywright = createPlaywright(true);
+    const args = [
+      '--app=data:text/html,',
+      '--window-size=600,600',
+      '--window-position=1280,10',
+    ];
+    if (isUnderTest())
+      args.push(`--remote-debugging-port=0`);
     const context = await recorderPlaywright.chromium.launchPersistentContext(internalCallMetadata(), '', {
       sdkLanguage: inspectedContext._options.sdkLanguage,
-      args: [
-        '--app=data:text/html,',
-        '--window-size=600,600',
-        '--window-position=1280,10',
-      ],
+      args,
       noDefaultViewport: true,
       headless: isUnderTest() && !inspectedContext._browser.options.headful
     });
-
+    const wsEndpoint = isUnderTest() ? await this._parseWsEndpoint(context._browser.options.browserLogsCollector) : undefined;
     const controller = new ProgressController(internalCallMetadata(), context._browser);
     await controller.run(async progress => {
       await context._browser._defaultContext!._loadDefaultContextAsIs(progress);
     });
 
     const [page] = context.pages();
-    const result = new RecorderApp(page);
+    const result = new RecorderApp(page, wsEndpoint);
     await result._init();
     return result;
   }
 
+  private static async _parseWsEndpoint(recentLogs: RecentLogsCollector): Promise<string> {
+    let callback: ((log: string) => void) | undefined;
+    const result = new Promise<string>(f => callback = f);
+    const check = (log: string) => {
+      const match = log.match(/DevTools listening on (.*)/);
+      if (match)
+        callback!(match[1]);
+    };
+    for (const log of recentLogs.recentLogs())
+      check(log);
+    recentLogs.on('log', check);
+    return result;
+  }
+
   async setMode(mode: 'none' | 'recording' | 'inspecting'): Promise<void> {
     await this._page.mainFrame()._evaluateExpression(((mode: Mode) => {
       window.playwrightSetMode(mode);

src/server/supplements/recorder/recorderTypes.ts

@@ -34,11 +34,12 @@ export type CallLog = {
   title: string;
   messages: string[];
   status: 'in-progress' | 'done' | 'error' | 'paused';
+  error?: string;
 };
 
 export type SourceHighlight = {
   line: number;
-  type: 'running' | 'paused';
+  type: 'running' | 'paused' | 'error';
 };
 
 export type Source = {

src/server/supplements/recorderSupplement.ts

@@ -33,6 +33,7 @@ import { RecorderApp } from './recorder/recorderApp';
 import { CallMetadata, internalCallMetadata, SdkObject } from '../instrumentation';
 import { Point } from '../../common/types';
 import { CallLog, EventData, Mode, Source, UIState } from './recorder/recorderTypes';
+import { isUnderTest } from '../../utils/utils';
 
 type BindingSource = { frame: Frame, page: Page };
 
@@ -179,7 +180,7 @@ export class RecorderSupplement {
       this._resume(false).catch(() => {});
     });
 
-    await this._context.extendInjectedScript(recorderSource.source);
+    await this._context.extendInjectedScript(recorderSource.source, { isUnderTest: isUnderTest() });
     await this._context.extendInjectedScript(consoleApiSource.source);
 
     (this._context as any).recorderAppForTest = recorderApp;
@@ -332,7 +333,8 @@ export class RecorderSupplement {
   }
 
   async onAfterCall(metadata: CallMetadata): Promise<void> {
-    this._currentCallsMetadata.delete(metadata);
+    if (!metadata.error)
+      this._currentCallsMetadata.delete(metadata);
     this._pausedCallsMetadata.delete(metadata);
     this._updateUserSources();
     this.updateCallLog([metadata]);
@@ -357,7 +359,7 @@ export class RecorderSupplement {
       }
       if (line) {
         const paused = this._pausedCallsMetadata.has(metadata);
-        source.highlight.push({ line, type: paused ? 'paused' : 'running' });
+        source.highlight.push({ line, type: metadata.error ? 'error' : (paused ? 'paused' : 'running') });
         if (paused)
           source.revealLine = line;
       }
@@ -387,7 +389,7 @@ export class RecorderSupplement {
         status = 'paused';
       if (metadata.error)
         status = 'error';
-      logs.push({ id: metadata.id, messages: metadata.log, title, status });
+      logs.push({ id: metadata.id, messages: metadata.log, title, status, error: metadata.error });
     }
     this._recorderApp?.updateCallLogs(logs);
   }

src/trace/tracer.ts

@@ -180,7 +180,7 @@ class ContextTracer implements SnapshotterDelegate {
       startTime: metadata.startTime,
       endTime: metadata.endTime,
       logs: metadata.log.slice(),
-      error: metadata.error ? metadata.error.stack : undefined,
+      error: metadata.error,
       snapshots: snapshotsForMetadata(metadata),
     };
     this._appendTraceEvent(event);

src/utils/debugLogger.ts

@@ -16,6 +16,7 @@
 
 import debug from 'debug';
 import fs from 'fs';
+import { EventEmitter } from 'events';
 
 const debugLoggerColorMap = {
   'api': 45, // cyan
@@ -63,10 +64,11 @@ class DebugLogger {
 export const debugLogger = new DebugLogger();
 
 const kLogCount = 50;
-export class RecentLogsCollector {
+export class RecentLogsCollector extends EventEmitter {
   private _logs: string[] = [];
 
   log(message: string) {
+    this.emit('log', message);
     this._logs.push(message);
     if (this._logs.length === kLogCount * 2)
       this._logs.splice(0, kLogCount);

src/web/components/source.css

@@ -45,12 +45,18 @@
 }
 
 .source-line-running {
-  background-color: #6fa8dc7f;
+  background-color: #b3dbff7f;
   z-index: 2;
 }
 
 .source-line-paused {
-  background-color: #ffc0cb7f;
-  outline: 1px solid red;
+  background-color: #b3dbff7f;
+  outline: 1px solid #009aff;
+  z-index: 2;
+}
+
+.source-line-error {
+  background-color: #fff0f0;
+  outline: 1px solid #ffd6d6;
   z-index: 2;
 }

src/web/components/source.tsx

@@ -21,7 +21,7 @@ import '../../third_party/highlightjs/highlightjs/tomorrow.css';
 
 export type SourceHighlight = {
   line: number;
-  type: 'running' | 'paused';
+  type: 'running' | 'paused' | 'error';
 };
 
 export interface SourceProps {

src/web/recorder/recorder.css

@@ -42,15 +42,11 @@
 
 .recorder-log-message {
   flex: none;
-  padding: 3px 12px;
+  padding: 3px 0 3px 36px;
   display: flex;
   align-items: center;
 }
 
-.recorder-log-message-sub-level {
-  padding-left: 40px;
-}
-
 .recorder-log-header {
   color: var(--toolbar-color);
   box-shadow: var(--box-shadow);
@@ -63,18 +59,36 @@
 }
 
 .recorder-log-call {
-  color: var(--toolbar-color);
-  background-color: var(--toolbar-bg-color);
-  border-top: 1px solid #ddd;
-  border-bottom: 1px solid #eee;
+  display: flex;
+  flex: none;
+  flex-direction: column;
+  border-top: 1px solid #eee;
+}
+
+.recorder-log-call-header {
   height: 24px;
   display: flex;
   align-items: center;
-  padding: 0 9px;
-  margin-bottom: 3px;
+  padding: 0 2px;
   z-index: 2;
 }
 
 .recorder-log-call .codicon {
   padding: 0 4px;
 }
+
+.recorder-log .codicon-check {
+  color: #21a945;
+  font-weight: bold;
+}
+
+.recorder-log-call.error {
+  background-color: #fff0f0;
+  border-top: 1px solid #ffd6d6;
+}
+
+.recorder-log-call.error .recorder-log-call-header,
+.recorder-log-message.error,
+.recorder-log .codicon-error {
+  color: red;
+}

src/web/recorder/recorder.tsx

@@ -79,12 +79,14 @@ export const Recorder: React.FC<RecorderProps> = ({
         copy(source.text);
       }}></ToolbarButton>
       <ToolbarButton icon='debug-continue' title='Resume' disabled={!paused} onClick={() => {
+        setPaused(false);
         window.dispatch({ event: 'resume' }).catch(() => {});
       }}></ToolbarButton>
       <ToolbarButton icon='debug-pause' title='Pause' disabled={paused} onClick={() => {
         window.dispatch({ event: 'pause' }).catch(() => {});
       }}></ToolbarButton>
       <ToolbarButton icon='debug-step-over' title='Step over' disabled={!paused} onClick={() => {
+        setPaused(false);
         window.dispatch({ event: 'step' }).catch(() => {});
       }}></ToolbarButton>
       <div style={{flex: 'auto'}}></div>
@@ -98,15 +100,18 @@ export const Recorder: React.FC<RecorderProps> = ({
         <div className='recorder-log-header' style={{flex: 'none'}}>Log</div>
         <div className='recorder-log' style={{flex: 'auto'}}>
           {[...log.values()].map(callLog => {
-            return <div className='vbox' style={{flex: 'none'}} key={callLog.id}>
-              <div className='recorder-log-call'>
+            return <div className={`recorder-log-call ${callLog.status}`} key={callLog.id}>
+              <div className='recorder-log-call-header'>
                 <span className={'codicon ' + iconClass(callLog)}></span>{ callLog.title }
               </div>
               { callLog.messages.map((message, i) => {
                 return <div className='recorder-log-message' key={i}>
-                  { message }
+                  { message.trim() }
                 </div>;
               })}
+              { callLog.error ? <div className='recorder-log-message error'>
+                { callLog.error }
+              </div> : undefined }
             </div>
           })}
           <div ref={messagesEndRef}></div>

test/cli/cli.fixtures.ts

@@ -34,10 +34,10 @@ type TestFixtures = {
 
 export const fixtures = baseFolio.extend<TestFixtures, WorkerFixtures>();
 
-fixtures.recorder.init(async ({ page, recorderFrame }, runTest) => {
+fixtures.recorder.init(async ({ page, recorderPageGetter }, runTest) => {
   await (page.context() as any)._enableRecorder({ language: 'javascript', startRecording: true });
-  const recorderFrameInstance = await recorderFrame();
-  await runTest(new Recorder(page, recorderFrameInstance));
+  const recorderPage = await recorderPageGetter();
+  await runTest(new Recorder(page, recorderPage));
 });
 
 fixtures.httpServer.init(async ({testWorkerIndex}, runTest) => {
@@ -65,12 +65,12 @@ class Recorder {
   _highlightInstalled: boolean
   _actionReporterInstalled: boolean
   _actionPerformedCallback: Function
-  recorderFrame: any;
+  recorderPage: Page;
   private _text: string = '';
 
-  constructor(page: Page, recorderFrame: any) {
+  constructor(page: Page, recorderPage: Page) {
     this.page = page;
-    this.recorderFrame = recorderFrame;
+    this.recorderPage = recorderPage;
     this._highlightCallback = () => { };
     this._highlightInstalled = false;
     this._actionReporterInstalled = false;
@@ -98,7 +98,7 @@ class Recorder {
   }
 
   async waitForOutput(text: string): Promise<void> {
-    this._text = await this.recorderFrame._evaluateExpression(((text: string) => {
+    this._text = await this.recorderPage.evaluate((text: string) => {
       const w = window as any;
       return new Promise(f => {
         const poll = () => {
@@ -110,7 +110,7 @@ class Recorder {
         };
         setTimeout(poll);
       });
-    }).toString(), true, text, 'main');
+    }, text);
   }
 
   output(): string {

test/pause.spec.ts

@@ -21,39 +21,102 @@ const { it, describe} = folio;
 describe('pause', (suite, { mode }) => {
   suite.skip(mode !== 'default');
 }, () => {
-  it('should pause and resume the script', async ({ page, recorderClick }) => {
-    await Promise.all([
-      page.pause(),
-      recorderClick('[title=Resume]')
-    ]);
+  it('should pause and resume the script', async ({ page, recorderPageGetter }) => {
+    const scriptPromise = (async () => {
+      await page.pause();
+    })();
+    const recorderPage = await recorderPageGetter();
+    await recorderPage.click('[title=Resume]');
+    await scriptPromise;
   });
 
   it('should resume from console', async ({page}) => {
+    const scriptPromise = (async () => {
+      await page.pause();
+    })();
     await Promise.all([
-      page.pause(),
       page.waitForFunction(() => (window as any).playwright && (window as any).playwright.resume).then(() => {
         return page.evaluate('window.playwright.resume()');
       })
     ]);
+    await scriptPromise;
   });
 
-  it('should pause after a navigation', async ({page, server, recorderClick}) => {
-    await page.goto(server.EMPTY_PAGE);
-    await Promise.all([
-      page.pause(),
-      recorderClick('[title=Resume]')
-    ]);
+  it('should pause after a navigation', async ({page, server, recorderPageGetter}) => {
+    const scriptPromise = (async () => {
+      await page.goto(server.EMPTY_PAGE);
+      await page.pause();
+    })();
+    const recorderPage = await recorderPageGetter();
+    await recorderPage.click('[title=Resume]');
+    await scriptPromise;
   });
 
-  it('should show source', async ({page, server, recorderClick, recorderFrame}) => {
-    await page.goto(server.EMPTY_PAGE);
-    const pausePromise = page.pause();
-    const frame = await recorderFrame();
-    const source = await frame._evaluateExpression((() => {
-      return document.querySelector('.source-line-paused .source-code').textContent;
-    }).toString(), true, undefined, 'main');
+  it('should show source', async ({page, recorderPageGetter}) => {
+    const scriptPromise = (async () => {
+      await page.pause();
+    })();
+    const recorderPage = await recorderPageGetter();
+    const source = await recorderPage.textContent('.source-line-paused .source-code');
     expect(source).toContain('page.pause()');
-    await recorderClick('[title=Resume]');
-    await pausePromise;
+    await recorderPage.click('[title=Resume]');
+    await scriptPromise;
+  });
+
+  it('should pause on next pause', async ({page, recorderPageGetter}) => {
+    const scriptPromise = (async () => {
+      await page.pause();  // 1
+      await page.pause();  // 2
+    })();
+    const recorderPage = await recorderPageGetter();
+    const source = await recorderPage.textContent('.source-line-paused');
+    expect(source).toContain('page.pause();  // 1');
+    await recorderPage.click('[title=Resume]');
+    await recorderPage.waitForSelector('.source-line-paused:has-text("page.pause();  // 2")');
+    await recorderPage.click('[title=Resume]');
+    await scriptPromise;
+  });
+
+  it('should step', async ({page, recorderPageGetter}) => {
+    await page.setContent('<button>Submit</button>');
+    const scriptPromise = (async () => {
+      await page.pause();
+      await page.click('button');
+    })();
+    const recorderPage = await recorderPageGetter();
+    const source = await recorderPage.textContent('.source-line-paused');
+    expect(source).toContain('page.pause();');
+
+    await recorderPage.click('[title="Step over"]');
+    await recorderPage.waitForSelector('.source-line-paused :has-text("page.click")');
+
+    await recorderPage.click('[title=Resume]');
+    await scriptPromise;
+  });
+
+  it('should highlight pointer', async ({page, recorderPageGetter}) => {
+    await page.setContent('<button>Submit</button>');
+    const scriptPromise = (async () => {
+      await page.pause();
+      await page.click('button');
+    })();
+    const recorderPage = await recorderPageGetter();
+    await recorderPage.click('[title="Step over"]');
+
+    const point = await page.waitForSelector('x-pw-action-point');
+    const button = await page.waitForSelector('button');
+    const box1 = await button.boundingBox();
+    const box2 = await point.boundingBox();
+
+    const x1 = box1.x + box1.width / 2;
+    const y1 = box1.y + box1.height / 2;
+    const x2 = box2.x + box2.width / 2;
+    const y2 = box2.y + box2.height / 2;
+
+    expect(Math.abs(x1 - x2) < 2).toBeTruthy();
+    expect(Math.abs(y1 - y2) < 2).toBeTruthy();
+
+    await recorderPage.click('[title="Step over"]');
+    await scriptPromise;
   });
 });

test/recorder.fixtures.ts

@@ -15,25 +15,21 @@
  */
 
 import { folio as baseFolio } from './fixtures';
-import { internalCallMetadata } from '../lib/server/instrumentation';
+import { Page } from '..';
+import { chromium } from '../index';
 
 const fixtures = baseFolio.extend<{
-  recorderFrame: () => Promise<any>,
-  recorderClick: (selector: string) => Promise<void>
+  recorderPageGetter: () => Promise<Page>,
 }>();
 
-fixtures.recorderFrame.init(async ({context, toImpl}, runTest) => {
+fixtures.recorderPageGetter.init(async ({context, toImpl}, runTest) => {
   await runTest(async () => {
     while (!toImpl(context).recorderAppForTest)
       await new Promise(f => setTimeout(f, 100));
-    return toImpl(context).recorderAppForTest._page.mainFrame();
-  });
-});
-
-fixtures.recorderClick.init(async ({ recorderFrame }, runTest) => {
-  await runTest(async (selector: string) => {
-    const frame = await recorderFrame();
-    await frame.click(internalCallMetadata(), selector, {});
+    const wsEndpoint = toImpl(context).recorderAppForTest.wsEndpoint;
+    const browser = await chromium.connectOverCDP({ wsEndpoint });
+    const c = browser.contexts()[0];
+    return c.pages()[0] || await c.waitForEvent('page');
   });
 });