fix: fix line/column number in errors

puppeteer · Sep 18, 2023 · 3db217b · 3db217b
1 parent e7c8ff8
commit 3db217b
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 19 deletions.
diff --git a/packages/puppeteer-core/src/bidi/Page.ts b/packages/puppeteer-core/src/bidi/Page.ts
@@ -344,24 +344,27 @@ export class BidiPage extends Page {
         )
       );
     } else if (isJavaScriptLogEntry(event)) {
-      let message = event.text ?? '';
+      const error = new Error(event.text ?? '');
 
+      const messageHeight = error.message.split('\n').length;
+      const messageLines = error.stack!.split('\n').splice(0, messageHeight);
+
+      const stackLines = [];
       if (event.stackTrace) {
-        for (const callFrame of event.stackTrace.callFrames) {
-          const location =
-            callFrame.url +
-            ':' +
-            callFrame.lineNumber +
-            ':' +
-            callFrame.columnNumber;
-          const functionName = callFrame.functionName || '<anonymous>';
-          message += `\n    at ${functionName} (${location})`;
+        for (const frame of event.stackTrace.callFrames) {
+          // Note we need to add `1` because the values are 0-indexed.
+          stackLines.push(
+            `    at ${frame.functionName || '<anonymous>'} (${frame.url}:${
+              frame.lineNumber + 1
+            }:${frame.columnNumber + 1})`
+          );
+          if (stackLines.length >= Error.stackTraceLimit) {
+            break;
+          }
         }
       }
 
-      const error = new Error(message);
-      error.stack = ''; // Don't capture Puppeteer stacktrace.
-
+      error.stack = [...messageLines, ...stackLines].join('\n');
       this.emit(PageEvent.PageError, error);
     } else {
       debugError(

diff --git a/packages/puppeteer-core/src/common/util.ts b/packages/puppeteer-core/src/common/util.ts
@@ -124,18 +124,20 @@ export function createClientError(
     name = detail.name;
     message = detail.message;
   }
-  const messageHeight = message.split('\n').length;
   const error = new Error(message);
   error.name = name;
 
-  const stackLines = [];
+  const messageHeight = error.message.split('\n').length;
   const messageLines = error.stack!.split('\n').splice(0, messageHeight);
-  if (details.stackTrace && stackLines.length < Error.stackTraceLimit) {
-    for (const frame of details.stackTrace.callFrames.reverse()) {
+
+  const stackLines = [];
+  if (details.stackTrace) {
+    for (const frame of details.stackTrace.callFrames) {
+      // Note we need to add `1` because the values are 0-indexed.
       stackLines.push(
         `    at ${frame.functionName || '<anonymous>'} (${frame.url}:${
-          frame.lineNumber
-        }:${frame.columnNumber})`
+          frame.lineNumber + 1
+        }:${frame.columnNumber + 1})`
       );
       if (stackLines.length >= Error.stackTraceLimit) {
         break;

diff --git a/test/src/page.spec.ts b/test/src/page.spec.ts
@@ -1369,6 +1369,7 @@ describe('Page', function () {
         page.goto(server.PREFIX + '/error.html'),
       ]);
       expect(error.message).toContain('Fancy');
+      expect(error.stack?.split('\n')[1]).toContain('error.html:13');
     });
   });