Add the check if `new` was used

[danielkcz]

I just run into issue when writing my Mocha tests with --check-leaks enabled. At some point I've got leak...

 Error: global leaks detected: jse_shortmsg, jse_summary, jse_cause, message

at Runner.checkGlobals (c:\Users\Fredy\AppData\Roaming\npm\node_modules\mocha\lib\runner.js:177:21)

It was kinda find to hard, but it happened when I created VError without new keyword, thus it created properties on global object. I consider this bad practice. When you are modifying this somehow, there should be a check if referencing to correct object...

Since number of arguments is variable, it might be easier (instead of trying to use apply and new keyword together) to change this...

var self = this;
if (!(self instanceof VError)) {
    self = Object.create( VError.prototype );
}
// rest of the constructor code uses `self`

[davepacheco]

FredyC: Sorry for the painful debugging experience. That sounds nasty.

I consider it bad practice to invoke a constructor function without "new" and assume it will "do the right thing", so I'm tempted to document explicitly that VError (as all constructors) should be invoked with "new", and have it fail an assertion if it's not. That said, these are by nature error paths, which tend to be less well exercised by tests. Plus, the JavaScript standard requires the Error function itself to work this way, so it probably does make sense to have the VError constructor do the same thing when invoked in a non-VError-object context.

[danielkcz]

I agree that calling constructor without new is bad practice, but sometimes it can happen by mistake. In such cases the constructor should be able to handle that, especially when modifying this directly. Thanks for reply and hopefully soon fix. I like VError very much so far :+1:

[davepacheco]

Published in v1.6.0.