fix(child-writer): Handle exits due to a signal (#1843)

This adds handling for cases where the writer child process
exits due to reception of a signal, while also adjusting some peripherals,
like the IPC socket directory and inherited process environment.

Another addition is that the child process is explicitly killed
should an error arise on the IPC.

Change-Type: patch

lib/child-writer/constants.js

@@ -17,6 +17,7 @@
 'use strict'
 
 const path = require('path')
+const os = require('os')
 
 /**
  * @summary Child writer constants
@@ -25,6 +26,13 @@ const path = require('path')
  */
 module.exports = {
 
+  /**
+   * @property {String} TMP_DIRECTORY
+   * @memberof CONSTANTS
+   * @constant
+   */
+  TMP_DIRECTORY: process.env.XDG_RUNTIME_DIR || os.tmpdir(),
+
   /**
    * @property {String} PROJECT_ROOT
    * @memberof CONSTANTS

lib/child-writer/index.js

@@ -26,6 +26,16 @@ const CONSTANTS = require('./constants')
 const EXIT_CODES = require('../shared/exit-codes')
 const robot = require('../shared/robot')
 
+// There might be multiple Etcher instances running at
+// the same time, therefore we must ensure each IPC
+// server/client has a different name.
+process.env.IPC_SERVER_ID = `etcher-server-${process.pid}`
+process.env.IPC_CLIENT_ID = `etcher-client-${process.pid}`
+
+ipc.config.id = process.env.IPC_SERVER_ID
+ipc.config.socketRoot = CONSTANTS.TMP_DIRECTORY
+ipc.config.silent = true
+
 /**
  * @summary Perform a write
  * @function
@@ -67,14 +77,6 @@ exports.write = (image, drive, options) => {
     unmountOnSuccess: options.unmountOnSuccess
   })
 
-  // There might be multiple Etcher instances running at
-  // the same time, therefore we must ensure each IPC
-  // server/client has a different name.
-  process.env.IPC_SERVER_ID = `etcher-server-${process.pid}`
-  process.env.IPC_CLIENT_ID = `etcher-client-${process.pid}`
-
-  ipc.config.id = process.env.IPC_SERVER_ID
-  ipc.config.silent = true
   ipc.serve()
 
   /**
@@ -192,7 +194,7 @@ exports.write = (image, drive, options) => {
 
     child.on('error', emitError)
 
-    child.on('close', (code) => {
+    child.on('exit', (code, signal) => {
       terminateServer()
 
       if (code === EXIT_CODES.CANCELLED) {
@@ -207,7 +209,11 @@ exports.write = (image, drive, options) => {
         return null
       }
 
-      return emitError(new Error(`Child process exited with error code: ${code}`))
+      const error = new Error(`Child process exited with code ${code}, signal ${signal}`)
+      error.code = code
+      error.signal = signal
+
+      return emitError(error)
     })
   })
 

lib/child-writer/writer-proxy.js

@@ -27,6 +27,9 @@ const EXIT_CODES = require('../shared/exit-codes')
 const robot = require('../shared/robot')
 const permissions = require('../shared/permissions')
 const packageJSON = require('../../package.json')
+const CONSTANTS = require('./constants')
+
+/* eslint-disable no-eq-null */
 
 // This script is in charge of spawning the writer process and
 // ensuring it has the necessary privileges. It might look a bit
@@ -64,6 +67,18 @@ const OPTIONS_INDEX_START = 2
  */
 const etcherArguments = process.argv.slice(OPTIONS_INDEX_START)
 
+ipc.config.id = process.env.IPC_CLIENT_ID
+ipc.config.socketRoot = CONSTANTS.TMP_DIRECTORY
+ipc.config.silent = true
+
+// > If set to 0, the client will NOT try to reconnect.
+// See https://github.com/RIAEvangelist/node-ipc/
+//
+// The purpose behind this change is for this process
+// to emit a "disconnect" event as soon as the GUI
+// process is closed, so we can kill the CLI as well.
+ipc.config.stopRetrying = 0
+
 permissions.isElevated().then((elevated) => {
   if (!elevated) {
     console.log('Attempting to elevate')
@@ -109,62 +124,84 @@ permissions.isElevated().then((elevated) => {
   console.log('Re-spawning with elevation')
 
   return new Bluebird((resolve, reject) => {
-    ipc.config.id = process.env.IPC_CLIENT_ID
+    let child = null
-    ipc.config.silent = true
 
-    // > If set to 0, the client will NOT try to reconnect.
+    /**
-    // See https://github.com/RIAEvangelist/node-ipc/
+     * @summary Emit an object message to the IPC server
-    //
+     * @function
-    // The purpose behind this change is for this process
+     * @private
-    // to emit a "disconnect" event as soon as the GUI
+     *
-    // process is closed, so we can kill the CLI as well.
+     * @param {Buffer} data - json message data
-    ipc.config.stopRetrying = 0
+     *
+     * @example
+     * emitMessage(Buffer.from(JSON.stringify({
+     *   foo: 'bar'
+     * })));
+     */
+    const emitMessage = (data) => {
+      // Output from stdout/stderr coming from the CLI might be buffered,
+      // causing several progress lines to come up at once as single message.
+      // Trying to parse multiple JSON objects separated by new lines will
+      // of course make the parser confused, causing errors later on.
+      _.each(utils.splitObjectLines(data.toString()), (object) => {
+        ipc.of[process.env.IPC_SERVER_ID].emit('message', object)
+      })
+    }
+
+    /**
+     * @summary Emit an error message over the IPC and shut down the child
+     * @function
+     * @private
+     * @param {Error} error - error
+     * @example
+     * onError(error)
+     */
+    const onError = (error) => {
+      ipc.of[process.env.IPC_SERVER_ID].emit('message', {
+        error: error.message,
+        data: error.stack
+      })
+      child && child.kill()
+      reject(error)
+    }
 
     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('error', onError)
+      ipc.of[process.env.IPC_SERVER_ID].on('disconnect', () => {
+        onError(new Error('Writer process disconnected'))
+      })
+
       ipc.of[process.env.IPC_SERVER_ID].on('connect', () => {
-        const child = childProcess.spawn(executable, etcherArguments, {
+        // Inherit the parent evnironment
-          env: {
+        const childEnv = _.assign({}, process.env, {
 
-            // The CLI might call operating system utilities (like `diskutil`),
+          // The CLI might call operating system utilities (like `diskutil`),
-            // so we must ensure the `PATH` is inherited.
+          // so we must ensure the `PATH` is inherited.
-            PATH: process.env.PATH,
+          PATH: process.env.PATH,
 
-            ELECTRON_RUN_AS_NODE: 1,
+          ELECTRON_RUN_AS_NODE: 1,
-            ETCHER_CLI_ROBOT: 1,
+          ETCHER_CLI_ROBOT: 1,
 
-            // Enable extra logging from mountutils
+          // Enable extra logging from mountutils
-            // See https://github.com/resin-io-modules/mountutils
+          // See https://github.com/resin-io-modules/mountutils
-            MOUNTUTILS_DEBUG: 1
+          MOUNTUTILS_DEBUG: 1
-
-          }
         })
 
-        ipc.of[process.env.IPC_SERVER_ID].on('disconnect', _.bind(child.kill, child))
+        child = childProcess.spawn(executable, etcherArguments, {
-        child.on('error', reject)
+          env: childEnv
-        child.on('close', resolve)
+        })
 
-        /**
+        child.on('error', onError)
-         * @summary Emit an object message to the IPC server
+        child.on('exit', (code, signal) => {
-         * @function
+          if (code != null && signal == null) {
-         * @private
+            resolve(code)
-         *
+          } else {
-         * @param {Buffer} data - json message data
+            const error = new Error(`Exited with code ${code}, signal ${signal}`)
-         *
+            error.code = code
-         * @example
+            error.signal = signal
-         * emitMessage(Buffer.from(JSON.stringify({
+            reject(error)
-         *   foo: 'bar'
+          }
-         * })));
+        })
-         */
-        const emitMessage = (data) => {
-          // Output from stdout/stderr coming from the CLI might be buffered,
-          // causing several progress lines to come up at once as single message.
-          // Trying to parse multiple JSON objects separated by new lines will
-          // of course make the parser confused, causing errors later on.
-          _.each(utils.splitObjectLines(data.toString()), (object) => {
-            ipc.of[process.env.IPC_SERVER_ID].emit('message', object)
-          })
-        }
 
         child.stdout.on('data', emitMessage)
         child.stderr.on('data', emitMessage)