Permalink
Browse files

Merge pull request #884 from meeber/fix-stack

Remove proxy frames from stack traces and improve docs/tests
  • Loading branch information...
2 parents c9bebe4 + 0c09836 commit 877dde88795be8af77f0893275a3fcc64e690733 @keithamus keithamus committed on GitHub Jan 3, 2017
View
@@ -26,11 +26,34 @@ module.exports = function (_chai, util) {
*
* Creates object for chaining.
*
+ * `Assertion` objects contain metadata in the form of flags. Three flags can
+ * be assigned during instantiation by passing arguments to this constructor:
+ *
+ * - `object`: This flag contains the target of the assertion. For example, in
+ * the assertion `expect(numKittens).to.equal(7);`, the `object` flag will
+ * contain `numKittens` so that the `equal` assertion can reference it when
+ * needed.
+ *
+ * - `message`: This flag contains an optional custom error message to be
+ * prepended to the error message that's generated by the assertion when it
+ * fails.
+ *
+ * - `ssfi`: This flag stands for "start stack function indicator". It
+ * contains a function reference that serves as the starting point for
+ * removing frames from the stack trace of the error that's created by the
+ * assertion when it fails. The goal is to provide a cleaner stack trace to
+ * end users by removing Chai's internal functions. Note that it only works
+ * in environments that support `Error.captureStackTrace`, and only when
+ * `Chai.config.includeStack` hasn't been set to `false`.
+ *
+ * @param {Mixed} obj target of the assertion
+ * @param {String} msg (optional) custom error message
+ * @param {Function} stack (optional) starting point for removing stack frames
* @api private
*/
- function Assertion (obj, msg, stack) {
- flag(this, 'ssfi', stack || Assertion);
+ function Assertion (obj, msg, ssfi) {
+ flag(this, 'ssfi', ssfi || Assertion);
flag(this, 'object', obj);
flag(this, 'message', msg);
@@ -59,7 +59,7 @@ var call = Function.prototype.call,
* @api public
*/
-module.exports = function (ctx, name, method, chainingBehavior) {
+module.exports = function addChainableMethod(ctx, name, method, chainingBehavior) {
if (typeof chainingBehavior !== 'function') {
chainingBehavior = function () { };
}
@@ -76,13 +76,18 @@ module.exports = function (ctx, name, method, chainingBehavior) {
ctx.__methods[name] = chainableBehavior;
Object.defineProperty(ctx, name,
- { get: function () {
+ { get: function chainableMethodGetter() {
chainableBehavior.chainingBehavior.call(this);
- var assert = function assert() {
- var old_ssfi = flag(this, 'ssfi');
- if (old_ssfi)
- flag(this, 'ssfi', assert);
+ var chainableMethodWrapper = function () {
+ // Use this chainable method wrapper as the starting point for
+ // removing implementation frames from the stack trace of a failed
+ // assertion. Note that this is the correct starting point even if
+ // this assertion has been overwritten since overwriting a chainable
+ // method merely replaces the saved methods in `ctx.__methods` instead
+ // of completely replacing the overwritten assertion.
+ flag(this, 'ssfi', chainableMethodWrapper);
+
var result = chainableBehavior.method.apply(this, arguments);
if (result !== undefined) {
@@ -94,12 +99,12 @@ module.exports = function (ctx, name, method, chainingBehavior) {
return newAssertion;
};
- addLengthGuard(assert, name, true);
+ addLengthGuard(chainableMethodWrapper, name, true);
// Use `__proto__` if available
if (hasProtoSupport) {
// Inherit all properties from the object by replacing the `Function` prototype
- var prototype = assert.__proto__ = Object.create(this);
+ var prototype = chainableMethodWrapper.__proto__ = Object.create(this);
// Restore the `call` and `apply` methods from `Function`
prototype.call = call;
prototype.apply = apply;
@@ -110,13 +115,13 @@ module.exports = function (ctx, name, method, chainingBehavior) {
asserterNames.forEach(function (asserterName) {
if (!excludeNames.test(asserterName)) {
var pd = Object.getOwnPropertyDescriptor(ctx, asserterName);
- Object.defineProperty(assert, asserterName, pd);
+ Object.defineProperty(chainableMethodWrapper, asserterName, pd);
}
});
}
- transferFlags(this, assert);
- return proxify(assert);
+ transferFlags(this, chainableMethodWrapper);
+ return proxify(chainableMethodWrapper);
}
, configurable: true
});
@@ -36,12 +36,19 @@ var transferFlags = require('./transferFlags');
* @api public
*/
-module.exports = function (ctx, name, method) {
- var fn = function () {
- var keep_ssfi = flag(this, 'keep_ssfi');
- var old_ssfi = flag(this, 'ssfi');
- if (!keep_ssfi && old_ssfi)
- flag(this, 'ssfi', fn);
+module.exports = function addMethod(ctx, name, method) {
+ var methodWrapper = function () {
+ // If this assertion hasn't been overwritten, then use this method wrapper
+ // as the starting point for removing implementation frames from the stack
+ // trace of a failed assertion.
+ //
+ // Note: If this assertion has been overwritten, and thus the `keep_ssfi`
+ // flag is set, then the overwriting method wrapper is used as the starting
+ // point instead. This prevents the overwriting method wrapper from showing
+ // up in the stack trace since it's invoked before this method wrapper.
+ if (!flag(this, 'keep_ssfi')) {
+ flag(this, 'ssfi', methodWrapper);
+ }
var result = method.apply(this, arguments);
if (result !== undefined)
@@ -52,6 +59,6 @@ module.exports = function (ctx, name, method) {
return newAssertion;
};
- addLengthGuard(fn, name, false);
- ctx[name] = proxify(fn, name);
+ addLengthGuard(methodWrapper, name, false);
+ ctx[name] = proxify(methodWrapper, name);
};
@@ -6,6 +6,7 @@
var chai = require('../../chai');
var flag = require('./flag');
+var isProxyEnabled = require('./isProxyEnabled');
var transferFlags = require('./transferFlags');
/**
@@ -34,15 +35,31 @@ var transferFlags = require('./transferFlags');
* @api public
*/
-module.exports = function (ctx, name, getter) {
+module.exports = function addProperty(ctx, name, getter) {
getter = getter === undefined ? new Function() : getter;
Object.defineProperty(ctx, name,
- { get: function addProperty() {
- var keep_ssfi = flag(this, 'keep_ssfi');
- var old_ssfi = flag(this, 'ssfi');
- if (!keep_ssfi && old_ssfi)
- flag(this, 'ssfi', addProperty);
+ { get: function propertyGetter() {
+ // If proxy protection is disabled and this assertion hasn't been
+ // overwritten, then use this property getter as the starting point for
+ // removing implementation frames from the stack trace of a failed
+ // assertion.
+ //
+ // Notes:
+ //
+ // - If proxy protection is enabled, then the proxy getter is used as
+ // the starting point instead. This prevents the proxy getter from
+ // showing up in the stack trace since it's invoked before this
+ // property getter.
+ //
+ // - If proxy protection is disabled but this assertion has been
+ // overwritten, and thus the `keep_ssfi` flag is set, then the
+ // overwriting property getter is used as the starting point instead.
+ // This prevents the overwriting property getter from showing up in
+ // the stack trace since it's invoked before this property getter.
+ if (!isProxyEnabled() && !flag(this, 'keep_ssfi')) {
+ flag(this, 'ssfi', propertyGetter);
+ }
var result = getter.call(this);
if (result !== undefined)
@@ -26,6 +26,6 @@ var inspect = require('./inspect');
* @api public
*/
-module.exports = function (a, b) {
+module.exports = function compareByInspect(a, b) {
return inspect(a) < inspect(b) ? -1 : 1;
};
@@ -22,7 +22,7 @@ var AssertionError = require('assertion-error');
var flag = require('./flag');
var type = require('type-detect');
-module.exports = function (obj, types) {
+module.exports = function expectTypes(obj, types) {
obj = flag(obj, 'object');
types = types.map(function (t) { return t.toLowerCase(); });
types.sort();
@@ -23,7 +23,7 @@
* @api private
*/
-module.exports = function (obj, key, value) {
+module.exports = function flag(obj, key, value) {
var flags = obj.__flags || (obj.__flags = Object.create(null));
if (arguments.length === 3) {
flags[key] = value;
@@ -15,6 +15,6 @@
* @name getActual
*/
-module.exports = function (obj, args) {
+module.exports = function getActual(obj, args) {
return args.length > 4 ? args[4] : obj._obj;
};
@@ -32,7 +32,7 @@ var flag = require('./flag')
* @api public
*/
-module.exports = function (obj, args) {
+module.exports = function getMessage(obj, args) {
var negate = flag(obj, 'negate')
, val = flag(obj, 'object')
, expected = args[3]
@@ -154,12 +154,18 @@ exports.checkError = require('check-error');
exports.proxify = require('./proxify');
/*!
- * Proxify util
+ * addLengthGuard util
*/
exports.addLengthGuard = require('./addLengthGuard');
/*!
+ * isProxyEnabled helper
+ */
+
+exports.isProxyEnabled = require('./isProxyEnabled');
+
+/*!
* isNaN method
*/
@@ -0,0 +1,24 @@
+var config = require('../config');
+
+/*!
+ * Chai - isProxyEnabled helper
+ * Copyright(c) 2012-2014 Jake Luer <jake@alogicalparadox.com>
+ * MIT Licensed
+ */
+
+/**
+ * # isProxyEnabled()
+ *
+ * Helper function to check if Chai's proxy protection feature is enabled. If
+ * proxies are unsupported or disabled via the user's Chai config, then return
+ * false. Otherwise, return true.
+ *
+ * @namespace Utils
+ * @name isProxyEnabled
+ */
+
+module.exports = function isProxyEnabled() {
+ return config.useProxy &&
+ typeof Proxy !== 'undefined' &&
+ typeof Reflect !== 'undefined';
+};
@@ -24,7 +24,7 @@ var config = require('../config');
* @api public
*/
-module.exports = function (obj) {
+module.exports = function objDisplay(obj) {
var str = inspect(obj)
, type = Object.prototype.toString.call(obj);
@@ -40,11 +40,11 @@ var transferFlags = require('./transferFlags');
* @api public
*/
-module.exports = function (ctx, name, method, chainingBehavior) {
+module.exports = function overwriteChainableMethod(ctx, name, method, chainingBehavior) {
var chainableBehavior = ctx.__methods[name];
var _chainingBehavior = chainableBehavior.chainingBehavior;
- chainableBehavior.chainingBehavior = function () {
+ chainableBehavior.chainingBehavior = function overwritingChainableMethodGetter() {
var result = chainingBehavior(_chainingBehavior).call(this);
if (result !== undefined) {
return result;
@@ -56,7 +56,7 @@ module.exports = function (ctx, name, method, chainingBehavior) {
};
var _method = chainableBehavior.method;
- chainableBehavior.method = function () {
+ chainableBehavior.method = function overwritingChainableMethodWrapper() {
var result = method(_method).apply(this, arguments);
if (result !== undefined) {
return result;
@@ -44,7 +44,7 @@ var transferFlags = require('./transferFlags');
* @api public
*/
-module.exports = function (ctx, name, method) {
+module.exports = function overwriteMethod(ctx, name, method) {
var _method = ctx[name]
, _super = function () {
throw new Error(name + ' is not a function');
@@ -53,12 +53,24 @@ module.exports = function (ctx, name, method) {
if (_method && 'function' === typeof _method)
_super = _method;
- var fn = function () {
- var keep_ssfi = flag(this, 'keep_ssfi');
- var old_ssfi = flag(this, 'ssfi');
- if (!keep_ssfi && old_ssfi)
- flag(this, 'ssfi', fn);
+ var overwritingMethodWrapper = function () {
+ // If proxy protection is disabled and this overwriting assertion hasn't
+ // been overwritten again by yet another assertion, then use this method
+ // wrapper as the starting point for removing implementation frames from the
+ // stack trace of a failed assertion.
+ //
+ // Note: If this assertion has been overwritten, and thus the `keep_ssfi`
+ // flag is set, then the overwriting method wrapper is used as the starting
+ // point instead. This prevents the overwriting method wrapper from showing
+ // up in the stack trace since it's invoked before this method wrapper.
+ if (!flag(this, 'keep_ssfi')) {
+ flag(this, 'ssfi', overwritingMethodWrapper);
+ }
+ // The `keep_ssfi` flag is set so that if this assertion ends up calling
+ // the overwritten assertion, then the overwritten assertion doesn't attempt
+ // to use itself as the starting point for removing implementation frames
+ // from the stack trace of a failed assertion.
flag(this, 'keep_ssfi', true);
var result = method(_super).apply(this, arguments);
flag(this, 'keep_ssfi', false);
@@ -72,6 +84,6 @@ module.exports = function (ctx, name, method) {
return newAssertion;
}
- addLengthGuard(fn, name, false);
- ctx[name] = proxify(fn, name);
+ addLengthGuard(overwritingMethodWrapper, name, false);
+ ctx[name] = proxify(overwritingMethodWrapper, name);
};
Oops, something went wrong.

0 comments on commit 877dde8

Please sign in to comment.