Error.prototype.toJSON #5

Merged
merged 4 commits into from Jun 2, 2012

Conversation

Projects
None yet
4 participants
Owner

indexzero commented May 15, 2012

Fix inconsistent behavior in newer versions of V8: https://gist.github.com/1553662

lib/errs.js
+// onto `target` and returns the resulting object.
+//
+function mixin(target) {
+ Array.prototype.slice.call(arguments, 1).forEach(function (o) {
@mmalecki

mmalecki May 15, 2012

Contributor

This might end up being a hot path, maybe we should iterate through arguments instead (as per this blog post)?

@indexzero

indexzero May 15, 2012

Owner

Good point. I didn't know about that. Fixed in 02b713b

Contributor

mmalecki commented May 15, 2012

+1.

indexzero added a commit that referenced this pull request Jun 2, 2012

@indexzero indexzero merged commit 17acfff into master Jun 2, 2012

Contributor

conorgil commented Nov 13, 2013

You are mucking with the global namespace by modifying Error.prototype. This will have unintended consequences in other people's code bases.

We are using nano-js (by nuno), which uses errs. In our code base, we have a custom error object (BaseError) which inherits from Error and overrides toJSON

BaseError.prototype.toJSON = function() {
    console.log('foo');
    return {};
}

Because we were using nano and that uses errs, overriding the method did not work.

var x = new BaseError();
JSON.stringify(x);

That should log "foo" to the console and return {}. Instead, the method defined in errs gets called.

+1 on @conorgil's comment. Any mucking of global namespace is bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment