diff --git a/.env.example b/.env.example index a02f6dc..359d3d8 100644 --- a/.env.example +++ b/.env.example @@ -1,6 +1,7 @@ ## Shared ## NODE_ENV=development LOG_LEVEL=info +ALWAYS_INCLUDE_ERROR_STACKS=false ## Auth Microservice ## JWT_SECRET=0a6b944d-d2fb-46fc-a85e-0295c986cd9f diff --git a/.env.test b/.env.test index 74ebcc3..8ceb1fb 100644 --- a/.env.test +++ b/.env.test @@ -1,5 +1,6 @@ NODE_ENV=test LOG_LEVEL=debug +ALWAYS_INCLUDE_ERROR_STACKS=false JWT_MODE=hmac JWT_SECRET=0a6b944d-d2fb-46fc-a85e-0295c986cd9f diff --git a/CHANGELOG.md b/CHANGELOG.md index 62e661e..452e522 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,17 @@ # Changelog ## [Unreleased] +### Added +- ENV `ALWAYS_INCLUDE_ERROR_STACKS` to include full error stack, including any causal errors. + * Use with caution in production, because the full stack could leak sensitive information. + ### Fixed - `auth.controller.js:login()` no longer throws duplicate errors for incorrect username or password - Use error handling middlware `winstonInstance.info(err)` * Previously, `winstonInstance.errorLogger()` always threw a `TypeError`, meaning the underlying error didn't actually show in the logs - +- `class:APIError` supports full stack traces, including causal errors. +- Update `npm:winston` to `^3.2.1` and `npm:winston-json-formatter` to `^0.10.0` for logging bug fixes and formatting features (see https://github.com/amida-tech/winston-json-formatter/pull/5). + * Logs "operational errors" at the `warn` level and "programmer errors" at the `error` level. ## [2.7.0] -- 2019-02-04 ### Added diff --git a/README.md b/README.md index 9980b4b..d3a602c 100644 --- a/README.md +++ b/README.md @@ -334,6 +334,12 @@ A description of what the variable is or does. - Valid values are [winston](https://github.com/winstonjs/winston) logging levels (`error`, `warn`, etc.). +##### `ALWAYS_INCLUDE_ERROR_STACKS` [`false`] + +The `APIError` class acts like a call-stackless "operational error" (https://www.joyent.com/node-js/production/design/errors) when called without a causal error as the first argument. +- When `false`, `APIError`s without a causal error will not have call stacks. +- When `true`, all `APIError`s will have call stacks. + ##### `JWT_SECRET` (Required) First, see description of `AUTH_SERVICE_JWT_MODE`. When `AUTH_SERVICE_JWT_MODE=hmac`, this is the shared secret between this service an all services using this service for authentication. Therefore, all other such service must set their `JWT_SECRET` to match this value. diff --git a/package.json b/package.json index 2a0c258..4ccbf34 100644 --- a/package.json +++ b/package.json @@ -63,8 +63,8 @@ "sequelize": "^4.40.0", "swagger-stats": "^0.95.6", "uuid": "3.1.0", - "winston": "^3.0.0", - "winston-json-formatter": "^0.9.2" + "winston": "^3.2.1", + "winston-json-formatter": "^0.10.0" }, "devDependencies": { "@babel/cli": "^7.1.5", diff --git a/src/config/config.js b/src/config/config.js index 9d3bbd0..24f6d44 100644 --- a/src/config/config.js +++ b/src/config/config.js @@ -16,6 +16,8 @@ const envVarsSchema = Joi.object({ .default('production'), LOG_LEVEL: Joi.string() .default('info'), + ALWAYS_INCLUDE_ERROR_STACKS: Joi.bool() + .default(false), AUTH_SERVICE_PORT: Joi.number() .default(4000), AUTH_SERVICE_PUBLIC_REGISTRATION: Joi.bool() @@ -127,9 +129,10 @@ if (error) { throw new Error(`Config validation error: ${error.message}`); } -module.exports = { +const config = { env: envVars.NODE_ENV, logLevel: envVars.LOG_LEVEL, + alwaysIncludeErrorStacks: envVars.ALWAYS_INCLUDE_ERROR_STACKS, port: envVars.AUTH_SERVICE_PORT, publicRegistration: envVars.AUTH_SERVICE_PUBLIC_REGISTRATION, registrarScopes: envVars.AUTH_SERVICE_REGISTRAR_SCOPES, @@ -169,3 +172,5 @@ module.exports = { scopes: ['admin'], }, }; + +module.exports = config; diff --git a/src/config/express.js b/src/config/express.js index 3861c5f..ebf1d56 100644 --- a/src/config/express.js +++ b/src/config/express.js @@ -68,13 +68,18 @@ app.use('/api', routes); app.use((err, req, res, next) => { if (err instanceof Sequelize.ValidationError) { const status = err.errors[0].type === 'unique violation' ? httpStatus.CONFLICT : httpStatus.BAD_REQUEST; - const unifiedErrors = JSON.stringify(err.errors); - const error = new APIError(unifiedErrors, 'GENERIC_ERROR', status, true); + // err.errors[0].message has the most useful information from Sequelize, so we want it to + // end up in the nested message of the APIError. However, it could have PHI or PII, so it + // must be within the causal error message and must not be passed directly into APIError's + // message argument. + // eslint-disable-next-line no-param-reassign + err.message = `${err.message}: ${err.errors[0].message}`; + const error = new APIError(err, 'Sequelize Validation Error', 'GENERIC_ERROR', status, { isOperational: true }); return next(error); } else if (err instanceof expressValidation.ValidationError) { // validation error contains errors which is an array of error each containing message[] const unifiedErrorMessage = err.errors.map(error => error.messages.join('. ')).join(' and '); - const error = new APIError(unifiedErrorMessage, 'GENERIC_ERROR', err.status, true); + const error = new APIError(unifiedErrorMessage, 'GENERIC_ERROR', err.status); return next(error); } return next(err); @@ -82,14 +87,19 @@ app.use((err, req, res, next) => { // catch 404 and forward to error handler app.use((req, res, next) => { - const err = new APIError('API not found', 'UNKNOWN_API', httpStatus.NOT_FOUND); + const err = new APIError('Not Found', 'NOT_FOUND', httpStatus.NOT_FOUND, { isPublic: true }); return next(err); }); // Log errors. if (config.env !== 'test') { app.use((err, req, res, next) => { - winstonInstance.warn(err); + if (err.isOperational) { + winstonInstance.warn(err); + } else { + winstonInstance.error(err); + } + return next(err); }); } @@ -97,9 +107,9 @@ if (config.env !== 'test') { // error handler, send stacktrace only during development app.use((err, req, res, next) => // eslint-disable-line no-unused-vars res.status(err.status || 500).json({ - code: err.isPublic ? err.message.code : 'UNKNOWN_ERROR', + code: err.isPublic ? err.code : 'UNKNOWN_ERROR', status: 'ERROR', - message: err.isPublic ? err.message.message : httpStatus[err.status], + message: err.isPublic ? err.message : httpStatus[err.status], stack: config.env === 'development' ? err.stack : {}, }) ); diff --git a/src/config/passport.js b/src/config/passport.js index 79f9635..b7dcb99 100644 --- a/src/config/passport.js +++ b/src/config/passport.js @@ -47,7 +47,7 @@ module.exports = (passport) => { } }) .spread((user) => { if (user !== null) return done(null, user); - const err = new APIError('New facebook user not created', 'FACEBOOK_CREATION_ERROR', httpStatus.INTERNAL_SERVER_ERROR, true); + const err = new APIError('New facebook user not created', 'FACEBOOK_CREATION_ERROR', httpStatus.INTERNAL_SERVER_ERROR); return done(err); }); }); diff --git a/src/controllers/auth.controller.js b/src/controllers/auth.controller.js index d01c4c9..ebedaa5 100644 --- a/src/controllers/auth.controller.js +++ b/src/controllers/auth.controller.js @@ -28,7 +28,7 @@ function login(req, res, next) { const validUserAndPassword = !_.isNull(user) && user.testPassword(params.password); if (!validUserAndPassword) { - const err = new APIError('Incorrect username or password', 'INCORRECT_USERNAME_OR_PASSWORD', httpStatus.NOT_FOUND, true); + const err = new APIError('Incorrect username or password', 'INCORRECT_USERNAME_OR_PASSWORD', httpStatus.UNAUTHORIZED); return next(err); } @@ -82,7 +82,7 @@ function submitRefreshToken(req, res, next) { .findOne({ where: { token: params.refreshToken, userId: userResult.id } }) .then((tokenResult) => { if (_.isNull(tokenResult)) { - const err = new APIError('Refresh token not found', 'MISSING_REFRESH_TOKEN', httpStatus.NOT_FOUND, true); + const err = new APIError('Refresh token not found', 'MISSING_REFRESH_TOKEN', httpStatus.NOT_FOUND); return next(err); } const userInfo = { @@ -122,7 +122,7 @@ function rejectRefreshToken(req, res, next) { return RefreshToken.findOne({ where: { token: params.refreshToken } }) .then((tokenResult) => { if (_.isNull(tokenResult)) { - const err = new APIError('Refresh token not found', 'MISSING_REFRESH_TOKEN', httpStatus.NOT_FOUND, true); + const err = new APIError('Refresh token not found', 'MISSING_REFRESH_TOKEN', httpStatus.NOT_FOUND); return next(err); } // delete the refresh token @@ -146,7 +146,7 @@ function updatePassword(req, res, next) { const params = _.pick(req.body, 'oldPassword', 'password'); if (!user.testPassword(params.oldPassword)) { - const err = new APIError('Incorrect password', 'INCORRECT_PASSWORD', httpStatus.FORBIDDEN, true); + const err = new APIError('Incorrect password', 'INCORRECT_PASSWORD', httpStatus.FORBIDDEN); return next(err); } @@ -171,7 +171,7 @@ function resetToken(req, res, next) { const email = _.get(req, 'body.email'); const resetPageUrl = _.get(req, 'body.resetPageUrl'); if (!email) { - const err = new APIError('Invalid email', 'INVALID_EMAIL', httpStatus.BAD_REQUEST, true); + const err = new APIError('Invalid email', 'INVALID_EMAIL', httpStatus.BAD_REQUEST); return next(err); } return User.resetPasswordToken(email, 3600) diff --git a/src/controllers/user.controller.js b/src/controllers/user.controller.js index 4561a0a..61f85e1 100644 --- a/src/controllers/user.controller.js +++ b/src/controllers/user.controller.js @@ -22,7 +22,7 @@ function load(req, res, next, id) { User.findById(id) .then((user) => { if (!user) { - const e = new APIError('User does not exist', 'UNKNOWN_USERNAME', httpStatus.NOT_FOUND, true); + const e = new APIError('User does not exist', 'UNKNOWN_USERNAME', httpStatus.NOT_FOUND); return next(e); } req.user = user; // eslint-disable-line no-param-reassign @@ -44,7 +44,7 @@ function get(req, res, next) { User.findById(req.params.userId) .then((user) => { if (req.user.username !== user.username && !req.user.isAdmin()) { - const e = new APIError('Cannot get another user\'s information', 'UNAUTHORIZED_REQUEST', httpStatus.FORBIDDEN, true); + const e = new APIError('Cannot get another user\'s information', 'UNAUTHORIZED_REQUEST', httpStatus.FORBIDDEN); return next(e); } return res.json(user.getBasicUserInfo()); @@ -70,7 +70,7 @@ function getByEmail(req, res, next) { return next(e); } if (req.user.username !== user.username && !req.user.isAdmin()) { - const e = new APIError('Cannot get another user\'s information', 'UNAUTHORIZED_REQUEST', httpStatus.FORBIDDEN, true); + const e = new APIError('Cannot get another user\'s information', 'UNAUTHORIZED_REQUEST', httpStatus.FORBIDDEN); return next(e); } return res.json(user.getBasicUserInfo()); @@ -104,7 +104,7 @@ function create(req, res, next) { // we are not admin but are registrar // if trying to create new registrar or admin, return forbidden if (potentialScopes.some(scope => ['admin', ...config.registrarScopes].includes(scope))) { - const e = new APIError('Registrar not allowed to create new admin or registrar users', 'CANNOT_CREATE_ADMIN', httpStatus.FORBIDDEN, true); + const e = new APIError('Registrar not allowed to create new admin or registrar users', 'CANNOT_CREATE_ADMIN', httpStatus.FORBIDDEN); return next(e); } // else go ahead @@ -123,7 +123,7 @@ function update(req, res, next) { User.findById(req.params.userId) .then((user) => { if (req.user.username !== user.username && !req.user.isAdmin()) { - const e = new APIError('User not allowed to update email', 'CANNOT_UPDATE_EMAIL', httpStatus.FORBIDDEN, true); + const e = new APIError('User not allowed to update email', 'CANNOT_UPDATE_EMAIL', httpStatus.FORBIDDEN); return next(e); } return user.update({ email: req.body.email }); diff --git a/src/helpers/APIError.js b/src/helpers/APIError.js index 987ad61..ef1e8c5 100644 --- a/src/helpers/APIError.js +++ b/src/helpers/APIError.js @@ -1,35 +1,115 @@ -import httpStatus from 'http-status'; +import { initLogLevelGte } from 'winston-json-formatter'; +import config from '../config/config'; + +// Read https://www.joyent.com/node-js/production/design/errors to understand the difference between +// "programmer errors" (i.e. bugs) and "operational errors" (i.e. not actual bugs). +// The APIError class assumes that if it was instantiated without a causal error, then it is being +// used to define an "operational error", and there is no actual "programmer error"/bug. + +const alwaysIncludeErrorStacks = config.alwaysIncludeErrorStacks; + +// Errors thrown by 3rd party libraries might log data that we pass into them, such as PHI or PII. +// And, devs might pass those errors as the causalError arg into APIError. So, to prevent accidental +// logging of PHI/PII, only include and thus log causal errors if your config.logLevel is >= debug, +// i.e. silly or debug. +const includeCausalError = initLogLevelGte(config.logLevel)('debug'); /** * @extends Error */ -class ExtendableError extends Error { - constructor(message, code, status, isPublic) { - super(message); - this.name = this.constructor.name; - this.message = { status: 'ERROR', message, code }; +class APIError extends Error { + /* eslint-disable max-len */ + /** + * Create an APIError. + * @param {Error | string} arg1 - Either JS Error (the causal error) (optional), or error message (required). + * @param {string} arg2 - Either error message (required) or error code (required). + * @param {string | Number} arg3 - Either error code (required) or HTTP status (required) + * @param {Number | Object} arg4 - Either HTTP status (required) or options object (optional) + * @param {Object} arg5 - Options object (optional) + * @returns APIError + * @example new APIError('Create user failed. Please provide a username.', 'CREATE_USER_FAILED', 400, { includeStack: true }) + * @example if (err) { throw new APIError(err, 'Create user failed. User already exists.', 'CREATE_USER_FAILED', 409, { isPublic: false }) } + */ + /* eslint-enable max-len */ + constructor(arg1, arg2, arg3, arg4, arg5) { + if (!(typeof arg1 === 'string' || arg1 instanceof String || arg1 instanceof Error)) { + // eslint-disable-next-line max-len + throw new Error('Attempting to run APIError constructor failed: The first arg to the constructor must be a string or object that is instanceof Error.'); + } + + // eslint-disable-next-line one-var,one-var-declaration-per-line + let causalError, message, code, status, options; + + if (typeof arg1 === 'string' || arg1 instanceof String) { + causalError = undefined; + message = arg1; + code = arg2; + status = arg3; + options = arg4; + } else { + causalError = arg1; + message = arg2; + code = arg3; + status = arg4; + options = arg5; + } + + // Default to empty object to prevent "Cannot read property 'X' of undefined" errors. + if (!options) { + options = {}; + } + + // If there is no causal error, assume this is an "operational error" (see comment above). + // (Define isOperational here because it must be used before the constructor, and therefore + // before this.isOperational is available for use.) + let isOperational = !causalError; + + // `options.isOperational` can override the default isOperational behavior. + if (options.isOperational !== undefined) { + isOperational = options.isOperational; + } + + // eslint-disable-next-line no-underscore-dangle + const _includeCausalError = includeCausalError || options.includeCausalError; + + // eslint-disable-next-line no-underscore-dangle + let _message; + if (_includeCausalError && causalError && causalError.message) { + _message = `${message}: ${causalError.message}`; + } else { + _message = message; + } + + super(_message); + + this.message = _message; this.code = code; this.status = status; - this.isPublic = isPublic; - this.isOperational = true; // This is required since bluebird 4 doesn't append it anymore. - Error.captureStackTrace(this, this.constructor.name); - } -} + this.options = options; + this.isOperational = isOperational; -/** - * Class representing an API error. - * @extends ExtendableError - */ -class APIError extends ExtendableError { + // When this.isPublic is truthy, the server's response body will include the error "code" + // and "message". + // We want isPublic to default to true. The reason is that most of the time we do want the + // code and message to be returned to the client rather than hidden, and we don't want to + // have to specify isPublic every time we create a new error. In fact, the primary purpose + // of the "code" is to support the front end's needs. + this.isPublic = (options.isPublic === undefined ? true : options.isPublic); - /** - * Creates an API error. - * @param {string} message - Error message. - * @param {number} status - HTTP status code of error. - * @param {boolean} isPublic - Whether the message should be visible to user or not. - */ - constructor(message, code, status = httpStatus.INTERNAL_SERVER_ERROR, isPublic = false) { - super(message, code, status, isPublic); + // See comments at top of file about logging of causal errors. + if (_includeCausalError) { + this.cause = causalError; + } + + // This class extends Error, so this.stack gets automatically generated; however we don't + // always want a stack, so we have to do something with it. + // We don't want a stack trace when there is an operational error because we don't want to + // mislead future developer maintainers into thinking there's a bug when there isn't. + // We only want this.stack when this class is being used for a "programmer error" OR if we + // override this behavior and always have stacks. + if (!(alwaysIncludeErrorStacks || options.includeStack || !this.isOperational)) { + delete this.stack; + } } } diff --git a/src/helpers/jwt.js b/src/helpers/jwt.js index dfe96a6..f3f7471 100644 --- a/src/helpers/jwt.js +++ b/src/helpers/jwt.js @@ -19,7 +19,7 @@ module.exports = { */ checkExternalProvider(req, res, next) { if (req.user.provider !== null) { - const err = new APIError('Cannot call this endpoint for user managed with external auth', 'EXTERNAL_AUTH_USED', httpStatus.FORBIDDEN, true); + const err = new APIError('Cannot call this endpoint for user managed with external auth', 'EXTERNAL_AUTH_USED', httpStatus.FORBIDDEN); return next(err); } return next(); diff --git a/src/helpers/owasp.js b/src/helpers/owasp.js index a2be8ba..6f82ae9 100644 --- a/src/helpers/owasp.js +++ b/src/helpers/owasp.js @@ -13,7 +13,7 @@ module.exports = { checkPassword(req, res, next) { const result = owasp.test(req.body.password); if (!result.strong) { - const err = new APIError(result.errors.join('\r\n'), 'STRONG_PASS_REQUIRED', httpStatus.BAD_REQUEST, true); + const err = new APIError(result.errors.join('\r\n'), 'STRONG_PASS_REQUIRED', httpStatus.BAD_REQUEST); return next(err); } return next(); diff --git a/test/helpers/APIError.spec.js b/test/helpers/APIError.spec.js new file mode 100644 index 0000000..e1648e9 --- /dev/null +++ b/test/helpers/APIError.spec.js @@ -0,0 +1,155 @@ +/* eslint-env mocha */ + +import { fail } from 'assert'; +import { expect } from 'chai'; +import httpStatus from 'http-status'; + +import APIError from '../../src/helpers/APIError'; + +const causalErrorMessage = 'Causal error message'; +const apiErrorMessage = 'APIError message'; +const errorCode = 'ERROR_CODE'; + +describe('APIError:', () => { + describe('isPublic:', () => { + it('should default to true...', () => { + const errWithoutOptions = new APIError('a', 'b', 'c'); + expect(errWithoutOptions.options).to.deep.equal({}); + expect(errWithoutOptions.isPublic).to.equal(true); + + const errWithOptionsButLackingIsPublic = new APIError('a', 'b', 'c', { optionA: 'A' }); + expect(errWithOptionsButLackingIsPublic.options).to.deep.equal({ optionA: 'A' }); + expect(errWithOptionsButLackingIsPublic.isPublic).to.equal(true); + }); + + it('should be settable via options.isPublic', () => { + const errWithOptionsWithIsPublic = new APIError('a', 'b', 'c', { optionA: 'A', isPublic: false }); + expect(errWithOptionsWithIsPublic.options).to.deep.equal({ optionA: 'A', isPublic: false }); + expect(errWithOptionsWithIsPublic.isPublic).to.equal(false); + }); + }); + + describe('Causal Error Handling and isOperational:', () => { + it('When a causal error is passed in, default isOperational to false', () => { + const causalError = new Error(causalErrorMessage); + const err = new APIError(causalError, apiErrorMessage); + + expect(err.isOperational).to.equal(false); + }); + + it('When a causal error is NOT passed in, default isOperational to true', () => { + const err = new APIError(apiErrorMessage); + + expect(err.isOperational).to.equal(true); + }); + + it('`options.isOperational` overrides the default behavior', () => { + const causalError = new Error(causalErrorMessage); + const err1 = new APIError(causalError, apiErrorMessage, errorCode, httpStatus.NOT_FOUND, { isOperational: 'SOMETHING' }); + const err2 = new APIError(apiErrorMessage, errorCode, httpStatus.NOT_FOUND, { isOperational: 'SOMETHING' }); + + expect(err1.isOperational).to.equal('SOMETHING'); + expect(err1.isOperational).to.equal(err1.options.isOperational); + expect(err2.isOperational).to.equal('SOMETHING'); + expect(err2.isOperational).to.equal(err2.options.isOperational); + }); + + // Note: includeCausalError is true because .env.test sets LOG_LEVEL=debug + describe('When includeCausalError is truthy...', () => { + it('the causal error is set to myAPIError.cause', () => { + const causalError = new Error(causalErrorMessage); + const err = new APIError(causalError, apiErrorMessage); + + expect(err.cause).to.deep.equal(causalError); + }); + + it('Concatenate messages of APIError and the causal errror', () => { + const causalError = new Error(causalErrorMessage); + const err = new APIError(causalError, apiErrorMessage); + + expect(err.message).to.equal(`${apiErrorMessage}: ${causalErrorMessage}`); + }); + }); + + // TODO: Mock APIError.js's call to `initLogLevelGte()` to force initialize it to a level + // below 'debug' because .env.test has LOG_LEVEL=deug and APIError.js has: + // ``` + // const logLevelGte = initLogLevelGte(config.logLevel); + // const includeCausalError = logLevelGte('debug'); + // ``` + // Then, write tests for the following cases: + // 1. Cases for: describe('When includeCausalError is falsy', () => { + // 2. Cases related to options.includeCausalError + }); + + describe('Supports argument shifting based on first argument being a causal error, or not:', () => { + it('Fist argument must be either an error or string', () => { + try { + new APIError(new Error('something')); // eslint-disable-line no-new + new APIError('this can be any string literal'); // eslint-disable-line no-new + new APIError(new String('something')); // eslint-disable-line no-new,no-new-wrappers + } catch (err) { + fail(); + } + }); + + it('Throws error when first arg is not an error or string', () => { + try { + new APIError(); // eslint-disable-line no-new + // The error should get thrown, so this line should never run. + fail(); + } catch (err) { + expect(err.message).to.equal('Attempting to run APIError constructor failed: The first arg to the constructor must be a string or object that is instanceof Error.'); + } + }); + + it('Supports calling with causal error', () => { + const causalError = new Error(causalErrorMessage); + const err = new APIError(causalError, apiErrorMessage, errorCode, httpStatus.NOT_FOUND, { optionA: 'A', isPublic: false }); + + expect(err.cause).to.deep.equal(causalError); + expect(err.message).to.equal(`${apiErrorMessage}: ${causalErrorMessage}`); + expect(err.code).to.equal(errorCode); + expect(err.status).to.equal(httpStatus.NOT_FOUND); + expect(err.options).to.deep.equal({ optionA: 'A', isPublic: false }); + }); + + it('Supports calling without causal error', () => { + const err = new APIError(apiErrorMessage, errorCode, httpStatus.NOT_FOUND, { optionA: 'A', isPublic: false }); + + expect(err.cause).to.equal(undefined); + expect(err.message).to.equal(apiErrorMessage); + expect(err.code).to.equal(errorCode); + expect(err.status).to.equal(httpStatus.NOT_FOUND); + expect(err.options).to.deep.equal({ optionA: 'A', isPublic: false }); + }); + }); + + describe('Error stacks:', () => { + describe('There should be a stack when...', () => { + it('There is a causal error', () => { + const causalError = new Error(); + const err = new APIError(causalError); + expect(err.stack).to.exist; // eslint-disable-line no-unused-expressions + }); + + // // TODO: Mock set config.alwaysIncludeErrorStacks before importing APIError.js + // it('alwaysIncludeErrorStacks is truthy', () => { + // const err = new APIError('some string'); + // expect(err.stack).to.exist; // eslint-disable-line no-unused-expressions + // }); + + it('options.includeStack is truthy', () => { + const err = new APIError('message', 'CODE', 'http status', { includeStack: true }); + expect(err.stack).to.exist; // eslint-disable-line no-unused-expressions + }); + }); + + describe('There should not be a stack...', () => { + it('In any other case', () => { + const err = new APIError('some string'); + expect(err.stack).to.equal(undefined); + }); + }); + }); +}); diff --git a/test/integration/auth.integration.spec.js b/test/integration/auth.integration.spec.js index a25345f..a5118cd 100644 --- a/test/integration/auth.integration.spec.js +++ b/test/integration/auth.integration.spec.js @@ -54,7 +54,7 @@ describe('Auth API:', () => { it('should return 401 error with bad password', () => request(app).post(`${common.baseURL}/auth/login`) .send(common.badPassword) - .expect(httpStatus.NOT_FOUND) + .expect(httpStatus.UNAUTHORIZED) .then((res) => { expect(res.body.message).to.equal('Incorrect username or password'); expect(res.body.code).to.equal('INCORRECT_USERNAME_OR_PASSWORD'); @@ -62,10 +62,10 @@ describe('Auth API:', () => { }) ); - it('should return 404 when there is no username found', () => + it('should return 401 when there is no username found', () => request(app).post(`${common.baseURL}/auth/login`) .send(common.missingUsername) - .expect(httpStatus.NOT_FOUND) + .expect(httpStatus.UNAUTHORIZED) .then((res) => { expect(res.body.message).to.equal('Incorrect username or password'); expect(res.body.code).to.equal('INCORRECT_USERNAME_OR_PASSWORD'); diff --git a/yarn.lock b/yarn.lock index c8d6863..f512269 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7337,14 +7337,14 @@ windows-release@^3.1.0: dependencies: execa "^0.10.0" -winston-json-formatter@^0.9.2: - version "0.9.2" - resolved "https://registry.yarnpkg.com/winston-json-formatter/-/winston-json-formatter-0.9.2.tgz#cb2adad2596a11c19d3ac5a454756712a2aacf98" - integrity sha512-IeWNsy/VzsZDXjXNrdxvw9MvCQ28QBSw6zgCkTLS2av28wJnScGDpq97ys7X9ZWygpEGZrsGXJxyWk3bBsqhZg== +winston-json-formatter@^0.10.0: + version "0.10.0" + resolved "https://registry.yarnpkg.com/winston-json-formatter/-/winston-json-formatter-0.10.0.tgz#4e777023e4416e16740734e1b7b50c5e72dfd4fd" + integrity sha512-4K9rFgawAOBsKbC5D5QvxiZccbuBR+xvPY+PAm6rTDtfaA5bwUjpFKW0YRexcrFlJzV/6ExtfUBgYpDXAm4NSA== dependencies: lodash "^4.17.11" os "^0.1.1" - winston "^3.1.0" + winston "^3.2.1" winston-transport@^4.3.0: version "4.3.0" @@ -7354,7 +7354,7 @@ winston-transport@^4.3.0: readable-stream "^2.3.6" triple-beam "^1.2.0" -winston@^3.0.0, winston@^3.1.0: +winston@^3.2.1: version "3.2.1" resolved "https://registry.yarnpkg.com/winston/-/winston-3.2.1.tgz#63061377976c73584028be2490a1846055f77f07" integrity sha512-zU6vgnS9dAWCEKg/QYigd6cgMVVNwyTzKs81XZtTFuRwJOcDdBg7AU0mXVyNbs7O5RH2zdv+BdNZUlx7mXPuOw==