Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

info: comparing verror and @Netflix/nerror #75

Closed
trentm opened this issue Oct 24, 2019 · 4 comments
Closed

info: comparing verror and @Netflix/nerror #75

trentm opened this issue Oct 24, 2019 · 4 comments

Comments

@trentm
Copy link
Contributor

trentm commented Oct 24, 2019

(It is perhaps a little odd that I opened this as an "issue". I wanted to record this somewhere associated with the project, so here it is. Because there isn't a thing to complete/resolve here, I propose to close this issue soon.)

Comparing verror and @Netflix/nerror

A while back Netflix did a "friendly" fork of https://github/joyent/node-verror to https://github.com/Netflix/nerror. Its readme includes:

We hope in the future after the initial development period we can seek convergence between the two projects.

I've been a user of verror for years as an employee of Joyent and have been around for its entire development and have made some small contributions to it. It's primary author, davepacheco, no longer works at Joyent. As part of moving Joyent code reviews from a Gerrit instance to GitHub PRs I was taking a look at some Gerrit CRs for node-verror. In particular there are two GH issues, two PRs, and three CRs (some closed, some not) regarding VError usage of printf that led to options.skipPrintf and PError support in @Netflix/nerror and this PR for the same in verror. See #63 for current discussion on that.

So how do these two modules compare?

nerror has not broken compatibility with verror

That's good! If I run nerror's test suite against verror/lib/verror.js it passes after I comment out the test cases for new functionality added to nerror.

That "new functionality" is:

  • VError.isVError
  • the skipPrintf and PError support that started this
  • instance scoped <verror instance>.info() method
  • <verror instance>.assignInfo() method
  • slighly different, but better assertion error messages when an element passed to errorFromList([...]) is not an Error class

verror tests pass using nerror/lib/verror.js

This is good too. If I run verror's test suite against nerror/lib/verror.js it works after these trivial changes:

diff --git a/package.json b/package.json
index 763a64f..4011511 100644
--- a/package.json
+++ b/package.json
@@ -10,7 +10,8 @@
        "dependencies": {
                "assert-plus": "^1.0.0",
                "core-util-is": "1.0.2",
-               "extsprintf": "^1.2.0"
+               "extsprintf": "^1.2.0",
+               "lodash": "^4.17.15"
        },
        "engines": [
                "node >=0.6.0"
diff --git a/test/tst.multierror.js b/test/tst.multierror.js
index a63125d..5453bc0 100644
--- a/test/tst.multierror.js
+++ b/test/tst.multierror.js
@@ -95,11 +95,11 @@ function main()
        /* errorForEach */
        mod_assert.throws(function () {
                console.error(errorForEach());
-       }, /^AssertionError.*: err must be an Error$/);
+       }, /^AssertionError.*: err must be an Error but got .*$/);

        mod_assert.throws(function () {
                console.error(errorForEach(null));
-       }, /^AssertionError.*: err must be an Error$/);
+       }, /^AssertionError.*: err must be an Error but got .*$/);

        mod_assert.throws(function () {
                console.error(errorForEach(err1));
@@ -111,7 +111,7 @@ function main()

        mod_assert.throws(function () {
                console.error(errorForEach({}, function () {}));
-       }, /^AssertionError.*: err must be an Error$/);
+       }, /^AssertionError.*: err must be an Error but got .*$/);

        accum = [];
        doAccum = function (e) { accum.push(e); };

In other words:

  • add the new lodash dep
  • allow for a better assertion error message

How does nerror differ?

These are my opinions.

Pros:

  • New features based on real usage at Netflix (at least I assume). See the list above.
  • Modernized: eslint, chai (though personally I'm a node-tap fan currently :), coverage via nyc, prettier, TypeScript support (though I don't know TypeScript myself), travis.

Cons:

  • (minor) For a set of Joyent services, compat with node 0.10 is still required. NError supports only node v8 at up, which isn't unreasonable.

Questions:

  • Is the lodash dep (4.8M installed) overkill for lodash.isError() vs core-util-is.isError? Actually perhaps this could be changed to lodash.iserror (16K installed)?
  • Is nerror better maintained? Recently clearly yes. Hard to predict the future. Restify (and other libs?) have moved to using @netflix/nerror, FWIW.
@trentm trentm changed the title discussing/comparing verror and @Netflix/nerror info: comparing verror and @Netflix/nerror Oct 24, 2019
@trentm
Copy link
Contributor Author

trentm commented Oct 28, 2019

I'm closing this. It was informational.

@trentm trentm closed this as completed Oct 28, 2019
@richardscarrott
Copy link

@trentm I absolutely love VError and I'm surprised it's not more commonly used. As you've been around since the beginning (and at the risk of sounding silly?), it's alway bothered me that I don't know what the 'V' stands for, do you know?

@kayomarz
Copy link

kayomarz commented Jun 22, 2020 via email

@davepacheco
Copy link
Contributor

davepacheco commented Jun 24, 2020

@richardscarrott The story of the name is a little silly: the first purpose for VError was to provide a printf-like format string for constructing errors. (Causes and informational properties came later.) In C, a bunch of the printf-related functions use v in the name (e.g., vsnprintf) to tell you that they take C-style varargs. As I recall, that's all there was to it -- I picked the v prefix because it reminded me of all the printf-like formatting functions with variable arguments. (The reason I say it's a little silly is that the C functions that look like printf all don't have a v in the name -- the v really refers to a very C-specific way of processing varargs.)

@richardscarrott @kayomarz It's very gratifying to hear this has been helpful for you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants