feat(CLI): replace --robot with an ETCHER_CLI_ROBOT env var (#1010)

The `--robot` option of the CLI causes the program to output
machine-parseable strings, which can be easily consumed by the GUI to
update progress and other information.

The problem is that if the CLI fails to parse its command line arguments
when being called from the GUI for whatever reason, the `.fail()` Yargs
handler will be called, which doesn't output error information in the
usual "robot" format, causing the GUI to not understand the error
message.

Since the `.fail()` Yargs handler doesn't have access to the passed
options, we moved the "robot" functionality to an environment variable,
which we can easily check from there.

As a bonus, this PR refactors the whole robot logic into
`lib/shared/robot.js` and adds unit tests to it.

See: https://github.com/resin-io/etcher/issues/986
Change-Type: major
Changelog-Entry: Replace the `--robot` CLI option with an `ETCHER_CLI_ROBOT` environment variable.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

docs/ARCHITECTURE.md

@@ -94,8 +94,9 @@ contains certain features to ease communication:
 
 - [Well-documented exit codes.][exit-codes]
 
-- A `--robot` option, which causes the Etcher CLI to output state in a way that
-can be easily machine-parsed.
+- An environment variable called `ETCHER_CLI_ROBOT` option, which when set
+  causes the Etcher CLI to output state in a way that can be easily
+  parser by a machine.
 
 Summary
 -------

docs/CLI.md

@@ -40,7 +40,6 @@ Options
   --version, -v  show version number
   --drive, -d    drive
   --check, -c    validate write
-  --robot, -r    parse-able output without interactivity
   --yes, -y      confirm non-interactively
   --unmount, -u  unmount on success
 ```
@@ -48,13 +47,13 @@ Options
 The robot option
 ----------------
 
-The `--robot` option is very particular since it allows other applications to
-easily consume the output of the Etcher CLI in real-time. When using the
-`--robot` option, the `--yes` option is implicit, therefore you need to
-manually specify `--drive`.
+Setting the `ETCHER_CLI_ROBOT` environment variable allows other applications
+to easily consume the output of the Etcher CLI in real-time When using the
+`ETCHER_CLI_ROBOT` option, the `--yes` option is implicit, therefore you need
+to manually specify `--drive`.
 
-When `--robot` is used, the program will output JSON lines containing the
-progress state and other useful information. For example:
+When `ETCHER_CLI_ROBOT` is used, the program will output JSON lines containing
+the progress state and other useful information. For example:
 
 ```
 $ sudo etcher image.iso --robot --drive /dev/disk2

lib/cli/cli.js

@@ -20,6 +20,7 @@ const _ = require('lodash');
 const fs = require('fs');
 const yargs = require('yargs');
 const utils = require('./utils');
+const robot = require('../shared/robot');
 const EXIT_CODES = require('../shared/exit-codes');
 const packageJSON = require('../../package.json');
 
@@ -63,9 +64,14 @@ module.exports = yargs
   .version(_.constant(packageJSON.version))
 
   // Error reporting
-  .fail((message, error) => {
-    yargs.showHelp();
-    utils.printError(error || message);
+  .fail(function(message, error) {
+    if (robot.isEnabled(process.env)) {
+      robot.printError(error || message);
+    } else {
+      yargs.showHelp();
+      utils.printError(error || message);
+    }
+
     process.exit(1);
   })
 
@@ -76,7 +82,7 @@ module.exports = yargs
   })
 
   .check((argv) => {
-    if (argv.robot && !argv.drive) {
+    if (robot.isEnabled(process.env) && !argv.drive) {
       throw new Error('Missing drive');
     }
 
@@ -105,11 +111,6 @@ module.exports = yargs
       alias: 'c',
       default: true
     },
-    robot: {
-      describe: 'parse-able output without interactivity',
-      boolean: true,
-      alias: 'r'
-    },
     yes: {
       describe: 'confirm non-interactively',
       boolean: true,

lib/cli/etcher.js

@@ -25,6 +25,7 @@ const drivelist = Bluebird.promisifyAll(require('drivelist'));
 const writer = require('./writer');
 const utils = require('./utils');
 const options = require('./cli');
+const robot = require('../shared/robot');
 const EXIT_CODES = require('../shared/exit-codes');
 
 isElevated().then((elevated) => {
@@ -51,7 +52,7 @@ isElevated().then((elevated) => {
       // If `options.yes` is `false`, pass `undefined`,
       // otherwise the question will not be asked because
       // `false` is a defined value.
-      yes: options.robot || options.yes || undefined
+      yes: robot.isEnabled(process.env) || options.yes || undefined
 
     }
   });
@@ -79,16 +80,13 @@ isElevated().then((elevated) => {
       validateWriteOnSuccess: options.check
     }, (state) => {
 
-      if (options.robot) {
-        console.log(JSON.stringify({
-          command: 'progress',
-          data: {
-            type: state.type,
-            percentage: Math.floor(state.percentage),
-            eta: state.eta,
-            speed: Math.floor(state.speed)
-          }
-        }));
+      if (robot.isEnabled(process.env)) {
+        robot.printMessage('progress', {
+          type: state.type,
+          percentage: Math.floor(state.percentage),
+          eta: state.eta,
+          speed: Math.floor(state.speed)
+        });
       } else {
         progressBars[state.type].update(state);
       }
@@ -98,13 +96,10 @@ isElevated().then((elevated) => {
 }).then((results) => {
 
   return Bluebird.try(() => {
-    if (options.robot) {
-      return console.log(JSON.stringify({
-        command: 'done',
-        data: {
-          sourceChecksum: results.sourceChecksum
-        }
-      }));
+    if (robot.isEnabled(process.env)) {
+      return robot.printMessage('done', {
+        sourceChecksum: results.sourceChecksum
+      });
     }
 
     console.log('Your flash is complete!');
@@ -120,16 +115,8 @@ isElevated().then((elevated) => {
 }).catch((error) => {
 
   return Bluebird.try(() => {
-    if (options.robot) {
-      return console.error(JSON.stringify({
-        command: 'error',
-        data: {
-          message: error.message,
-          description: error.description,
-          stacktrace: error.stack,
-          code: error.code
-        }
-      }));
+    if (robot.isEnabled(process.env)) {
+      return robot.printError(error);
     }
 
     utils.printError(error);

lib/shared/child-writer/index.js

@@ -24,6 +24,7 @@ const rendererUtils = require('./renderer-utils');
 const utils = require('./utils');
 const CONSTANTS = require('./constants');
 const EXIT_CODES = require('../exit-codes');
+const robot = require('../robot');
 
 /**
  * @summary Perform a write
@@ -97,32 +98,21 @@ exports.write = (image, drive, options) => {
   ipc.server.on('error', emitError);
   ipc.server.on('message', (data) => {
     let message;
-    try {
-      message = JSON.parse(data);
-    } catch (error) {
-      error.description = `${data}, ${error.message}`;
-      error.message = 'Invalid message from the writer process';
-      return emitError(error);
-    }
 
-    if (!message.command || !message.data) {
-      const error = new Error('Invalid message from the writer process');
-      error.description = `No command or data: ${data}`;
+    try {
+      message = robot.parseMessage(data);
+    } catch (error) {
       return emitError(error);
     }
 
     // The error object is decomposed by the CLI for serialisation
     // purposes. We compose it back to an `Error` here in order
     // to provide better encapsulation.
-    if (message.command === 'error') {
-      const error = new Error(message.data.message);
-      error.code = message.data.code;
-      error.description = message.data.description;
-      error.stack = message.data.stacktrace;
-      return emitError(error);
+    if (robot.getCommand(message) === 'error') {
+      return emitError(robot.recomposeErrorMessage(message));
     }
 
-    emitter.emit(message.command, message.data);
+    emitter.emit(robot.getCommand(message), robot.getData(message));
   });
 
   ipc.server.on('start', () => {

lib/shared/child-writer/utils.js

@@ -72,7 +72,6 @@ exports.getCLIWriterArguments = (options) => {
   const argv = [
     options.entryPoint,
     options.image,
-    '--robot',
     '--drive',
     options.device,
 

lib/shared/child-writer/writer-proxy.js

@@ -152,7 +152,19 @@ return isElevated().then((elevated) => {
     ipc.connectTo(process.env.IPC_SERVER_ID, () => {
       ipc.of[process.env.IPC_SERVER_ID].on('error', reject);
       ipc.of[process.env.IPC_SERVER_ID].on('connect', () => {
-        const child = childProcess.spawn(EXECUTABLE, ETCHER_ARGUMENTS);
+
+        const child = childProcess.spawn(EXECUTABLE, ETCHER_ARGUMENTS, {
+          env: {
+
+            // The CLI might call operating system utilities (like `diskutil`),
+            // so we must ensure the `PATH` is inherited.
+            PATH: process.env.PATH,
+
+            ELECTRON_RUN_AS_NODE: 1,
+            ETCHER_CLI_ROBOT: 1
+          }
+        });
+
         ipc.of[process.env.IPC_SERVER_ID].on('disconnect', _.bind(child.kill, child));
         child.on('error', reject);
         child.on('close', resolve);

lib/shared/robot.js

@@ -0,0 +1,235 @@
+/*
+ * 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');
+
+/**
+ * @summary Check whether we should emit parseable output
+ * @function
+ * @public
+ *
+ * @param {Object} environment - environment
+ * @returns {Boolean} whether we should emit parseable output
+ *
+ * @example
+ * if (robot.isEnabled(process.env)) {
+ *   console.log('We should emit parseable output');
+ * }
+ */
+exports.isEnabled = (environment) => {
+  const value = _.get(environment, 'ETCHER_CLI_ROBOT', false);
+  return Boolean(value === 'false' ? false : value);
+};
+
+/**
+ * @summary Build a machine-parseable message
+ * @function
+ * @private
+ *
+ * @param {String} title - message title
+ * @param {Object} [data] - message data
+ * @returns {String} parseable message
+ *
+ * @example
+ * const message = robot.buildMessage('progress', {
+ *   percentage: 50
+ * });
+ *
+ * console.log(message);
+ * > '{"command":"progress","data":{"percentage":50}}'
+ */
+exports.buildMessage = (title, data = {}) => {
+  if (!_.isPlainObject(data)) {
+    throw new Error(`Invalid data: ${data}`);
+  }
+
+  return JSON.stringify({
+    command: title,
+    data: data
+  });
+};
+
+/**
+ * @summary Parse a machine-parseable message
+ * @function
+ * @public
+ *
+ * @param {String} string - message string
+ * @returns {Object} parsed message
+ *
+ * @example
+ * const result = robot.parseMessage('{"command":"progress","data":{"foo":50}}');
+ * console.log(message);
+ * > {
+ * >   command: 'progress',
+ * >   data: {
+ * >     foo: 50
+ * >   }
+ * > }
+ */
+exports.parseMessage = (string) => {
+  let output;
+
+  try {
+    output = JSON.parse(string);
+  } catch (error) {
+    error.message = 'Invalid message';
+    error.description = `${string}, ${error.message}`;
+    throw error;
+  }
+
+  if (!output.command || !output.data) {
+    const error = new Error('Invalid message');
+    error.description = `No command or data: ${string}`;
+    throw error;
+  }
+
+  return output;
+};
+
+/**
+ * @summary Build a machine-parseable error message
+ * @function
+ * @private
+ *
+ * @param {(String|Error)} error - error
+ * @returns {String} parseable error message
+ *
+ * @example
+ * const error = new Error('foo');
+ * const errorMessage = robot.buildErrorMessage(error);
+ *
+ * console.log(error.command);
+ * > 'error'
+ *
+ * console.log(error.data.message);
+ * > 'foo'
+ *
+ * error.data.stacktrace === error.stack;
+ * > true
+ */
+exports.buildErrorMessage = (error) => {
+  if (_.isString(error)) {
+    error = new Error(error);
+  }
+
+  return exports.buildMessage('error', {
+    message: error.message,
+    description: error.description,
+    stacktrace: error.stack,
+    code: error.code
+  });
+};
+
+/**
+ * @summary Recompose an error message
+ * @function
+ * @public
+ *
+ * @param {String} message - error message
+ * @returns {Error} error object
+ *
+ * @example
+ * const message = robot.buildErrorMessage(new Error('foo'));
+ * const error = robot.recomposeErrorMessage(robot.parseMessage(message));
+ *
+ * error instanceof Error;
+ * > true
+ *
+ * console.log(error.message);
+ * > 'foo'
+ */
+exports.recomposeErrorMessage = (message) => {
+  const error = new Error(message.data.message);
+  _.assign(error, _.omit(message.data, 'stacktrace'));
+  error.stack = message.data.stacktrace;
+  return error;
+};
+
+/**
+ * @summary Get message command
+ * @function
+ * @public
+ *
+ * @param {Object} message - message
+ * @returns {String} command
+ *
+ * @example
+ * const command = robot.getCommand({
+ *   command: 'foo',
+ *   data: {}
+ * });
+ *
+ * console.log(command);
+ * > 'foo'
+ */
+exports.getCommand = (message) => {
+  return _.get(message, 'command');
+};
+
+/**
+ * @summary Get message data
+ * @function
+ * @public
+ *
+ * @param {Object} message - message
+ * @returns {Object} data
+ *
+ * @example
+ * const data = robot.getData({
+ *   command: 'foo',
+ *   data: {
+ *     foo: 1
+ *   }
+ * });
+ *
+ * console.log(data);
+ * > { foo: 1 }
+ */
+exports.getData = (message) => {
+  return _.get(message, 'data', {});
+};
+
+/**
+ * @summary Print an error in a machine-friendly way
+ * @function
+ * @public
+ *
+ * @param {(Error|String)} error - error
+ *
+ * @example
+ * robot.printError(new Error('This is an error'));
+ */
+exports.printError = (error) => {
+  console.error(exports.buildErrorMessage(error));
+};
+
+/**
+ * @summary Print a message in a machine-friendly way
+ * @function
+ * @public
+ *
+ * @param {String} message - message
+ * @param {Object} [data] - data
+ *
+ * @example
+ * robot.printMessage('progress', { percentage: 50 });
+ */
+exports.printMessage = (message, data) => {
+  console.log(exports.buildMessage(message, data));
+};

tests/shared/robot.spec.js

@@ -0,0 +1,256 @@
+/*
+ * 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 robot = require('../../lib/shared/robot');
+
+describe('Shared: Robot', function() {
+
+  describe('.isEnabled()', function() {
+
+    it('should return false if ETCHER_CLI_ROBOT is not set', function() {
+      m.chai.expect(robot.isEnabled({})).to.be.false;
+    });
+
+    it('should return true if ETCHER_CLI_ROBOT=1', function() {
+      m.chai.expect(robot.isEnabled({
+        ETCHER_CLI_ROBOT: 1
+      })).to.be.true;
+    });
+
+    it('should return false if ETCHER_CLI_ROBOT=0', function() {
+      m.chai.expect(robot.isEnabled({
+        ETCHER_CLI_ROBOT: 0
+      })).to.be.false;
+    });
+
+    it('should return true if ETCHER_CLI_ROBOT="true"', function() {
+      m.chai.expect(robot.isEnabled({
+        ETCHER_CLI_ROBOT: 'true'
+      })).to.be.true;
+    });
+
+    it('should return false if ETCHER_CLI_ROBOT="false"', function() {
+      m.chai.expect(robot.isEnabled({
+        ETCHER_CLI_ROBOT: 'false'
+      })).to.be.false;
+    });
+
+    it('should return true if ETCHER_CLI_ROBOT=true', function() {
+      m.chai.expect(robot.isEnabled({
+        ETCHER_CLI_ROBOT: true
+      })).to.be.true;
+    });
+
+    it('should return false if ETCHER_CLI_ROBOT=false', function() {
+      m.chai.expect(robot.isEnabled({
+        ETCHER_CLI_ROBOT: false
+      })).to.be.false;
+    });
+
+  });
+
+  describe('.buildMessage()', function() {
+
+    it('should build a message without data', function() {
+      const message = robot.buildMessage('hello');
+      const result = '{"command":"hello","data":{}}';
+      m.chai.expect(message).to.equal(result);
+    });
+
+    it('should build a message with data', function() {
+      const message = robot.buildMessage('hello', {
+        foo: 1,
+        bar: 2
+      });
+      const result = '{"command":"hello","data":{"foo":1,"bar":2}}';
+      m.chai.expect(message).to.equal(result);
+    });
+
+    it('should throw if data is defined but it not an object', function() {
+      m.chai.expect(() => {
+        robot.buildMessage('hello', 'world');
+      }).to.throw('Invalid data: world');
+    });
+
+  });
+
+  describe('.buildErrorMessage()', function() {
+
+    it('should build a message from a simple error', function() {
+      const error = new Error('foo');
+      const message = robot.buildErrorMessage(error);
+
+      m.chai.expect(JSON.parse(message)).to.deep.equal({
+        command: 'error',
+        data: {
+          message: 'foo',
+          stacktrace: error.stack
+        }
+      });
+    });
+
+    it('should save the error description', function() {
+      const error = new Error('foo');
+      error.description = 'error description';
+      const message = robot.buildErrorMessage(error);
+
+      m.chai.expect(JSON.parse(message)).to.deep.equal({
+        command: 'error',
+        data: {
+          message: 'foo',
+          description: 'error description',
+          stacktrace: error.stack
+        }
+      });
+    });
+
+    it('should save the error code', function() {
+      const error = new Error('foo');
+      error.code = 'MYERROR';
+      const message = robot.buildErrorMessage(error);
+
+      m.chai.expect(JSON.parse(message)).to.deep.equal({
+        command: 'error',
+        data: {
+          message: 'foo',
+          code: 'MYERROR',
+          stacktrace: error.stack
+        }
+      });
+    });
+
+    it('should handle a string error', function() {
+      const message = JSON.parse(robot.buildErrorMessage('foo'));
+      m.chai.expect(message.data.message).to.equal('foo');
+      m.chai.expect(message.data.stacktrace).to.be.a.string;
+      m.chai.expect(_.isEmpty(message.data.stacktrace)).to.be.false;
+    });
+
+  });
+
+  describe('.parseMessage()', function() {
+
+    it('should parse a valid message', function() {
+      const message = robot.buildMessage('foo', {
+        bar: 1
+      });
+
+      m.chai.expect(robot.parseMessage(message)).to.deep.equal({
+        command: 'foo',
+        data: {
+          bar: 1
+        }
+      });
+    });
+
+    it('should parse a valid without data', function() {
+      const message = robot.buildMessage('foo');
+      m.chai.expect(robot.parseMessage(message)).to.deep.equal({
+        command: 'foo',
+        data: {}
+      });
+    });
+
+    it('should throw if input is not valid JSON', function() {
+      m.chai.expect(() => {
+        robot.parseMessage('Hello world\nFoo Bar');
+      }).to.throw('Invalid message');
+    });
+
+    it('should throw if input has no command', function() {
+      m.chai.expect(() => {
+        robot.parseMessage('{"data":{"foo":"bar"}}');
+      }).to.throw('Invalid message');
+    });
+
+    it('should throw if input has no data', function() {
+      m.chai.expect(() => {
+        robot.parseMessage('{"command":"foo"}');
+      }).to.throw('Invalid message');
+    });
+
+  });
+
+  describe('.getCommand()', function() {
+
+    it('should get the command of a message', function() {
+      const message = robot.parseMessage(robot.buildMessage('hello', {
+        foo: 1,
+        bar: 2
+      }));
+
+      m.chai.expect(robot.getCommand(message)).to.equal('hello');
+    });
+
+  });
+
+  describe('.getData()', function() {
+
+    it('should get the data of a message', function() {
+      const message = robot.parseMessage(robot.buildMessage('hello', {
+        foo: 1,
+        bar: 2
+      }));
+
+      m.chai.expect(robot.getData(message)).to.deep.equal({
+        foo: 1,
+        bar: 2
+      });
+    });
+
+    it('should return an empty object if the message has no data', function() {
+      m.chai.expect(robot.getData({
+        command: 'foo'
+      })).to.deep.equal({});
+    });
+
+  });
+
+  describe('.recomposeErrorMessage()', function() {
+
+    it('should return an instance of Error', function() {
+      const error = new Error('Foo bar');
+      const message = robot.parseMessage(robot.buildErrorMessage(error));
+      m.chai.expect(robot.recomposeErrorMessage(message)).to.be.an.instanceof(Error);
+    });
+
+    it('should be able to recompose an error object', function() {
+      const error = new Error('Foo bar');
+      const message = robot.parseMessage(robot.buildErrorMessage(error));
+      m.chai.expect(robot.recomposeErrorMessage(message)).to.deep.equal(error);
+    });
+
+    it('should be able to recompose an error object with a code', function() {
+      const error = new Error('Foo bar');
+      error.code = 'FOO';
+      const message = robot.parseMessage(robot.buildErrorMessage(error));
+      m.chai.expect(robot.recomposeErrorMessage(message)).to.deep.equal(error);
+    });
+
+    it('should be able to recompose an error object with a description', function() {
+      const error = new Error('Foo bar');
+      error.description = 'My description';
+      const message = robot.parseMessage(robot.buildErrorMessage(error));
+      m.chai.expect(robot.recomposeErrorMessage(message)).to.deep.equal(error);
+    });
+
+  });
+
+});