Store an error object in AssertionError rather than a stack trace

avajs · Jun 10, 2019 · 7366a9d · 7366a9d
1 parent 007c7af
commit 7366a9d
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 42 deletions.
diff --git a/lib/assert.js b/lib/assert.js
@@ -53,25 +53,21 @@ class AssertionError extends Error {
 		// Reserved for power-assert statements
 		this.statements = [];
 
-		if (opts.stack) {
-			this.stack = opts.stack;
+		if (opts.savedError) {
+			this.savedError = opts.savedError;
 		} else {
-			const limitBefore = Error.stackTraceLimit;
-			Error.stackTraceLimit = Infinity;
-			Error.captureStackTrace(this);
-			Error.stackTraceLimit = limitBefore;
+			this.savedError = getErrorWithLongStackTrace();
 		}
 	}
 }
 exports.AssertionError = AssertionError;
 
-function getStack() {
+function getErrorWithLongStackTrace() {
 	const limitBefore = Error.stackTraceLimit;
 	Error.stackTraceLimit = Infinity;
-	const obj = {};
-	Error.captureStackTrace(obj, getStack);
+	const err = new Error();
 	Error.stackTraceLimit = limitBefore;
-	return obj.stack;
+	return err;
 }
 
 function validateExpectations(assertion, expectations, numArgs) { // eslint-disable-line complexity
@@ -143,12 +139,12 @@ function validateExpectations(assertion, expectations, numArgs) { // eslint-disa
 
 // Note: this function *must* throw exceptions, since it can be used
 // as part of a pending assertion for promises.
-function assertExpectations({assertion, actual, expectations, message, prefix, stack}) {
+function assertExpectations({assertion, actual, expectations, message, prefix, savedError}) {
 	if (!isError(actual)) {
 		throw new AssertionError({
 			assertion,
 			message,
-			stack,
+			savedError,
 			values: [formatWithLabel(`${prefix} exception that is not an error:`, actual)]
 		});
 	}
@@ -159,7 +155,7 @@ function assertExpectations({assertion, actual, expectations, message, prefix, s
 		throw new AssertionError({
 			assertion,
 			message,
-			stack,
+			savedError,
 			actualStack,
 			values: [
 				formatWithLabel(`${prefix} unexpected exception:`, actual),
@@ -172,7 +168,7 @@ function assertExpectations({assertion, actual, expectations, message, prefix, s
 		throw new AssertionError({
 			assertion,
 			message,
-			stack,
+			savedError,
 			actualStack,
 			values: [
 				formatWithLabel(`${prefix} unexpected exception:`, actual),
@@ -185,7 +181,7 @@ function assertExpectations({assertion, actual, expectations, message, prefix, s
 		throw new AssertionError({
 			assertion,
 			message,
-			stack,
+			savedError,
 			actualStack,
 			values: [
 				formatWithLabel(`${prefix} unexpected exception:`, actual),
@@ -198,7 +194,7 @@ function assertExpectations({assertion, actual, expectations, message, prefix, s
 		throw new AssertionError({
 			assertion,
 			message,
-			stack,
+			savedError,
 			actualStack,
 			values: [
 				formatWithLabel(`${prefix} unexpected exception:`, actual),
@@ -211,7 +207,7 @@ function assertExpectations({assertion, actual, expectations, message, prefix, s
 		throw new AssertionError({
 			assertion,
 			message,
-			stack,
+			savedError,
 			actualStack,
 			values: [
 				formatWithLabel(`${prefix} unexpected exception:`, actual),
@@ -224,7 +220,7 @@ function assertExpectations({assertion, actual, expectations, message, prefix, s
 		throw new AssertionError({
 			assertion,
 			message,
-			stack,
+			savedError,
 			actualStack,
 			values: [
 				formatWithLabel(`${prefix} unexpected exception:`, actual),
@@ -480,14 +476,14 @@ class Assertions {
 			}
 
 			const handlePromise = (promise, wasReturned) => {
-				// Record stack before it gets lost in the promise chain.
-				const stack = getStack();
+				// Create an error object to record the stack before it gets lost in the promise chain.
+				const savedError = getErrorWithLongStackTrace();
 				// Handle "promise like" objects by casting to a real Promise.
 				const intermediate = Promise.resolve(promise).then(value => { // eslint-disable-line promise/prefer-await-to-then
 					throw new AssertionError({
 						assertion: 'throwsAsync',
 						message,
-						stack,
+						savedError,
 						values: [formatWithLabel(`${wasReturned ? 'Returned promise' : 'Promise'} resolved with:`, value)]
 					});
 				}, reason => {
@@ -497,7 +493,7 @@ class Assertions {
 						expectations,
 						message,
 						prefix: `${wasReturned ? 'Returned promise' : 'Promise'} rejected with`,
-						stack
+						savedError
 					});
 					return reason;
 				});
@@ -587,14 +583,14 @@ class Assertions {
 			}
 
 			const handlePromise = (promise, wasReturned) => {
-				// Record stack before it gets lost in the promise chain.
-				const stack = getStack();
+				// Create an error object to record the stack before it gets lost in the promise chain.
+				const savedError = getErrorWithLongStackTrace();
 				// Handle "promise like" objects by casting to a real Promise.
 				const intermediate = Promise.resolve(promise).then(noop, error => { // eslint-disable-line promise/prefer-await-to-then
 					throw new AssertionError({
 						assertion: 'notThrowsAsync',
 						message,
-						actualStack: stack,
+						savedError,
 						values: [formatWithLabel(`${wasReturned ? 'Returned promise' : 'Promise'} rejected with:`, error)]
 					});
 				});

diff --git a/lib/serialize-error.js b/lib/serialize-error.js
@@ -51,7 +51,11 @@ function buildSource(source) {
 }
 
 function trySerializeError(err, shouldBeautifyStack) {
-	const stack = shouldBeautifyStack ? beautifyStack(err.stack) : err.stack;
+	let stack = err.savedError ? err.savedError.stack : err.stack;
+
+	if (shouldBeautifyStack) {
+		stack = beautifyStack(stack);
+	}
 
 	const retval = {
 		avaAssertionError: isAvaAssertionError(err),

diff --git a/lib/test.js b/lib/test.js
@@ -13,13 +13,12 @@ function formatErrorValue(label, error) {
 	return {label, formatted};
 }
 
-const captureStack = start => {
+const captureSavedError = () => {
 	const limitBefore = Error.stackTraceLimit;
 	Error.stackTraceLimit = 1;
-	const obj = {};
-	Error.captureStackTrace(obj, start);
+	const err = new Error();
 	Error.stackTraceLimit = limitBefore;
-	return obj.stack;
+	return err;
 };
 
 const testMap = new WeakMap();
@@ -60,7 +59,7 @@ class ExecutionContext extends assert.Assertions {
 		};
 
 		this.plan = count => {
-			test.plan(count, captureStack(test.plan));
+			test.plan(count, captureSavedError());
 		};
 
 		this.plan.skip = () => {};
@@ -72,7 +71,7 @@ class ExecutionContext extends assert.Assertions {
 
 	get end() {
 		const end = testMap.get(this).bindEndCallback();
-		const endFn = error => end(error, captureStack(endFn));
+		const endFn = error => end(error, captureSavedError());
 		return endFn;
 	}
 
@@ -143,15 +142,15 @@ class Test {
 
 	bindEndCallback() {
 		if (this.metadata.callback) {
-			return (error, stack) => {
-				this.endCallback(error, stack);
+			return (error, savedError) => {
+				this.endCallback(error, savedError);
 			};
 		}
 
 		throw new Error('`t.end()`` is not supported in this context. To use `t.end()` as a callback, you must use "callback mode" via `test.cb(testName, fn)`');
 	}
 
-	endCallback(error, stack) {
+	endCallback(error, savedError) {
 		if (this.calledEnd) {
 			this.saveFirstError(new Error('`t.end()` called more than once'));
 			return;
@@ -163,7 +162,7 @@ class Test {
 			this.saveFirstError(new assert.AssertionError({
 				actual: error,
 				message: 'Callback called with an error',
-				stack,
+				savedError,
 				values: [formatErrorValue('Callback called with an error:', error)]
 			}));
 		}
@@ -223,7 +222,7 @@ class Test {
 		}
 	}
 
-	plan(count, planStack) {
+	plan(count, planError) {
 		if (typeof count !== 'number') {
 			throw new TypeError('Expected a number');
 		}
@@ -232,7 +231,7 @@ class Test {
 
 		// In case the `planCount` doesn't match `assertCount, we need the stack of
 		// this function to throw with a useful stack.
-		this.planStack = planStack;
+		this.planError = planError;
 	}
 
 	timeout(ms) {
@@ -274,7 +273,7 @@ class Test {
 				assertion: 'plan',
 				message: `Planned for ${this.planCount} ${plur('assertion', this.planCount)}, but got ${this.assertCount}.`,
 				operator: '===',
-				stack: this.planStack
+				savedError: this.planError
 			}));
 		}
 	}
@@ -311,7 +310,7 @@ class Test {
 			fixedSource: {file: pending.file, line: pending.line},
 			improperUsage: true,
 			message: `Improper usage of \`t.${pending.assertion}()\` detected`,
-			stack: error instanceof Error && error.stack,
+			savedError: error instanceof Error && error,
 			values
 		}));
 		return true;
@@ -365,7 +364,7 @@ class Test {
 			if (!this.detectImproperThrows(result.error)) {
 				this.saveFirstError(new assert.AssertionError({
 					message: 'Error thrown in test',
-					stack: result.error instanceof Error && result.error.stack,
+					savedError: result.error instanceof Error && result.error,
 					values: [formatErrorValue('Error thrown in test:', result.error)]
 				}));
 			}
@@ -438,7 +437,7 @@ class Test {
 						if (!this.detectImproperThrows(error)) {
 							this.saveFirstError(new assert.AssertionError({
 								message: 'Rejected promise returned by test',
-								stack: error instanceof Error && error.stack,
+								savedError: error instanceof Error && error,
 								values: [formatErrorValue('Rejected promise returned by test. Reason:', error)]
 							}));
 						}