Skip to content

feat(HttpError): use @netflix/nerror instead of verror #96

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 6, 2019
Merged

feat(HttpError): use @netflix/nerror instead of verror #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2024981
feat(HttpError): use @netflix/nerror instead of verror
May 6, 2019
f4d1c68
chore(package): update dependencies and elliminate vulnabirities
May 6, 2019
d42d83e
chore(package): drop 4.x and 6.x Node support
May 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
language: node_js
node_js:
- "4"
Copy link
Member

Choose a reason for hiding this comment

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

Is this a breaking change if we stop supporting 4/6?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, in my opinion it is

- "6"
- "8"
- "stable"
- '8'
- '10'
- "lts/*" # Active LTS release
- "node" # Latest stable release
after_success:
- make report-coverage
- make nsp

8 changes: 1 addition & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ ESLINT := $(NODE_BIN)/eslint
ISTANBUL := $(NODE_BIN)/istanbul
MOCHA := $(NODE_BIN)/mocha
_MOCHA := $(NODE_BIN)/_mocha
NSP := $(NODE_BIN)/nsp
UNLEASH := $(NODE_BIN)/unleash


Expand Down Expand Up @@ -77,13 +76,8 @@ lint: $(NODE_MODULES) ## Run lint and style checks
@$(ESLINT) $(ALL_FILES)


.PHONY: nsp
nsp: $(NODE_MODULES) $(YARN_LOCK) ## Check for dependency vulnerabilities
@$(NSP) check --preprocessor yarn


.PHONY: prepush
prepush: $(NODE_MODULES) lint test nsp ## Run all required tasks for a git push
prepush: $(NODE_MODULES) lint test ## Run all required tasks for a git push


.PHONY: test
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ restify-errors exports:

```js
var errors = require('restify-errors');
var verror = require('verror');
var nerror = require('@netflix/nerror');

var err = new errors.InternalServerError({
info: {
Expand Down
10 changes: 6 additions & 4 deletions lib/baseClasses/HttpError.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ var util = require('util');

// external modules
var assert = require('assert-plus');
var verror = require('verror');
var nerror = require('@netflix/nerror');

// internal files
var helpers = require('./../helpers');

var WError = nerror.WError;


/**
* Base HttpError class. inherits from WError.
Expand Down Expand Up @@ -39,7 +41,7 @@ function HttpError() {
// inherit from WError, call super first before doing anything else! WError
// will handle calling captureStackTrace, using arguments.callee to
// eliminate unnecessary stack frames.
verror.WError.apply(self, verrorArgs);
WError.apply(self, verrorArgs);

/**
* the http status code of the error.
Expand Down Expand Up @@ -102,7 +104,7 @@ function HttpError() {
self.toString = opts.toString;
}
}
util.inherits(HttpError, verror.WError);
util.inherits(HttpError, WError);

/**
* migration method to allow continued use of `.context` property that has now
Expand All @@ -112,7 +114,7 @@ util.inherits(HttpError, verror.WError);
Object.defineProperty(HttpError.prototype, 'context', {
get: function getContext() {
var self = this;
return verror.info(self);
return nerror.info(self);
}
});

Expand Down
4 changes: 2 additions & 2 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

var _ = require('lodash');
var assert = require('assert-plus');
var verror = require('verror');
var nerror = require('@netflix/nerror');

var bunyanSerializer = require('./serializer');
var helpers = require('./helpers');
Expand Down Expand Up @@ -70,7 +70,7 @@ function makeInstance(constructor, constructorOpt, args) {



module.exports = _.assign({}, httpErrors, restErrors, verror, {
module.exports = _.assign({}, httpErrors, restErrors, nerror, {
// export base classes
HttpError: HttpError,
RestError: RestError,
Expand Down
12 changes: 7 additions & 5 deletions lib/serializer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
// external modules
var assert = require('assert-plus');
var _ = require('lodash');
var verror = require('verror');
var nerror = require('@netflix/nerror');
var domain = require('domain');
var MultiError = nerror.MultiError;
var VError = nerror.VError;
var safeJsonStringify;

// try to require optional dependency
Expand Down Expand Up @@ -39,7 +41,7 @@ function ErrorSerializer(opts) {


/**
* loop through all errors() in a verror.MultiError and build a stack trace
* loop through all errors() in a MultiError and build a stack trace
* output.
* @private
* @method _getMultiErrorStack
Expand Down Expand Up @@ -160,7 +162,7 @@ function _getSerializedContext(err) {
}

// combine all fields into a pojo, and serialize
var allFields = _.assign({}, topLevelFields, err.context, verror.info(err));
var allFields = _.assign({}, topLevelFields, err.context, nerror.info(err));

if (!_.isEmpty(allFields)) {
ret = ' (' + serializeIntoEqualString(allFields) + ')';
Expand Down Expand Up @@ -191,8 +193,8 @@ ErrorSerializer.prototype._findKnownFields = function _findKnownFields() {
];

// make a verror and multierror and find expected fields
var verr = new verror.VError();
var multiErr = new verror.MultiError([ verr ]);
var verr = new VError();
var multiErr = new MultiError([ verr ]);
fields.push(_.keys(verr));
fields.push(_.keys(Object.getPrototypeOf(verr)));
fields.push(_.keys(multiErr));
Expand Down
18 changes: 8 additions & 10 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,21 @@
},
"devDependencies": {
"bunyan": "^1.8.10",
"chai": "^4.0.2",
"coveralls": "^3.0.0",
"eslint": "^4.1.1",
"chai": "^4.2.0",
Copy link
Member

Choose a reason for hiding this comment

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

In the future I'd probably do these dev deps in a separate PR to keep the commits easier to reason about.

"coveralls": "^3.0.3",
"eslint": "^5.16.0",
"istanbul": "^0.4.5",
"mkdirp": "^0.5.1",
"mocha": "^5.0.1",
"nsp": "^3.2.1",
"nsp-preprocessor-yarn": "^1.0.1",
"restify": "^6.3.4",
"restify-clients": "^1.5.0"
"mocha": "^6.1.4",
"restify": "^8.3.1",
"restify-clients": "^2.6.4"
},
"optionalDependencies": {
"safe-json-stringify": "^1.0.4"
},
"dependencies": {
"@netflix/nerror": "^1.0.0",
"assert-plus": "^1.0.0",
"lodash": "^4.17.4",
"verror": "^1.10.0"
"lodash": "^4.17.11"
}
}
18 changes: 10 additions & 8 deletions test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ var bunyan = require('bunyan');
var _ = require('lodash');
var restify = require('restify');
var restifyClients = require('restify-clients');
var verror = require('verror');
var WError = verror.WError;
var nerror = require('@netflix/nerror');
var WError = nerror.WError;
var VError = nerror.VError;
var MultiError = nerror.MultiError;

// internal
var helpers = require('../lib/helpers');
Expand Down Expand Up @@ -113,7 +115,7 @@ describe('restify-errors node module.', function() {
info: info
}, 'boom');

assert.deepEqual(verror.info(myErr), info);
assert.deepEqual(nerror.info(myErr), info);
assert.deepEqual(restifyErrors.info(myErr), info);
assert.deepEqual(myErr.context, info);
});
Expand Down Expand Up @@ -893,7 +895,7 @@ describe('restify-errors node module.', function() {

it('should serialize a VError with info', function() {

var err = new verror.VError({
var err = new VError({
name: 'VErrorInfo',
info: {
foo: 'qux',
Expand All @@ -916,23 +918,23 @@ describe('restify-errors node module.', function() {
baz: 1
}
}, 'ISE');
var err3 = new verror.VError({
var err3 = new VError({
name: 'VErrorInfo',
cause: err1,
info: {
foo: 'qux',
baz: 2
}
}, 'this is a verror with info');
var multiError = new verror.MultiError([ err1, err2, err3 ]);
var multiError = new MultiError([ err1, err2, err3 ]);

assert.doesNotThrow(function() {
logger.error(multiError, 'MultiError');
});
});

it('should not serialize arbitrary fields on VError', function() {
var err1 = new verror.VError({
var err1 = new VError({
name: 'VErrorInfo',
info: {
espresso: 'ristretto'
Expand Down Expand Up @@ -971,7 +973,7 @@ describe('restify-errors node module.', function() {
var serializer = restifyErrors.bunyanSerializer.create({
topLevelFields: true
});
var err1 = new verror.VError({
var err1 = new VError({
name: 'VErrorInfo',
info: {
espresso: 'ristretto'
Expand Down