fix: properly rewrite error message (#2392)

Error message is included in error's stack, so we should
re-write stack as well.

Fixes #2373

src/chromium/crConnection.ts

@@ -20,6 +20,7 @@ import { ConnectionTransport, ProtocolRequest, ProtocolResponse, protocolLog } f
 import { Protocol } from './protocol';
 import { EventEmitter } from 'events';
 import { InnerLogger } from '../logger';
+import { rewriteErrorMessage } from '../debug/stackTrace';
 
 export const ConnectionEvents = {
   Disconnected: Symbol('ConnectionEvents.Disconnected')
@@ -189,7 +190,7 @@ export class CRSession extends EventEmitter {
 
   _onClosed() {
     for (const callback of this._callbacks.values())
-      callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`));
+      callback.reject(rewriteErrorMessage(callback.error, `Protocol error (${callback.method}): Target closed.`));
     this._callbacks.clear();
     this._connection = null;
     Promise.resolve().then(() => this.emit(CRSessionEvents.Disconnected));
@@ -200,12 +201,7 @@ function createProtocolError(error: Error, method: string, protocolError: { mess
   let message = `Protocol error (${method}): ${protocolError.message}`;
   if ('data' in protocolError)
     message += ` ${protocolError.data}`;
-  return rewriteError(error, message);
-}
-
-function rewriteError(error: Error, message: string): Error {
-  error.message = message;
-  return error;
+  return rewriteErrorMessage(error, message);
 }
 
 function rewriteInjectedScriptEvaluationLog(message: ProtocolRequest): string {

src/chromium/crExecutionContext.ts

@@ -21,6 +21,7 @@ import { getExceptionMessage, releaseObject } from './crProtocolHelper';
 import { Protocol } from './protocol';
 import * as js from '../javascript';
 import * as debugSupport from '../debug/debugSupport';
+import { rewriteErrorMessage } from '../debug/stackTrace';
 import { parseEvaluationResultValue } from '../utilityScriptSerializers';
 
 export class CRExecutionContext implements js.ExecutionContextDelegate {
@@ -130,7 +131,7 @@ function rewriteError(error: Error): Protocol.Runtime.evaluateReturnValue {
   if (error.message.endsWith('Cannot find context with specified id') || error.message.endsWith('Inspected target navigated or closed') || error.message.endsWith('Execution context was destroyed.'))
     throw new Error('Execution context was destroyed, most likely because of a navigation.');
   if (error instanceof TypeError && error.message.startsWith('Converting circular structure to JSON'))
-    error.message += ' Are you passing a nested JSHandle?';
+    rewriteErrorMessage(error, error.message + ' Are you passing a nested JSHandle?');
   throw error;
 }
 

src/chromium/crPage.ts

@@ -38,6 +38,7 @@ import { ConsoleMessage } from '../console';
 import { NotConnectedError } from '../errors';
 import { logError } from '../logger';
 import * as debugSupport from '../debug/debugSupport';
+import { rewriteErrorMessage } from '../debug/stackTrace';
 
 
 const UTILITY_WORLD_NAME = '__playwright_utility_world__';
@@ -321,7 +322,7 @@ export class CRPage implements PageDelegate {
     const parentSession = this._sessionForFrame(parent);
     const { backendNodeId } = await parentSession._client.send('DOM.getFrameOwner', { frameId: frame._id }).catch(e => {
       if (e instanceof Error && e.message.includes('Frame with the given id was not found.'))
-        e.message = 'Frame has been detached.';
+        rewriteErrorMessage(e, 'Frame has been detached.');
       throw e;
     });
     parent = frame.parentFrame();

src/debug/stackTrace.ts

@@ -79,3 +79,14 @@ export function getCurrentApiCall(prefix = PLAYWRIGHT_LIB_PATH): string {
   }
   return parts.join('.');
 }
+
+export function rewriteErrorMessage(e: Error, newMessage: string): Error {
+  if (e.stack) {
+    const index = e.stack.indexOf(e.message);
+    if (index !== -1)
+      e.stack = e.stack.substring(0, index) + newMessage + e.stack.substring(index + e.message.length);
+  }
+  e.message = newMessage;
+  return e;
+}
+

src/firefox/ffConnection.ts

@@ -20,6 +20,7 @@ import { assert } from '../helper';
 import { ConnectionTransport, ProtocolRequest, ProtocolResponse, protocolLog } from '../transport';
 import { Protocol } from './protocol';
 import { InnerLogger } from '../logger';
+import { rewriteErrorMessage } from '../debug/stackTrace';
 
 export const ConnectionEvents = {
   Disconnected: Symbol('Disconnected'),
@@ -112,7 +113,7 @@ export class FFConnection extends EventEmitter {
     this._transport.onmessage = undefined;
     this._transport.onclose = undefined;
     for (const callback of this._callbacks.values())
-      callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`));
+      callback.reject(rewriteErrorMessage(callback.error, `Protocol error (${callback.method}): Target closed.`));
     this._callbacks.clear();
     for (const session of this._sessions.values())
       session.dispose();
@@ -200,7 +201,7 @@ export class FFSession extends EventEmitter {
 
   dispose() {
     for (const callback of this._callbacks.values())
-      callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`));
+      callback.reject(rewriteErrorMessage(callback.error, `Protocol error (${callback.method}): Target closed.`));
     this._callbacks.clear();
     this._disposed = true;
     this._connection._sessions.delete(this._sessionId);
@@ -212,12 +213,7 @@ function createProtocolError(error: Error, method: string, protocolError: { mess
   let message = `Protocol error (${method}): ${protocolError.message}`;
   if ('data' in protocolError)
     message += ` ${protocolError.data}`;
-  return rewriteError(error, message);
-}
-
-function rewriteError(error: Error, message: string): Error {
-  error.message = message;
-  return error;
+  return rewriteErrorMessage(error, message);
 }
 
 function rewriteInjectedScriptEvaluationLog(message: ProtocolRequest): string {

src/firefox/ffExecutionContext.ts

@@ -20,6 +20,7 @@ import * as js from '../javascript';
 import { FFSession } from './ffConnection';
 import { Protocol } from './protocol';
 import * as debugSupport from '../debug/debugSupport';
+import { rewriteErrorMessage } from '../debug/stackTrace';
 import { parseEvaluationResultValue } from '../utilityScriptSerializers';
 
 export class FFExecutionContext implements js.ExecutionContextDelegate {
@@ -136,7 +137,7 @@ function rewriteError(error: Error): (Protocol.Runtime.evaluateReturnValue | Pro
   if (error.message.includes('Failed to find execution context with id') || error.message.includes('Execution context was destroyed!'))
     throw new Error('Execution context was destroyed, most likely because of a navigation.');
   if (error instanceof TypeError && error.message.startsWith('Converting circular structure to JSON'))
-    error.message += ' Are you passing a nested JSHandle?';
+    rewriteErrorMessage(error, error.message + ' Are you passing a nested JSHandle?');
   throw error;
 }
 

src/firefox/ffPage.ts

@@ -33,6 +33,7 @@ import { Protocol } from './protocol';
 import { selectors } from '../selectors';
 import { NotConnectedError } from '../errors';
 import { logError } from '../logger';
+import { rewriteErrorMessage } from '../debug/stackTrace';
 
 const UTILITY_WORLD_NAME = '__playwright_utility_world__';
 
@@ -360,7 +361,7 @@ export class FFPage implements PageDelegate {
       clip: documentRect,
     }).catch(e => {
       if (e instanceof Error && e.message.includes('document.documentElement is null'))
-        e.message = kScreenshotDuringNavigationError;
+        rewriteErrorMessage(e, kScreenshotDuringNavigationError);
       throw e;
     });
     return Buffer.from(data, 'base64');

src/frames.ts

@@ -29,6 +29,7 @@ import { selectors } from './selectors';
 import * as types from './types';
 import { waitForTimeoutWasUsed } from './hints';
 import { BrowserContext } from './browserContext';
+import { rewriteErrorMessage } from './debug/stackTrace';
 
 type ContextType = 'main' | 'utility';
 type ContextData = {
@@ -1063,7 +1064,7 @@ export class FrameTask {
       return result!;
     this.done();
     if (this._url)
-      error.message = error.message + ` while navigating to ${this._url}`;
+      rewriteErrorMessage(error, error.message + ` while navigating to ${this._url}`);
     throw error;
   }
 

src/screenshotter.ts

@@ -22,6 +22,7 @@ import * as dom from './dom';
 import { assert, helper } from './helper';
 import { Page } from './page';
 import * as types from './types';
+import { rewriteErrorMessage } from './debug/stackTrace';
 
 export class Screenshotter {
   private _queue = new TaskQueue();
@@ -220,6 +221,6 @@ function validateScreenshotOptions(options: types.ScreenshotOptions): 'png' | 'j
 export const kScreenshotDuringNavigationError = 'Cannot take a screenshot while page is navigating';
 function rewriteError(e: any) {
   if (typeof e === 'object' && e instanceof Error && e.message.includes('Execution context was destroyed'))
-    e.message = kScreenshotDuringNavigationError;
+    rewriteErrorMessage(e, kScreenshotDuringNavigationError);
   throw e;
 }

src/server/browserType.ts

@@ -28,6 +28,7 @@ import { assert, helper } from '../helper';
 import { TimeoutSettings } from '../timeoutSettings';
 import { launchProcess, Env, waitForLine } from './processLauncher';
 import { Events } from '../events';
+import { rewriteErrorMessage } from '../debug/stackTrace';
 import { TimeoutError } from '../errors';
 import { PipeTransport } from './pipeTransport';
 
@@ -134,9 +135,9 @@ export abstract class BrowserTypeBase implements BrowserType {
       const browser = await helper.waitWithDeadline(promise, 'the browser to launch', deadline, 'pw:browser*');
       return browser;
     } catch (e) {
-      e.message += '\n=============== Process output during launch: ===============\n' +
+      rewriteErrorMessage(e, e.message + '\n=============== Process output during launch: ===============\n' +
           logger.launchRecording() +
-          '\n=============================================================';
+          '\n=============================================================');
       if (browserServer)
         await browserServer._closeOrKill(deadline);
       throw e;
@@ -182,9 +183,9 @@ export abstract class BrowserTypeBase implements BrowserType {
       logger.stopLaunchRecording();
       return browser;
     } catch (e) {
-      e.message += '\n=============== Process output during connect: ===============\n' +
+      rewriteErrorMessage(e, e.message + '\n=============== Process output during connect: ===============\n' +
           logger.launchRecording() +
-          '\n=============================================================';
+          '\n=============================================================');
       try {
         if (transport)
           transport.close();

src/webkit/wkConnection.ts

@@ -20,6 +20,7 @@ import { assert } from '../helper';
 import { ConnectionTransport, ProtocolRequest, ProtocolResponse, protocolLog } from '../transport';
 import { Protocol } from './protocol';
 import { InnerLogger } from '../logger';
+import { rewriteErrorMessage } from '../debug/stackTrace';
 
 // WKPlaywright uses this special id to issue Browser.close command which we
 // should ignore.
@@ -147,7 +148,7 @@ export class WKSession extends EventEmitter {
 
   dispose() {
     for (const callback of this._callbacks.values())
-      callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): ${this.errorText}`));
+      callback.reject(rewriteErrorMessage(callback.error, `Protocol error (${callback.method}): ${this.errorText}`));
     this._callbacks.clear();
     this._disposed = true;
   }
@@ -173,12 +174,7 @@ export function createProtocolError(error: Error, method: string, protocolError:
   let message = `Protocol error (${method}): ${protocolError.message}`;
   if ('data' in protocolError)
     message += ` ${JSON.stringify(protocolError.data)}`;
-  return rewriteError(error, message);
-}
-
-export function rewriteError(error: Error, message: string): Error {
-  error.message = message;
-  return error;
+  return rewriteErrorMessage(error, message);
 }
 
 export function isSwappedOutError(e: Error) {