From c707c76633ae870f6edcf53ba58f79388a61423a Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Wed, 11 Jan 2017 18:45:59 -0400 Subject: [PATCH] feat(CLI): improve error messages (#1015) - Inline dictionary of common error codes and friendly descriptions We used to rely on `resin-cli-errors` for this, however the friendly descriptions set there can be usually very Resin CLI specific, and thus confusing to Etcher users. The dictionary, along with other error related utilities live in `lib/cli/errors.js`. - Print error descriptions and codes if they are found - Move `utils.printError()` to `errors.print()` Change-Type: minor Changelog-Entry: Improve Etcher CLI error messages. Signed-off-by: Juan Cruz Viotti --- lib/cli/cli.js | 4 +- lib/cli/errors.js | 130 ++++++++++++++++++++++++++++++++++++++ lib/cli/etcher.js | 4 +- lib/cli/utils.js | 39 ------------ npm-shrinkwrap.json | 12 ---- package.json | 1 - tests/cli/errors.spec.js | 132 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 266 insertions(+), 56 deletions(-) create mode 100644 lib/cli/errors.js delete mode 100644 lib/cli/utils.js create mode 100644 tests/cli/errors.spec.js diff --git a/lib/cli/cli.js b/lib/cli/cli.js index f1cb22e3..b70b42c2 100644 --- a/lib/cli/cli.js +++ b/lib/cli/cli.js @@ -19,7 +19,7 @@ const _ = require('lodash'); const fs = require('fs'); const yargs = require('yargs'); -const utils = require('./utils'); +const errors = require('./errors'); const robot = require('../shared/robot'); const EXIT_CODES = require('../shared/exit-codes'); const packageJSON = require('../../package.json'); @@ -69,7 +69,7 @@ module.exports = yargs robot.printError(error || message); } else { yargs.showHelp(); - utils.printError(error || message); + errors.print(error || message); } process.exit(1); diff --git a/lib/cli/errors.js b/lib/cli/errors.js new file mode 100644 index 00000000..c791e7fc --- /dev/null +++ b/lib/cli/errors.js @@ -0,0 +1,130 @@ +/* + * Copyright 2016 resin.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +const _ = require('lodash'); +const chalk = require('chalk'); + +/** + * @summary Human-friendly error messages + * @namespace HUMAN_FRIENDLY + * @public + */ +exports.HUMAN_FRIENDLY = { + + /* eslint-disable new-cap */ + + /** + * @property {Function} ENOENT + * @memberof HUMAN_FRIENDLY + * @param {Error} error - error object + * @returns {String} message + */ + ENOENT: (error) => { + return `No such file or directory: ${error.path}`; + }, + + /** + * @property {Function} EPERM + * @memberof HUMAN_FRIENDLY + * @returns {String} message + */ + EPERM: () => { + return 'You\'re not authorized to perform this operation'; + }, + + /** + * @property {Function} EACCES + * @memberof HUMAN_FRIENDLY + * @returns {String} message + */ + EACCES: () => { + return 'You\'re don\'t have access to this resource'; + } + + /* eslint-enable new-cap */ + +}; + +/** + * @summary Get default error message + * @function + * @private + * + * @param {Error} error - error + * @returns {String} error message + * + * @example + * const message = defaultMessageGetter(new Error('foo bar')); + * console.log(message); + * > 'foo bar' + * + * @example + * const message = defaultMessageGetter(new Error()); + * console.log(message); + * > 'Unknown error' + */ +const defaultMessageGetter = (error) => { + return error.message || 'Unknown error'; +}; + +/** + * @summary Get error message + * @function + * @public + * + * @param {(String|Error)} error - error + * @returns {String} error message + * + * @example + * const error = new Error('Foo bar'); + * error.description = 'This is a fake error'; + * + * console.log(errors.getErrorMessage(error)); + * > 'Foo bar\n\nThis is a fake error' + */ +exports.getErrorMessage = (error) => { + if (_.isString(error)) { + return exports.getErrorMessage(new Error(error)); + } + + const message = _.attempt(() => { + const title = _.get(exports.HUMAN_FRIENDLY, error.code, defaultMessageGetter)(error); + return error.code ? `${error.code}: ${title}` : title; + }); + + if (error.description) { + return message + '\n\n' + error.description; + } + + return message; +}; + +/** + * @summary Print an error to stderr + * @function + * @public + * + * @param {(Error|String)} error - error + * + * @example + * errors.print(new Error('Oops!')); + */ +exports.print = (error) => { + const message = exports.getErrorMessage(error); + console.error(chalk.red(message)); +}; diff --git a/lib/cli/etcher.js b/lib/cli/etcher.js index 4d82f98d..09582580 100644 --- a/lib/cli/etcher.js +++ b/lib/cli/etcher.js @@ -23,7 +23,7 @@ const visuals = require('resin-cli-visuals'); const form = require('resin-cli-form'); const drivelist = Bluebird.promisifyAll(require('drivelist')); const writer = require('./writer'); -const utils = require('./utils'); +const errors = require('./errors'); const options = require('./cli'); const robot = require('../shared/robot'); const EXIT_CODES = require('../shared/exit-codes'); @@ -119,7 +119,7 @@ isElevated().then((elevated) => { return robot.printError(error); } - utils.printError(error); + errors.print(error); }).then(() => { if (error.code === 'EVALIDATION') { process.exit(EXIT_CODES.VALIDATION_ERROR); diff --git a/lib/cli/utils.js b/lib/cli/utils.js deleted file mode 100644 index 13b27511..00000000 --- a/lib/cli/utils.js +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright 2016 resin.io - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -const _ = require('lodash'); -const errors = require('resin-cli-errors'); -const chalk = require('chalk'); - -/** - * @summary Print an error to stderr - * @function - * @public - * - * @param {(Error|String)} error - error - * - * @example - * utils.printError(new Error('Oops!')); - */ -exports.printError = (error) => { - if (_.isString(error)) { - error = new Error(error); - } - - console.error(chalk.red(errors.interpret(error))); -}; diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 5a64c414..33c1cdc4 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1417,18 +1417,6 @@ "from": "require-uncached@>=1.0.2 <2.0.0", "resolved": "https://registry.npmjs.org/require-uncached/-/require-uncached-1.0.2.tgz" }, - "resin-cli-errors": { - "version": "1.2.0", - "from": "resin-cli-errors@>=1.2.0 <2.0.0", - "resolved": "https://registry.npmjs.org/resin-cli-errors/-/resin-cli-errors-1.2.0.tgz", - "dependencies": { - "lodash": { - "version": "3.10.1", - "from": "lodash@>=3.10.1 <4.0.0", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-3.10.1.tgz" - } - } - }, "resin-cli-form": { "version": "1.4.1", "from": "resin-cli-form@>=1.4.1 <2.0.0", diff --git a/package.json b/package.json index 1a5a461f..08e4490c 100644 --- a/package.json +++ b/package.json @@ -83,7 +83,6 @@ "node-ipc": "^8.9.2", "redux": "^3.5.2", "redux-localstorage": "^0.4.1", - "resin-cli-errors": "^1.2.0", "resin-cli-form": "^1.4.1", "resin-cli-visuals": "^1.2.8", "rx": "^4.1.0", diff --git a/tests/cli/errors.spec.js b/tests/cli/errors.spec.js new file mode 100644 index 00000000..c7d76a12 --- /dev/null +++ b/tests/cli/errors.spec.js @@ -0,0 +1,132 @@ +/* + * Copyright 2016 resin.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +const m = require('mochainon'); +const _ = require('lodash'); +const errors = require('../../lib/cli/errors'); + +describe('CLI: Errors', function() { + + describe('.HUMAN_FRIENDLY', function() { + + it('should be a plain object', function() { + m.chai.expect(_.isPlainObject(errors.HUMAN_FRIENDLY)).to.be.true; + }); + + it('should contain function properties', function() { + m.chai.expect(_.every(_.map(errors.HUMAN_FRIENDLY, _.isFunction))).to.be.true; + }); + + }); + + describe('.getErrorMessage()', function() { + + describe('given errors without code properties', function() { + + it('should understand a string error', function() { + const error = 'foo bar'; + m.chai.expect(errors.getErrorMessage(error)).to.equal('foo bar'); + }); + + it('should return a generic error message if there is none', function() { + const error = new Error(); + m.chai.expect(errors.getErrorMessage(error)).to.equal('Unknown error'); + }); + + it('should return a generic error message if error is an empty string', function() { + const error = ''; + m.chai.expect(errors.getErrorMessage(error)).to.equal('Unknown error'); + }); + + it('should return the error message', function() { + const error = new Error('foo bar'); + m.chai.expect(errors.getErrorMessage(error)).to.equal('foo bar'); + }); + + it('should make use of a description if there is one', function() { + const error = new Error('foo bar'); + error.description = 'This is a description'; + m.chai.expect(errors.getErrorMessage(error)).to.equal(_.join([ + 'foo bar', + '', + 'This is a description' + ], '\n')); + }); + + }); + + describe('given errors with code properties', function() { + + it('should provide a friendly message for ENOENT', function() { + const error = new Error('foo bar'); + error.code = 'ENOENT'; + error.path = 'foo.bar'; + const message = 'ENOENT: No such file or directory: foo.bar'; + m.chai.expect(errors.getErrorMessage(error)).to.equal(message); + }); + + it('should provide a friendly message for EPERM', function() { + const error = new Error('foo bar'); + error.code = 'EPERM'; + const message = 'EPERM: You\'re not authorized to perform this operation'; + m.chai.expect(errors.getErrorMessage(error)).to.equal(message); + }); + + it('should provide a friendly message for EACCES', function() { + const error = new Error('foo bar'); + error.code = 'EACCES'; + const message = 'EACCES: You\'re don\'t have access to this resource'; + m.chai.expect(errors.getErrorMessage(error)).to.equal(message); + }); + + it('should make use of a description if there is one', function() { + const error = new Error('foo bar'); + error.code = 'EPERM'; + error.description = 'This is the EPERM description'; + + const message = _.join([ + 'EPERM: You\'re not authorized to perform this operation', + '', + 'This is the EPERM description' + ], '\n'); + + m.chai.expect(errors.getErrorMessage(error)).to.equal(message); + }); + + describe('given the code is not recognised', function() { + + it('should make use of the error message', function() { + const error = new Error('foo bar'); + error.code = 'EFOO'; + const message = 'EFOO: foo bar'; + m.chai.expect(errors.getErrorMessage(error)).to.equal(message); + }); + + it('should return a generic error message if no there is no message', function() { + const error = new Error(); + error.code = 'EFOO'; + m.chai.expect(errors.getErrorMessage(error)).to.equal('EFOO: Unknown error'); + }); + + }); + + }); + + }); + +});