From 57952f6f55760ebc4fbdf13ac79b3d7f41c59bd3 Mon Sep 17 00:00:00 2001
From: Juan Cruz Viotti <jviotti@openmailbox.org>
Date: Tue, 28 Mar 2017 09:43:15 -0400
Subject: [PATCH] fix(CLI): don't print stack traces by default (#1206)

Currently, the Etcher CLI will print scary stack traces for every single
error (e.g: if you forgot to pass an image to the tool), given that
`errors.getDescription()` will return a stack trace if no other
description could be found.

This commit introduces an `ETCHER_CLI_DEBUG` environment variable, which
when set, it will cause the Etcher CLI to output stack traces, plus a
boolean `userFriendlyDescriptionsOnly` option to
`errors.getDescription()`, so we can control whether
`errors.getDescription()` returns things like stack traces, or
stringified error objects.

Change-Type: minor
Changelog-Entry: Don't print stack traces by default in the CLI.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>
---
 docs/CLI.md                 |  6 +++++
 lib/cli/utils.js            |  9 ++++++-
 lib/shared/errors.js        | 12 ++++++++-
 tests/shared/errors.spec.js | 54 +++++++++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/docs/CLI.md b/docs/CLI.md
index 20a3c431..211ecd63 100644
--- a/docs/CLI.md
+++ b/docs/CLI.md
@@ -43,3 +43,9 @@ Options
   --yes, -y      confirm non-interactively
   --unmount, -u  unmount on success
 ```
+
+Debug mode
+----------
+
+You can set the `ETCHER_CLI_DEBUG` environment variable to make the Etcher CLI
+print error stack traces.
diff --git a/lib/cli/utils.js b/lib/cli/utils.js
index e8f26dbd..d75a7b4b 100644
--- a/lib/cli/utils.js
+++ b/lib/cli/utils.js
@@ -31,10 +31,17 @@ const errors = require('../shared/errors');
  */
 exports.printError = (error) => {
   const title = errors.getTitle(error);
-  const description = errors.getDescription(error);
+  const description = errors.getDescription(error, {
+    userFriendlyDescriptionsOnly: true
+  });
 
   console.error(chalk.red(title));
+
   if (description) {
     console.error(`\n${chalk.red(description)}`);
   }
+
+  if (process.env.ETCHER_CLI_DEBUG && error.stack) {
+    console.error(`\n${chalk.red(error.stack)}`);
+  }
 };
diff --git a/lib/shared/errors.js b/lib/shared/errors.js
index 6980959a..9ef72256 100644
--- a/lib/shared/errors.js
+++ b/lib/shared/errors.js
@@ -206,6 +206,8 @@ exports.getTitle = (error) => {
  * @public
  *
  * @param {Error} error - error
+ * @param {Object} options - options
+ * @param {Boolean} [options.userFriendlyDescriptionsOnly=false] - only return user friendly descriptions
  * @returns {String} error description
  *
  * @example
@@ -213,7 +215,11 @@ exports.getTitle = (error) => {
  * const description = errors.getDescription(error);
  * console.log(description);
  */
-exports.getDescription = (error) => {
+exports.getDescription = (error, options = {}) => {
+  _.defaults(options, {
+    userFriendlyDescriptionsOnly: false
+  });
+
   if (!_.isError(error) && !_.isPlainObject(error)) {
     return '';
   }
@@ -227,6 +233,10 @@ exports.getDescription = (error) => {
     return codeDescription;
   }
 
+  if (options.userFriendlyDescriptionsOnly) {
+    return '';
+  }
+
   if (error.stack) {
     return error.stack;
   }
diff --git a/tests/shared/errors.spec.js b/tests/shared/errors.spec.js
index 55fe61ee..bb01d223 100644
--- a/tests/shared/errors.spec.js
+++ b/tests/shared/errors.spec.js
@@ -422,6 +422,60 @@ describe('Shared: Errors', function() {
       m.chai.expect(errors.getDescription(error)).to.equal('Memory error');
     });
 
+    describe('given userFriendlyDescriptionsOnly is false', function() {
+
+      it('should return the stack for a basic error', function() {
+        const error = new Error('Foo');
+        m.chai.expect(errors.getDescription(error, {
+          userFriendlyDescriptionsOnly: false
+        })).to.equal(error.stack);
+      });
+
+      it('should return the stack if the description is an empty string', function() {
+        const error = new Error('Foo');
+        error.description = '';
+        m.chai.expect(errors.getDescription(error, {
+          userFriendlyDescriptionsOnly: false
+        })).to.equal(error.stack);
+      });
+
+      it('should return the stack if the description is a blank string', function() {
+        const error = new Error('Foo');
+        error.description = '   ';
+        m.chai.expect(errors.getDescription(error, {
+          userFriendlyDescriptionsOnly: false
+        })).to.equal(error.stack);
+      });
+
+    });
+
+    describe('given userFriendlyDescriptionsOnly is true', function() {
+
+      it('should return an empty string for a basic error', function() {
+        const error = new Error('Foo');
+        m.chai.expect(errors.getDescription(error, {
+          userFriendlyDescriptionsOnly: true
+        })).to.equal('');
+      });
+
+      it('should return an empty string if the description is an empty string', function() {
+        const error = new Error('Foo');
+        error.description = '';
+        m.chai.expect(errors.getDescription(error, {
+          userFriendlyDescriptionsOnly: true
+        })).to.equal('');
+      });
+
+      it('should return an empty string if the description is a blank string', function() {
+        const error = new Error('Foo');
+        error.description = '   ';
+        m.chai.expect(errors.getDescription(error, {
+          userFriendlyDescriptionsOnly: true
+        })).to.equal('');
+      });
+
+    });
+
   });
 
   describe('.createError()', function() {