refactor unexpected shutdown, and fix lint so we can use upgraded appium eslint

Author: jlipps

gulpfile.js

@@ -1,4 +1,5 @@
 /* eslint no-console:0 */
+/* eslint-disable promise/prefer-await-to-callbacks */
 "use strict";
 
 // turn all logging on since we have tests that rely on npmlog logs actually
@@ -32,10 +33,12 @@ gulp.task('fixShrinkwrap', function (done) {
 boilerplate({
   build: 'appium',
   jscs: false,
+  jshint: false,
   test: {
     files: ['${testDir}/**/*-specs.js']
   },
   extraPrepublishTasks: ['fixShrinkwrap'],
+  preCommitTasks: ['eslint', 'once'],
 });
 
 // generates server arguments readme
@@ -66,7 +69,7 @@ gulp.task('docs', ['transpile'], function () {
     }
 
     // handle empty objects
-    if (JSON.stringify(argOpts.defaultValue) === '{}'){
+    if (JSON.stringify(argOpts.defaultValue) === '{}') {
       argOpts.defaultValue = '{}';
     }
 

lib/appium.js

@@ -150,18 +150,11 @@ class AppiumDriver extends BaseDriver {
     let [innerSessionId, dCaps] = await d.createSession(caps, reqCaps, curSessions);
     this.sessions[innerSessionId] = d;
 
-    // Remove the session on unexpected shutdown, so that we are in a position
-    // to open another session later on.
-    // TODO: this should be removed and replaced by a onShutdown callback.
-    d.onUnexpectedShutdown
-      .then(() => { throw new Error('Unexpected shutdown'); })
-      .catch(B.CancellationError, () => {})
-      .catch((err) => {
-        log.warn(`Closing session, cause was '${err.message}'`);
-        log.info(`Removing session ${innerSessionId} from our master session list`);
-        delete this.sessions[innerSessionId];
-      })
-      .done();
+    // this is an async function but we don't await it because it handles
+    // an out-of-band promise which is fulfilled if the inner driver
+    // unexpectedly shuts down
+    this.attachUnexpectedShutdownHandler(d, innerSessionId);
+
 
     log.info(`New ${InnerDriver.name} session created successfully, session ` +
              `${innerSessionId} added to master session list`);
@@ -172,6 +165,26 @@ class AppiumDriver extends BaseDriver {
     return [innerSessionId, dCaps];
   }
 
+  async attachUnexpectedShutdownHandler (driver, innerSessionId) {
+    // Remove the session on unexpected shutdown, so that we are in a position
+    // to open another session later on.
+    // TODO: this should be removed and replaced by a onShutdown callback.
+    try {
+      await driver.onUnexpectedShutdown; // this is a cancellable promise
+      // if we get here, we've had an unexpected shutdown, so error
+      throw new Error('Unexpected shutdown');
+    } catch (e) {
+      if (e instanceof B.CancellationError) {
+        // if we cancelled the unexpected shutdown promise, that means we
+        // no longer care about it, and can safely ignore it
+        return;
+      }
+      log.warn(`Closing session, cause was '${e.message}'`);
+      log.info(`Removing session ${innerSessionId} from our master session list`);
+      delete this.sessions[innerSessionId];
+    }
+  }
+
   curSessionDataForDriver (InnerDriver) {
     let data = _.values(this.sessions)
                 .filter((s) => s.constructor.name === InnerDriver.name)

lib/logsink.js

@@ -53,7 +53,7 @@ function timestamp () {
 // having to create 2 loggers.
 function applyStripColorPatch (transport) {
   let _log = transport.log.bind(transport);
-  transport.log = function (level, msg, meta, callback) {
+  transport.log = function (level, msg, meta, callback) { // eslint-disable-line promise/prefer-await-to-callbacks
     let code = /\u001b\[(\d+(;\d+)*)?m/g;
     msg = ('' + msg).replace(code, '');
     _log(level, msg, meta, callback);

test/driver-specs.js

@@ -9,8 +9,9 @@ import chai from 'chai';
 import chaiAsPromised from 'chai-as-promised';
 import { XCUITestDriver } from 'appium-xcuitest-driver';
 import { IosDriver } from 'appium-ios-driver';
+import { sleep } from 'asyncbox';
 
-
+chai.should();
 chai.use(chaiAsPromised);
 
 const BASE_CAPS = {platformName: 'Fake', deviceName: 'Fake', app: TEST_FAKE_APP};
@@ -172,6 +173,42 @@ describe('AppiumDriver', () => {
     });
     describe('sessionExists', () => {
     });
+    describe('attachUnexpectedShutdownHandler', () => {
+      let appium
+        , mockFakeDriver;
+      beforeEach(() => {
+        [appium, mockFakeDriver] = getDriverAndFakeDriver();
+      });
+      afterEach(() => {
+        mockFakeDriver.restore();
+        appium.args.defaultCapabilities = {};
+      });
+
+      it('should remove session if inner driver unexpectedly exits with an error', async () => {
+        let [sessionId,] = await appium.createSession(_.clone(BASE_CAPS)); // eslint-disable-line comma-spacing
+        _.keys(appium.sessions).should.contain(sessionId);
+        appium.sessions[sessionId].unexpectedShutdownDeferred.reject(new Error("Oops"));
+        // let event loop spin so rejection is handled
+        await sleep(1);
+        _.keys(appium.sessions).should.not.contain(sessionId);
+      });
+      it('should remove session if inner driver unexpectedly exits with no error', async () => {
+        let [sessionId,] = await appium.createSession(_.clone(BASE_CAPS)); // eslint-disable-line comma-spacing
+        _.keys(appium.sessions).should.contain(sessionId);
+        appium.sessions[sessionId].unexpectedShutdownDeferred.resolve();
+        // let event loop spin so rejection is handled
+        await sleep(1);
+        _.keys(appium.sessions).should.not.contain(sessionId);
+      });
+      it('should not remove session if inner driver cancels unexpected exit', async () => {
+        let [sessionId,] = await appium.createSession(_.clone(BASE_CAPS)); // eslint-disable-line comma-spacing
+        _.keys(appium.sessions).should.contain(sessionId);
+        appium.sessions[sessionId].onUnexpectedShutdown.cancel();
+        // let event loop spin so rejection is handled
+        await sleep(1);
+        _.keys(appium.sessions).should.contain(sessionId);
+      });
+    });
     describe('getDriverForCaps', () => {
       it('should not blow up if user does not provide platformName', () => {
         let appium = new AppiumDriver({});

test/helpers.js

@@ -1,5 +1,6 @@
 import path from 'path';
 import wd from 'wd';
+import B from 'bluebird';
 
 const TEST_HOST = 'localhost';
 const TEST_PORT = 4723;
@@ -18,7 +19,7 @@ function initSession (caps) {
   after(async () => {
     await driver.quit();
   });
-  return new Promise((_resolve) => {
+  return new B((_resolve) => {
     resolve = _resolve;
   });
 }