Skip to content

Commit

Permalink
[Fix] throws: only reassign “message” when it is not already non-en…
Browse files Browse the repository at this point in the history
…umerable.

Fixes #320.
  • Loading branch information
ljharb committed Sep 10, 2016
1 parent 918e217 commit c1b483c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 7 deletions.
10 changes: 7 additions & 3 deletions lib/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ var inherits = require('inherits');
var EventEmitter = require('events').EventEmitter;
var has = require('has');
var trim = require('string.prototype.trim');
var bind = require('function-bind');
var isEnumerable = bind.call(Function.call, Object.prototype.propertyIsEnumerable);

module.exports = Test;

Expand Down Expand Up @@ -444,9 +446,11 @@ Test.prototype['throws'] = function (fn, expected, msg, extra) {
fn();
} catch (err) {
caught = { error : err };
var message = err.message;
delete err.message;
err.message = message;
if (!isEnumerable(err, 'message') || !has(err, 'message')) {
var message = err.message;
delete err.message;
err.message = message;
}
}

var passed = caught;
Expand Down
25 changes: 21 additions & 4 deletions test/throws.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,14 @@ tap.test('failures', function (tt) {
+ ' operator: throws\n'
+ ' expected: undefined\n'
+ ' actual: undefined\n'
+ ' ...\n\n'
+ '1..9\n'
+ '# tests 9\n'
+ '# pass 0\n'
+ ' ...\n'
+ '# custom error messages\n'
+ 'ok 10 "message" is enumerable\n'
+ "ok 11 { custom: 'error', message: 'message' }\n"
+ 'ok 12 getter is still the same\n'
+ '\n1..12\n'
+ '# tests 12\n'
+ '# pass 3\n'
+ '# fail 9\n'
);
}));
Expand All @@ -117,4 +121,17 @@ tap.test('failures', function (tt) {
t.plan(1);
t.throws(function () {});
});

test('custom error messages', function (t) {
t.plan(3);
var getter = function () { return 'message'; };
var messageGetterError = Object.defineProperty(
{ custom: 'error' },
'message',
{ configurable: true, enumerable: true, get: getter }
);
t.equal(Object.prototype.propertyIsEnumerable.call(messageGetterError, 'message'), true, '"message" is enumerable');
t.throws(function () { throw messageGetterError; }, "{ custom: 'error', message: 'message' }");
t.equal(Object.getOwnPropertyDescriptor(messageGetterError, 'message').get, getter, 'getter is still the same');
});
});

4 comments on commit c1b483c

@dcousens
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance of a release for this?

@ljharb
Copy link
Collaborator Author

@ljharb ljharb commented on c1b483c Sep 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dcousens sure, sorry about the delay. v4.6.1 published.

@dcousens
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ljharb!

@dcousens
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb the effect is real... tests that took >20seconds now take 1-2 seconds! Thanks @ljharb and yay for strict mode

Please sign in to comment.