feat(rpc): remove PageAttribution from the protocol, attribute on the client side (#2957)

This also changes timeout error format to
"page.click: Timeout 5000ms exceeded", so that all errors
can be similarly prefixed with api name.

We can now have different api names in different clients,
and our protocol is more reasonable.

Author: dgozman

src/dom.ts

@@ -287,11 +287,11 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     };
   }
 
-  async _retryPointerAction(progress: Progress, action: (point: types.Point) => Promise<void>, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
+  async _retryPointerAction(progress: Progress, actionName: string, action: (point: types.Point) => Promise<void>, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
     let first = true;
     while (progress.isRunning()) {
-      progress.logger.info(`${first ? 'attempting' : 'retrying'} ${progress.apiName} action`);
-      const result = await this._performPointerAction(progress, action, options);
+      progress.logger.info(`${first ? 'attempting' : 'retrying'} ${actionName} action`);
+      const result = await this._performPointerAction(progress, actionName, action, options);
       first = false;
       if (result === 'error:notvisible') {
         if (options.force)
@@ -316,7 +316,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     return 'done';
   }
 
-  async _performPointerAction(progress: Progress, action: (point: types.Point) => Promise<void>, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notvisible' | 'error:notconnected' | 'error:notinviewport' | 'error:nothittarget' | 'done'> {
+  async _performPointerAction(progress: Progress, actionName: string, action: (point: types.Point) => Promise<void>, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notvisible' | 'error:notconnected' | 'error:notinviewport' | 'error:nothittarget' | 'done'> {
     const { force = false, position } = options;
     if ((options as any).__testHookBeforeStable)
       await (options as any).__testHookBeforeStable();
@@ -357,9 +357,9 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       let restoreModifiers: types.KeyboardModifier[] | undefined;
       if (options && options.modifiers)
         restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
-      progress.logger.info(`  performing ${progress.apiName} action`);
+      progress.logger.info(`  performing ${actionName} action`);
       await action(point);
-      progress.logger.info(`  ${progress.apiName} action done`);
+      progress.logger.info(`  ${actionName} action done`);
       progress.logger.info('  waiting for scheduled navigations to finish');
       if ((options as any).__testHookAfterPointerAction)
         await (options as any).__testHookAfterPointerAction();
@@ -379,7 +379,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
   }
 
   _hover(progress: Progress, options: types.PointerActionOptions & types.PointerActionWaitOptions): Promise<'error:notconnected' | 'done'> {
-    return this._retryPointerAction(progress, point => this._page.mouse.move(point.x, point.y), options);
+    return this._retryPointerAction(progress, 'hover', point => this._page.mouse.move(point.x, point.y), options);
   }
 
   click(options: types.MouseClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise<void> {
@@ -390,7 +390,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
   }
 
   _click(progress: Progress, options: types.MouseClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
-    return this._retryPointerAction(progress, point => this._page.mouse.click(point.x, point.y, options), options);
+    return this._retryPointerAction(progress, 'click', point => this._page.mouse.click(point.x, point.y, options), options);
   }
 
   dblclick(options: types.MouseMultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise<void> {
@@ -401,7 +401,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
   }
 
   _dblclick(progress: Progress, options: types.MouseMultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
-    return this._retryPointerAction(progress, point => this._page.mouse.dblclick(point.x, point.y, options), options);
+    return this._retryPointerAction(progress, 'dblclick', point => this._page.mouse.dblclick(point.x, point.y, options), options);
   }
 
   async selectOption(values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[] | null, options: types.NavigatingActionWaitOptions = {}): Promise<string[]> {

src/logger.ts

@@ -58,13 +58,15 @@ export class Logger {
     return this._innerLog('error', message, args);
   }
 
-  createScope(scopeName: string, record?: boolean): Logger {
-    this._loggerSink.log(this._name, 'info', `=> ${scopeName} started`, [], this._hints);
+  createScope(scopeName: string | undefined, record?: boolean): Logger {
+    if (scopeName)
+      this._loggerSink.log(this._name, 'info', `=> ${scopeName} started`, [], this._hints);
     return new Logger(this._loggerSink, this._name, this._hints, scopeName, record);
   }
 
   endScope(status: string) {
-    this._loggerSink.log(this._name, 'info', `<= ${this._scopeName} ${status}`, [], this._hints);
+    if (this._scopeName)
+      this._loggerSink.log(this._name, 'info', `<= ${this._scopeName} ${status}`, [], this._hints);
   }
 
   private _innerLog(severity: LoggerSeverity, message: string | Error, ...args: any[]) {

src/progress.ts

@@ -20,7 +20,6 @@ import { assert } from './helper';
 import { rewriteErrorMessage } from './utils/stackTrace';
 
 export interface Progress {
-  readonly apiName: string;
   readonly aborted: Promise<void>;
   readonly logger: Logger;
   timeUntilDeadline(): number;
@@ -34,6 +33,11 @@ export async function runAbortableTask<T>(task: (progress: Progress) => Promise<
   return controller.run(task);
 }
 
+let useApiName = true;
+export function setUseApiName(value: boolean) {
+  useApiName = value;
+}
+
 export class ProgressController {
   // Promise and callback that forcefully abort the progress.
   // This promise always rejects.
@@ -70,10 +74,9 @@ export class ProgressController {
     assert(this._state === 'before');
     this._state = 'running';
 
-    const loggerScope = this._logger.createScope(this._apiName, true);
+    const loggerScope = this._logger.createScope(useApiName ? this._apiName : undefined, true);
 
     const progress: Progress = {
-      apiName: this._apiName,
       aborted: this._abortedPromise,
       logger: loggerScope,
       timeUntilDeadline: () => this._deadline ? this._deadline - monotonicTime() : 2147483647, // 2^31-1 safe setTimeout in Node.
@@ -90,7 +93,7 @@ export class ProgressController {
       },
     };
 
-    const timeoutError = new TimeoutError(`Timeout ${this._timeout}ms exceeded during ${this._apiName}.`);
+    const timeoutError = new TimeoutError(`Timeout ${this._timeout}ms exceeded.`);
     const timer = setTimeout(() => this._forceAbort(timeoutError), progress.timeUntilDeadline());
     try {
       const promise = task(progress);
@@ -101,7 +104,11 @@ export class ProgressController {
       return result;
     } catch (e) {
       this._aborted();
-      rewriteErrorMessage(e, e.message + formatLogRecording(loggerScope.recording(), this._apiName) + kLoggingNote);
+      rewriteErrorMessage(e,
+          (useApiName ? `${this._apiName}: ` : '') +
+          e.message +
+          formatLogRecording(loggerScope.recording()) +
+          kLoggingNote);
       clearTimeout(timer);
       this._state = 'aborted';
       loggerScope.endScope(`failed`);
@@ -124,14 +131,14 @@ async function runCleanup(cleanup: () => any) {
 
 const kLoggingNote = `\nNote: use DEBUG=pw:api environment variable and rerun to capture Playwright logs.`;
 
-function formatLogRecording(log: string[], name: string): string {
+function formatLogRecording(log: string[]): string {
   if (!log.length)
     return '';
-  name = ` ${name} logs `;
+  const header = ` logs `;
   const headerLength = 60;
-  const leftLength = (headerLength - name.length) / 2;
-  const rightLength = headerLength - name.length - leftLength;
-  return `\n${'='.repeat(leftLength)}${name}${'='.repeat(rightLength)}\n${log.join('\n')}\n${'='.repeat(headerLength)}`;
+  const leftLength = (headerLength - header.length) / 2;
+  const rightLength = headerLength - header.length - leftLength;
+  return `\n${'='.repeat(leftLength)}${header}${'='.repeat(rightLength)}\n${log.join('\n')}\n${'='.repeat(headerLength)}`;
 }
 
 function monotonicTime(): number {

src/rpc/channels.ts

@@ -173,43 +173,41 @@ export type PageInitializer = {
   isClosed: boolean
 };
 
-export type PageAttribution = { isPage?: boolean };
-
 export interface FrameChannel extends Channel {
   on(event: 'loadstate', callback: (params: { add?: types.LifecycleEvent, remove?: types.LifecycleEvent }) => void): this;
 
-  evalOnSelector(params: { selector: string; expression: string, isFunction: boolean, arg: any} & PageAttribution): Promise<{ value: any }>;
-  evalOnSelectorAll(params: { selector: string; expression: string, isFunction: boolean, arg: any} & PageAttribution): Promise<{ value: any }>;
-  addScriptTag(params: { url?: string, content?: string, type?: string } & PageAttribution): Promise<{ element: ElementHandleChannel }>;
-  addStyleTag(params: { url?: string, content?: string } & PageAttribution): Promise<{ element: ElementHandleChannel }>;
-  check(params: { selector: string, force?: boolean, noWaitAfter?: boolean } & types.TimeoutOptions & PageAttribution): Promise<void>;
-  click(params: { selector: string, force?: boolean, noWaitAfter?: boolean } & types.PointerActionOptions & types.MouseClickOptions & types.TimeoutOptions & PageAttribution): Promise<void>;
+  evalOnSelector(params: { selector: string; expression: string, isFunction: boolean, arg: any}): Promise<{ value: any }>;
+  evalOnSelectorAll(params: { selector: string; expression: string, isFunction: boolean, arg: any}): Promise<{ value: any }>;
+  addScriptTag(params: { url?: string, content?: string, type?: string }): Promise<{ element: ElementHandleChannel }>;
+  addStyleTag(params: { url?: string, content?: string }): Promise<{ element: ElementHandleChannel }>;
+  check(params: { selector: string, force?: boolean, noWaitAfter?: boolean } & types.TimeoutOptions): Promise<void>;
+  click(params: { selector: string, force?: boolean, noWaitAfter?: boolean } & types.PointerActionOptions & types.MouseClickOptions & types.TimeoutOptions): Promise<void>;
   content(): Promise<{ value: string }>;
-  dblclick(params: { selector: string, force?: boolean } & types.PointerActionOptions & types.MouseMultiClickOptions & types.TimeoutOptions & PageAttribution): Promise<void>;
-  dispatchEvent(params: { selector: string, type: string, eventInit: any } & types.TimeoutOptions & PageAttribution): Promise<void>;
-  evaluateExpression(params: { expression: string, isFunction: boolean, arg: any} & PageAttribution): Promise<{ value: any }>;
-  evaluateExpressionHandle(params: { expression: string, isFunction: boolean, arg: any} & PageAttribution): Promise<{ handle: JSHandleChannel }>;
-  fill(params: { selector: string, value: string } & types.NavigatingActionWaitOptions & PageAttribution): Promise<void>;
-  focus(params: { selector: string } & types.TimeoutOptions & PageAttribution): Promise<void>;
+  dblclick(params: { selector: string, force?: boolean } & types.PointerActionOptions & types.MouseMultiClickOptions & types.TimeoutOptions): Promise<void>;
+  dispatchEvent(params: { selector: string, type: string, eventInit: any } & types.TimeoutOptions): Promise<void>;
+  evaluateExpression(params: { expression: string, isFunction: boolean, arg: any}): Promise<{ value: any }>;
+  evaluateExpressionHandle(params: { expression: string, isFunction: boolean, arg: any}): Promise<{ handle: JSHandleChannel }>;
+  fill(params: { selector: string, value: string } & types.NavigatingActionWaitOptions): Promise<void>;
+  focus(params: { selector: string } & types.TimeoutOptions): Promise<void>;
   frameElement(): Promise<{ element: ElementHandleChannel }>;
-  getAttribute(params: { selector: string, name: string } & types.TimeoutOptions & PageAttribution): Promise<{ value: string | null }>;
-  goto(params: { url: string } & types.GotoOptions & PageAttribution): Promise<{ response: ResponseChannel | null }>;
-  hover(params: { selector: string, force?: boolean } & types.PointerActionOptions & types.TimeoutOptions & PageAttribution): Promise<void>;
-  innerHTML(params: { selector: string } & types.TimeoutOptions & PageAttribution): Promise<{ value: string }>;
-  innerText(params: { selector: string } & types.TimeoutOptions & PageAttribution): Promise<{ value: string }>;
-  press(params: { selector: string, key: string, delay?: number, noWaitAfter?: boolean } & types.TimeoutOptions & PageAttribution): Promise<void>;
-  querySelector(params: { selector: string} & PageAttribution): Promise<{ element: ElementHandleChannel | null }>;
-  querySelectorAll(params: { selector: string} & PageAttribution): Promise<{ elements: ElementHandleChannel[] }>;
-  selectOption(params: { selector: string, elements?: ElementHandleChannel[], options?: types.SelectOption[] } & types.NavigatingActionWaitOptions & PageAttribution): Promise<{ values: string[] }>;
-  setContent(params: { html: string } & types.NavigateOptions & PageAttribution): Promise<void>;
-  setInputFiles(params: { selector: string, files: { name: string, mimeType: string, buffer: Binary }[] } & types.NavigatingActionWaitOptions & PageAttribution): Promise<void>;
-  textContent(params: { selector: string } & types.TimeoutOptions & PageAttribution): Promise<{ value: string | null }>;
+  getAttribute(params: { selector: string, name: string } & types.TimeoutOptions): Promise<{ value: string | null }>;
+  goto(params: { url: string } & types.GotoOptions): Promise<{ response: ResponseChannel | null }>;
+  hover(params: { selector: string, force?: boolean } & types.PointerActionOptions & types.TimeoutOptions): Promise<void>;
+  innerHTML(params: { selector: string } & types.TimeoutOptions): Promise<{ value: string }>;
+  innerText(params: { selector: string } & types.TimeoutOptions): Promise<{ value: string }>;
+  press(params: { selector: string, key: string, delay?: number, noWaitAfter?: boolean } & types.TimeoutOptions): Promise<void>;
+  querySelector(params: { selector: string}): Promise<{ element: ElementHandleChannel | null }>;
+  querySelectorAll(params: { selector: string}): Promise<{ elements: ElementHandleChannel[] }>;
+  selectOption(params: { selector: string, elements?: ElementHandleChannel[], options?: types.SelectOption[] } & types.NavigatingActionWaitOptions): Promise<{ values: string[] }>;
+  setContent(params: { html: string } & types.NavigateOptions): Promise<void>;
+  setInputFiles(params: { selector: string, files: { name: string, mimeType: string, buffer: Binary }[] } & types.NavigatingActionWaitOptions): Promise<void>;
+  textContent(params: { selector: string } & types.TimeoutOptions): Promise<{ value: string | null }>;
   title(): Promise<{ value: string }>;
-  type(params: { selector: string, text: string, delay?: number, noWaitAfter?: boolean } & types.TimeoutOptions & PageAttribution): Promise<void>;
-  uncheck(params: { selector: string, force?: boolean, noWaitAfter?: boolean } & types.TimeoutOptions & PageAttribution): Promise<void>;
-  waitForFunction(params: { expression: string, isFunction: boolean, arg: any } & types.WaitForFunctionOptions & PageAttribution): Promise<{ handle: JSHandleChannel }>;
-  waitForNavigation(params: types.WaitForNavigationOptions & PageAttribution): Promise<{ response: ResponseChannel | null }>;
-  waitForSelector(params: { selector: string } & types.WaitForElementOptions & PageAttribution): Promise<{ element: ElementHandleChannel | null }>;
+  type(params: { selector: string, text: string, delay?: number, noWaitAfter?: boolean } & types.TimeoutOptions): Promise<void>;
+  uncheck(params: { selector: string, force?: boolean, noWaitAfter?: boolean } & types.TimeoutOptions): Promise<void>;
+  waitForFunction(params: { expression: string, isFunction: boolean, arg: any } & types.WaitForFunctionOptions): Promise<{ handle: JSHandleChannel }>;
+  waitForNavigation(params: types.WaitForNavigationOptions): Promise<{ response: ResponseChannel | null }>;
+  waitForSelector(params: { selector: string } & types.WaitForElementOptions): Promise<{ element: ElementHandleChannel | null }>;
 }
 export type FrameInitializer = {
   url: string,

src/rpc/client/browserType.ts

@@ -43,30 +43,39 @@ export class BrowserType extends ChannelOwner<BrowserTypeChannel, BrowserTypeIni
   async launch(options: types.LaunchOptions & { logger?: LoggerSink } = {}): Promise<Browser> {
     const logger = options.logger;
     options = { ...options, logger: undefined };
-    const browser = Browser.from((await this._channel.launch(options)).browser);
-    browser._logger = logger;
-    return browser;
+    return this._wrapApiCall('browserType.launch', async () => {
+      const browser = Browser.from((await this._channel.launch(options)).browser);
+      browser._logger = logger;
+      return browser;
+    }, logger);
   }
 
   async launchServer(options: types.LaunchServerOptions & { logger?: LoggerSink } = {}): Promise<BrowserServer> {
+    const logger = options.logger;
     options = { ...options, logger: undefined };
-    return BrowserServer.from((await this._channel.launchServer(options)).server);
+    return this._wrapApiCall('browserType.launchServer', async () => {
+      return BrowserServer.from((await this._channel.launchServer(options)).server);
+    }, logger);
   }
 
   async launchPersistentContext(userDataDir: string, options: types.LaunchOptions & types.BrowserContextOptions & { logger?: LoggerSink } = {}): Promise<BrowserContext> {
     const logger = options.logger;
     options = { ...options, logger: undefined };
-    const result = await this._channel.launchPersistentContext({ userDataDir, ...options });
-    const context = BrowserContext.from(result.context);
-    context._logger = logger;
-    return context;
+    return this._wrapApiCall('browserType.launchPersistentContext', async () => {
+      const result = await this._channel.launchPersistentContext({ userDataDir, ...options });
+      const context = BrowserContext.from(result.context);
+      context._logger = logger;
+      return context;
+    }, logger);
   }
 
   async connect(options: types.ConnectOptions & { logger?: LoggerSink }): Promise<Browser> {
     const logger = options.logger;
     options = { ...options, logger: undefined };
-    const browser = Browser.from((await this._channel.connect(options)).browser);
-    browser._logger = logger;
-    return browser;
+    return this._wrapApiCall('browserType.connect', async () => {
+      const browser = Browser.from((await this._channel.connect(options)).browser);
+      browser._logger = logger;
+      return browser;
+    }, logger);
   }
 }