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

Added custom error objects including code #3467

Merged
merged 17 commits into from
Jan 1, 2019
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
15 changes: 15 additions & 0 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ Mocha is a feature-rich JavaScript test framework running on [Node.js](https://n
- [Configuring Mocha (Node.js)](#configuring-mocha-nodejs)
- [`mocha.opts`](#mochaopts)
- [The `test/` Directory](#the-test-directory)
- [Error Codes](#error-codes)
- [Editor Plugins](#editor-plugins)
- [Examples](#examples)
- [Testing Mocha](#testing-mocha)
Expand Down Expand Up @@ -1658,6 +1659,20 @@ $ mocha "./spec/**/*.js"

*Note*: Double quotes around the glob are recommended for portability.

## Error Codes

List of codes associated with Errors thrown inside Mocha. Following NodeJS practices.

| Code | Meaning |
| ------------- | ------------- |
| ERR_MOCHA_INVALID_ARG_VALUE | an argument used did not use the supported value |
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
| ERR_MOCHA_INVALID_ARG_TYPE | an argument used did not use the supported type |
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
| ERR_MOCHA_INVALID_INTERFACE | interface specified in options not found |
| ERR_MOCHA_INVALID_REPORTER | reporter specified in options not found |
| ERR_MOCHA_NO_FILES_MATCH_PATTERN | file/s of test could not be found |
| ERR_MOCHA_NOT_SUPPORTED | type of output specified was not supported |
| ERR_MOCHA_UNDEFINED_ERROR | an error was thrown but no details were specified |

## Editor Plugins

The following editor-related packages are available:
Expand Down
6 changes: 2 additions & 4 deletions lib/cli/run-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,8 @@ exports.handleFiles = ({
try {
newFiles = utils.lookupFiles(arg, extension, recursive);
} catch (err) {
if (err.message.indexOf('cannot resolve path') === 0) {
console.error(
`Warning: Could not find any test files matching pattern: ${arg}`
);
if (err.code === 'ERR_MOCHA_NO_FILES_MATCH_PATTERN') {
Copy link
Contributor

Choose a reason for hiding this comment

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

future PR: use variables instead of hardcoded strings

Copy link
Contributor

@plroebuck plroebuck Jan 2, 2019

Choose a reason for hiding this comment

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

Regrettably, it's the standard Node Error.code processing pattern.
This seems more like what I would have preferred -- exported constants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I had it like that on first iteration of PR but the change was lost in the ether. Will look at updating.

console.warn('Warning: %s: %O', err.message, err.pattern);
return;
}

Expand Down
4 changes: 3 additions & 1 deletion lib/cli/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

const Mocha = require('../mocha');
const ansi = require('ansi-colors');
var errors = require('../errors');
Copy link
Contributor

Choose a reason for hiding this comment

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

const?

var createInvalidReporterError = errors.createInvalidReporterError;

const {
list,
Expand Down Expand Up @@ -190,7 +192,7 @@ exports.builder = yargs =>
const pair = opt.split('=');

if (pair.length > 2 || !pair.length) {
throw new Error(`invalid reporter option '${opt}'`);
throw createInvalidReporterError('invalid reporter option', opt);
Copy link
Contributor

@plroebuck plroebuck Dec 28, 2018

Choose a reason for hiding this comment

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

Said this last time. This is not an InvalidReporterError, but a TypeError with code ERR_INVALID_ARG_VALUE. This is CLI parsing code...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok will add createInvalidArgumentTypeError. Also have been using mocha prefix even for type errors so ERR_MOCHA_INVALID_ARG_VALUE .

Copy link
Contributor

@plroebuck plroebuck Dec 31, 2018

Choose a reason for hiding this comment

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

               throw createInvalidArgumentValueError(
                 `invalid reporter option '${opt}'`,
                 '--reporter-option',
                 opt,
                 'expected "key=value" format');

}

acc[pair[0]] = pair.length === 2 ? pair[1] : true;
Expand Down
138 changes: 138 additions & 0 deletions lib/errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
'use strict';
/**
* @module Errors
*/
/**
* Factory functions to create throwable error objects
*/

plroebuck marked this conversation as resolved.
Show resolved Hide resolved
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
/**
* Creates an error object used when no files to be tested could be found using specified pattern.
*
* @public
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
* @param {string} message - Error message to be displayed.
* @param {string} pattern - User-specified argument value.
* @returns {Error} instance detailing the error condition
*/
function createNoFilesMatchPatternError(message, pattern) {
var err = new Error(message);
Copy link
Contributor

@plroebuck plroebuck Dec 31, 2018

Choose a reason for hiding this comment

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

In a later revision (PR), we should generate the messages using additional arguments rather than requiring the caller to do so.

   var err = new Error(`{message}: pattern: "{pattern}"`);

But let's get what you've done thusfar in place first...

err.code = 'ERR_MOCHA_NO_FILES_MATCH_PATTERN';
err.pattern = pattern;
return err;
}

/**
* Creates an error object used when the reporter specified in the options was not found.
*
* @public
* @param {string} message - Error message to be displayed.
* @param {string} reporter - User-specified reporter value.
* @returns {Error} instance detailing the error condition
*/
function createInvalidReporterError(message, reporter) {
var err = new TypeError(message);
err.code = 'ERR_MOCHA_INVALID_REPORTER';
err.reporter = reporter;
return err;
}

/**
* Creates an error object used when the interface specified in the options was not found.
*
* @public
* @param {string} message - Error message to be displayed.
* @param {string} ui - User-specified interface value.
* @returns {Error} instance detailing the error condition
*/
function createInvalidInterfaceError(message, ui) {
var err = new Error(message);
err.code = 'ERR_MOCHA_INVALID_INTERFACE';
err.interface = ui;
return err;
}

/**
* Creates an error object used when the type of output specified was not supported.
*
* @public
* @param {string} message - Error message to be displayed.
* @returns {Error} instance detailing the error condition
*/
function createNotSupportedError(message) {
var err = new Error(message);
err.code = 'ERR_MOCHA_NOT_SUPPORTED';
return err;
}

/**
* Creates an error object used when an argument is missing.
*
* @public
* @param {string} message - Error message to be displayed.
* @param {string} argument - User-specified missing argument name.
* @param {string} expected - User-specified expected argument name.
Copy link
Contributor

Choose a reason for hiding this comment

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

Patch argument and expected descriptions to match those below

* @returns {Error} instance detailing the error condition
*/
function createMissingArgumentError(message, argument, expected) {
return createInvalidArgumentTypeError(message, argument, expected);
}
plroebuck marked this conversation as resolved.
Show resolved Hide resolved

/**
* Creates an error object used when an argument did not use the supported type
*
* @public
* @param {string} message - Error message to be displayed.
* @param {string} argument - User-specified actual argument type.
Copy link
Contributor

@plroebuck plroebuck Dec 31, 2018

Choose a reason for hiding this comment

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

  * @param {string} argument - Argument name.

* @param {string} expected - User-specified expected argument type.
Copy link
Contributor

@plroebuck plroebuck Dec 31, 2018

Choose a reason for hiding this comment

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

  * @param {string} expected - Expected argument datatype.

* @returns {Error} instance detailing the error condition
*/
function createInvalidArgumentTypeError(message, argument, expected) {
var err = new TypeError(message);
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
err.code = 'ERR_MOCHA_INVALID_ARG_TYPE';
err.argument = argument;
err.expected = expected;
err.actual = typeof argument;
return err;
}

/**
* Creates an error object used when an argument did not use the supported value
*
* @public
* @param {string} message - Error message to be displayed.
* @param {string} argument - User-specified actual argument value.
* @param {string} expected - User-specified expected argument value.
Copy link
Contributor

@plroebuck plroebuck Dec 31, 2018

Choose a reason for hiding this comment

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

  * @param {string} argument - Argument name.
  * @param {string} value - Argument value.
  * @param {string} [reason] - Why value is invalid.

* @returns {Error} instance detailing the error condition
*/
function createInvalidArgumentValueError(message, argument, expected) {
Copy link
Contributor

@plroebuck plroebuck Dec 31, 2018

Choose a reason for hiding this comment

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

 function createInvalidArgumentValueError(message, argument, value, reason) {

var err = new TypeError(message);
err.code = 'ERR_MOCHA_INVALID_ARG_VALUE';
err.argument = argument;
err.expected = expected;
err.actual = typeof argument;
Copy link
Contributor

Choose a reason for hiding this comment

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

   err.value = value;
   err.reason = (typeof reason !== 'undefined')
     ? reason
     : 'is invalid';

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I'm not just making these up arbitrarily. Mimicking Node errors.

return err;
}

plroebuck marked this conversation as resolved.
Show resolved Hide resolved
/**
* Creates an error object used when an error was thrown but no details were specified.
*
* @public
* @param {string} message - Error message to be displayed.
* @returns {Error} instance detailing the error condition
*/
function createUndefinedError(message) {
var err = new Error(message);
err.code = 'ERR_MOCHA_UNDEFINED_ERROR';
return err;
}

module.exports = {
createInvalidArgumentValueError: createInvalidArgumentValueError,
createInvalidArgumentTypeError: createInvalidArgumentTypeError,
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
createInvalidInterfaceError: createInvalidInterfaceError,
createInvalidReporterError: createInvalidReporterError,
createMissingArgumentError: createMissingArgumentError,
createNoFilesMatchPatternError: createNoFilesMatchPatternError,
createNotSupportedError: createNotSupportedError,
createUndefinedError: createUndefinedError
};
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 3 additions & 1 deletion lib/interfaces/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

var Suite = require('../suite');
var utils = require('../utils');
var errors = require('../errors');
var MissingCallbackError = errors.MissingCallbackError;

/**
* Functions common to more than one interface.
Expand Down Expand Up @@ -147,7 +149,7 @@ module.exports = function(suites, context, mocha) {
}
suites.shift();
} else if (typeof opts.fn === 'undefined' && !suite.pending) {
throw new Error(
throw MissingCallbackError(
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
'Suite "' +
suite.fullTitle() +
'" was defined but no callback was supplied. ' +
Expand Down
21 changes: 17 additions & 4 deletions lib/mocha.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ var builtinReporters = require('./reporters');
var utils = require('./utils');
var mocharc = require('./mocharc.json');
var assign = require('object.assign').getPolyfill();
var errors = require('./errors');
var createInvalidReporterError = errors.createInvalidReporterError;
var createInvalidInterfaceError = errors.createInvalidInterfaceError;

exports = module.exports = Mocha;

Expand Down Expand Up @@ -220,12 +223,16 @@ Mocha.prototype.reporter = function(reporter, reporterOptions) {
try {
_reporter = require(reporter);
} catch (err) {
if (err.message.indexOf('Cannot find module') !== -1) {
if (
err.code !== 'MODULE_NOT_FOUND' ||
err.message.includes('Cannot find module')
) {
// Try to load reporters from a path (absolute or relative)
try {
_reporter = require(path.resolve(process.cwd(), reporter));
} catch (_err) {
err.message.indexOf('Cannot find module') !== -1
_err.code !== 'MODULE_NOT_FOUND' ||
_err.message.includes('Cannot find module')
? console.warn('"' + reporter + '" reporter not found')
: console.warn(
'"' +
Expand All @@ -249,7 +256,10 @@ Mocha.prototype.reporter = function(reporter, reporterOptions) {
);
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
}
if (!_reporter) {
throw new Error('invalid reporter "' + reporter + '"');
throw createInvalidReporterError(
'invalid reporter "' + reporter + '"',
reporter
);
}
this._reporter = _reporter;
}
Expand All @@ -275,7 +285,10 @@ Mocha.prototype.ui = function(name) {
try {
this._ui = require(name);
} catch (err) {
throw new Error('invalid interface "' + name + '"');
throw createInvalidInterfaceError(
'invalid interface "' + name + '"',
name
);
}
}
this._ui = this._ui(this.suite);
Expand Down
5 changes: 3 additions & 2 deletions lib/reporters/xunit.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ var fs = require('fs');
var escape = utils.escape;
var mkdirp = require('mkdirp');
var path = require('path');

var errors = require('../errors');
var createNotSupportedError = errors.createNotSupportedError;
/**
* Save timer references to avoid Sinon interfering (see GH-237).
*/
Expand Down Expand Up @@ -50,7 +51,7 @@ function XUnit(runner, options) {
if (options && options.reporterOptions) {
if (options.reporterOptions.output) {
if (!fs.createWriteStream) {
throw new Error('file output not supported in browser');
throw createNotSupportedError('file output not supported in browser');
}

mkdirp.sync(path.dirname(options.reporterOptions.output));
Expand Down
10 changes: 7 additions & 3 deletions lib/suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ var utils = require('./utils');
var inherits = utils.inherits;
var debug = require('debug')('mocha:suite');
var milliseconds = require('ms');
var errors = require('./errors');
var createInvalidArgumentTypeError = errors.createInvalidArgumentTypeError;

/**
* Expose `Suite`.
Expand Down Expand Up @@ -49,10 +51,12 @@ exports.create = function(parent, title) {
*/
function Suite(title, parentContext) {
if (!utils.isString(title)) {
throw new Error(
'Suite `title` should be a "string" but "' +
throw createInvalidArgumentTypeError(
'Suite argument "title" must be a string. Received type "' +
typeof title +
'" was given instead.'
'"',
'title',
'string'
);
}
this.title = title;
Expand Down
10 changes: 7 additions & 3 deletions lib/test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict';
var Runnable = require('./runnable');
var utils = require('./utils');
var errors = require('./errors');
var createInvalidArgumentTypeError = errors.createInvalidArgumentTypeError;
var isString = utils.isString;

module.exports = Test;
Expand All @@ -15,10 +17,12 @@ module.exports = Test;
*/
function Test(title, fn) {
if (!isString(title)) {
throw new Error(
'Test `title` should be a "string" but "' +
throw createInvalidArgumentTypeError(
'Test argument "title" should be a string. Received type "' +
typeof title +
'" was given instead.'
'"',
'title',
'string'
);
}
Runnable.call(this, title, fn);
Expand Down
16 changes: 12 additions & 4 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ var glob = require('glob');
var path = require('path');
var join = path.join;
var he = require('he');
var errors = require('./errors');
var createNoFilesMatchPatternError = errors.createNoFilesMatchPatternError;
var createMissingArgumentError = errors.createMissingArgumentError;
var createUndefinedError = errors.createUndefinedError;

/**
* Ignored directories.
Expand Down Expand Up @@ -515,7 +519,10 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) {
} else {
files = glob.sync(filepath);
if (!files.length) {
throw new Error("cannot resolve path (or pattern) '" + filepath + "'");
throw createNoFilesMatchPatternError(
'cannot find any files matching pattern "' + filepath + '"',
filepath
);
}
return files;
}
Expand Down Expand Up @@ -546,8 +553,9 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) {
return;
}
if (!extensions) {
throw new Error(
'extensions parameter required when filepath is a directory'
throw createMissingArgumentError(
'Argument "extensions" required when argument "filepath" is a directory',
'extensions'
);
}
var re = new RegExp('\\.(?:' + extensions.join('|') + ')$');
Expand All @@ -567,7 +575,7 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) {
*/

exports.undefinedError = function() {
return new Error(
return createUndefinedError(
'Caught undefined error, did you throw without specifying what?'
);
};
Expand Down
10 changes: 5 additions & 5 deletions test/integration/file-utils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ describe('file utils', function() {
var dirLookup = function() {
return utils.lookupFiles(tmpDir, undefined, false);
};
expect(
dirLookup,
'to throw',
'extensions parameter required when filepath is a directory'
);
expect(dirLookup, 'to throw', {
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
name: 'TypeError',
code: 'ERR_MOCHA_INVALID_ARG_TYPE',
argument: 'extensions'
});
});
});

Expand Down
Loading