Core: Report full stack trace for uncaught errors if possible (#1324)

Currently, when QUnit catches an uncaught error in its window.onerror handler, it reports it with a source of `fileName + ":" + lineNumber`. While it's not standard, most modern browsers include an errorObj argument to window.onerror, which can be used to get a more complete stacktrace ([source](https://blog.sentry.io/2016/01/04/client-javascript-reporting-window-onerror)). This change modifies the HTML reporter and the `QUnit.onError` method to do that.
qunitjs · Nov 1, 2018 · ed88074 · ed88074
1 parent 112d782
commit ed88074
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 4 deletions.
diff --git a/reporter/html.js b/reporter/html.js
@@ -1,4 +1,5 @@
 import QUnit from "../src/core";
+import { extractStacktrace } from "../src/core/stacktrace";
 import { window, navigator } from "../src/globals";
 import "./urlparams";
 
@@ -970,10 +971,18 @@ export function escapeText( s ) {
 	// Cover uncaught exceptions
 	// Returning true will suppress the default browser handler,
 	// returning false will let it run.
-	window.onerror = function( message, fileName, lineNumber, ...args ) {
+	window.onerror = function( message, fileName, lineNumber, columnNumber, errorObj, ...args ) {
 		var ret = false;
 		if ( originalWindowOnError ) {
-			ret = originalWindowOnError.call( this, message, fileName, lineNumber, ...args );
+			ret = originalWindowOnError.call(
+				this,
+				message,
+				fileName,
+				lineNumber,
+				columnNumber,
+				errorObj,
+				...args
+			);
 		}
 
 		// Treat return value as window.onerror itself does,
@@ -985,6 +994,14 @@ export function escapeText( s ) {
 				lineNumber
 			};
 
+			// According to
+			// https://blog.sentry.io/2016/01/04/client-javascript-reporting-window-onerror,
+			// most modern browsers support an errorObj argument; use that to
+			// get a full stack trace if it's available.
+			if ( errorObj && errorObj.stack ) {
+				error.stacktrace = extractStacktrace( errorObj, 0 );
+			}
+
 			ret = QUnit.onError( error );
 		}
 

diff --git a/src/core/onerror.js b/src/core/onerror.js
@@ -12,10 +12,18 @@ export default function onError( error, ...args ) {
 		if ( config.current.ignoreGlobalErrors ) {
 			return true;
 		}
-		pushFailure( error.message, error.fileName + ":" + error.lineNumber, ...args );
+		pushFailure(
+			error.message,
+			error.stacktrace || error.fileName + ":" + error.lineNumber,
+			...args
+		);
 	} else {
 		test( "global failure", extend( function() {
-			pushFailure( error.message, error.fileName + ":" + error.lineNumber, ...args );
+			pushFailure(
+				error.message,
+				error.stacktrace || error.fileName + ":" + error.lineNumber,
+				...args
+			);
 		}, { validTest: true } ) );
 	}
 

diff --git a/test/onerror/inside-test.js b/test/onerror/inside-test.js
@@ -16,6 +16,29 @@ QUnit.module( "QUnit.onError", function() {
 		assert.strictEqual( result, false, "onError should allow other error handlers to run" );
 	} );
 
+	QUnit.test( "Should use stacktrace argument when it is present", function( assert ) {
+		assert.expect( 3 );
+
+		QUnit.config.current.pushFailure = function( message, source ) {
+			assert.strictEqual( message, "Error message", "Message is correct" );
+			assert.strictEqual(
+				source,
+				"DummyError\nfilePath.js:1 foo()\nfilePath.js:2 bar()",
+				"Source is correct"
+			);
+		};
+
+		var result = QUnit.onError( {
+			message: "Error message",
+			fileName: "filePath.js",
+			lineNumber: 1,
+			stacktrace: "DummyError\nfilePath.js:1 foo()\nfilePath.js:2 bar()"
+		} );
+
+		assert.strictEqual( result, false, "onError should allow other error handlers to run" );
+	} );
+
+
 	QUnit.test( "Shouldn't push failure if ignoreGlobalErrors is enabled", function( assert ) {
 		assert.expect( 1 );
 

diff --git a/test/reporter-html/window-onerror.js b/test/reporter-html/window-onerror.js
@@ -19,6 +19,21 @@ QUnit.module( "window.onerror (no preexisting handler)", function( hooks ) {
 		window.onerror( "An error message", "filename.js", 1 );
 	} );
 
+	QUnit.test( "Should extract stacktrace if it is available", function( assert ) {
+		assert.expect( 1 );
+
+		const errorObj = {
+			stack: "dummy.js:1 top()\ndummy.js:2 middle()\ndummy.js:3 bottom()"
+		};
+
+		QUnit.onError = function( error ) {
+			assert.equal( error.stacktrace, errorObj.stack, "QUnit.onError was called" );
+		};
+
+		window.onerror( "An error message", "filename.js", 1, 1, errorObj );
+	} );
+
+
 	QUnit.test( "Should return QUnit.error return value if it is called", function( assert ) {
 		assert.expect( 1 );