refactor: Prefer event emitting to promises cancelation

lib/basedriver/README.md

@@ -31,6 +31,6 @@ Appium Base driver has some properties that all drivers share:
 - `driver.jwpProxyAvoid` - used by mjsonwp module. You can specify what REST api routes which you want to SKIP the automatic proxy to another server (which is optional) and instead be handled by your driver.
 
 
-Base driver exposes a promise called `onUnexpectedShutdown` which is a promise which your driver must reject in cases where an unexpected error occurs and you want to signal to the appium server at large that your driver is now shutting down.
+Base driver exposes an event called `onUnexpectedShutdown` which is called when the driver is shut down unexpectedly (usually after invocation of the `startUnexpectedShutdown` method).
 
 Your driver should also implement a startUnexpectedShutdown method?

lib/basedriver/commands/session.js

@@ -78,10 +78,6 @@ commands.createSession = async function createSession (jsonwpDesiredCapabilities
     this.newCommandTimeoutMs = (this.caps.newCommandTimeout * 1000);
   }
 
-  // We need to ininitialize one onUnexpectedShutdow promise per session
-  // to avoid the promise fulfilment being propagated between sessions.
-  this.resetOnUnexpectedShutdown();
-
   log.info(`Session created with session id: ${this.sessionId}`);
 
   return [this.sessionId, caps];

lib/basedriver/driver.js

@@ -12,6 +12,7 @@ import _ from 'lodash';
 import { util } from 'appium-support';
 import { ImageElement, makeImageElementCache, getImgElFromArgs } from './image-element';
 import AsyncLock from 'async-lock';
+import { EventEmitter } from 'events';
 
 
 B.config({
@@ -24,6 +25,7 @@ const EVENT_SESSION_INIT = 'newSessionRequested';
 const EVENT_SESSION_START = 'newSessionStarted';
 const EVENT_SESSION_QUIT_START = 'quitSessionRequested';
 const EVENT_SESSION_QUIT_DONE = 'quitSessionFinished';
+const ON_UNEXPECTED_SHUTDOWN_EVENT = 'onUnexpectedShutdown';
 
 class BaseDriver extends Protocol {
 
@@ -75,8 +77,6 @@ class BaseDriver extends Protocol {
     // the settings functionality itself
     this.settings = new DeviceSettings({}, _.noop);
 
-    this.resetOnUnexpectedShutdown();
-
     // keeping track of initial opts
     this.initialOpts = _.cloneDeep(this.opts);
 
@@ -91,9 +91,25 @@ class BaseDriver extends Protocol {
     // cache the image elements
     this._imgElCache = makeImageElementCache();
 
+    // used to handle driver events
+    this.eventEmitter = new EventEmitter();
+
     this.protocol = null;
   }
 
+  /**
+   * Set a callback handler if needed to execute a custom piece of code
+   * when the driver is shut down unexpectedly. Multiple calls to this method
+   * will cause the handler to be executed mutiple times
+   *
+   * @param {Function} handler The code to be executed on unexpected shutdown.
+   * The function may accept one argument, which is the actual error instance, which
+   * caused the driver to shut down.
+   */
+  onUnexpectedShutdown (handler) {
+    this.eventEmitter.on(ON_UNEXPECTED_SHUTDOWN_EVENT, handler);
+  }
+
   /**
    * This property is used by AppiumDriver to store the data of the
    * specific driver sessions. This data can be later used to adjust
@@ -156,22 +172,6 @@ class BaseDriver extends Protocol {
     return {};
   }
 
-  /*
-   * Initialize a new onUnexpectedShutdown promise, cancelling existing one.
-   */
-  resetOnUnexpectedShutdown () {
-    if (this.onUnexpectedShutdown && !this.onUnexpectedShutdown.isFulfilled()) {
-      this.onUnexpectedShutdown.cancel();
-    }
-    this.onUnexpectedShutdown = new B((resolve, reject, onCancel) => {
-      onCancel(() => reject(new B.CancellationError()));
-      this.unexpectedShutdownDeferred = {resolve, reject};
-    });
-    this.unexpectedShutdownDeferred.promise = this.onUnexpectedShutdown;
-    // noop handler to avoid warning.
-    this.onUnexpectedShutdown.catch(() => {});
-  }
-
   // we only want subclasses to ever extend the contraints
   set desiredCapConstraints (constraints) {
     this._constraints = Object.assign(this._constraints, constraints);
@@ -325,7 +325,10 @@ class BaseDriver extends Protocol {
 
     const commandExecutor = async () => imgElId
       ? await ImageElement.execute(this, cmd, imgElId, ...args)
-      : await B.race([this[cmd](...args), this.unexpectedShutdownDeferred.promise]);
+      : await B.race([
+        this[cmd](...args),
+        new B((resolve, reject) => this.eventEmitter.on(ON_UNEXPECTED_SHUTDOWN_EVENT, reject))
+      ]);
     const res = this.isCommandsQueueEnabled && cmd !== 'executeDriverScript'
       ? await this.commandsQueueGuard.acquire(BaseDriver.name, commandExecutor)
       : await commandExecutor();
@@ -354,7 +357,7 @@ class BaseDriver extends Protocol {
   }
 
   async startUnexpectedShutdown (err = new errors.NoSuchDriverError('The driver was unexpectedly shut down!')) {
-    this.unexpectedShutdownDeferred.reject(err); // allow others to listen for this
+    this.eventEmitter.emit(ON_UNEXPECTED_SHUTDOWN_EVENT, err); // allow others to listen for this
     this.shutdownUnexpectedly = true;
     try {
       await this.deleteSession(this.sessionId);

test/basedriver/driver-e2e-tests.js

@@ -236,11 +236,15 @@ function baseDriverE2ETests (DriverClass, defaultCaps = {}) {
         });
         // make sure that the request gets to the server before our shutdown
         await B.delay(100);
+        const shutdownEventPromise = new B((resolve, reject) => {
+          setTimeout(() => reject(new Error('onUnexpectedShutdown event is expected to be fired within 5 seconds timeout')), 5000);
+          d.onUnexpectedShutdown(resolve);
+        });
         d.startUnexpectedShutdown(new Error('Crashytimes'));
         let res = await p;
         res.status.should.equal(13);
         res.value.message.should.contain('Crashytimes');
-        await d.onUnexpectedShutdown.should.be.rejectedWith('Crashytimes');
+        await shutdownEventPromise;
         d.getStatus = d._oldGetStatus;
       });
     });

test/basedriver/driver-tests.js

@@ -85,9 +85,13 @@ function baseDriverUnitTests (DriverClass, defaultCaps = {}) {
       }.bind(d);
       let cmdPromise = d.executeCommand('getStatus');
       await B.delay(10);
+      const p = new B((resolve, reject) => {
+        setTimeout(() => reject(new Error('onUnexpectedShutdown event is expected to be fired within 5 seconds timeout')), 5000);
+        d.onUnexpectedShutdown(resolve);
+      });
       d.startUnexpectedShutdown(new Error('We crashed'));
       await cmdPromise.should.be.rejectedWith(/We crashed/);
-      await d.onUnexpectedShutdown.should.be.rejectedWith(/We crashed/);
+      await p;
     });
 
     it('should not allow commands in middle of unexpected shutdown', async function () {
@@ -99,8 +103,12 @@ function baseDriverUnitTests (DriverClass, defaultCaps = {}) {
       }.bind(d);
       let caps = _.clone(defaultCaps);
       await d.createSession(caps);
+      const p = new B((resolve, reject) => {
+        setTimeout(() => reject(new Error('onUnexpectedShutdown event is expected to be fired within 5 seconds timeout')), 5000);
+        d.onUnexpectedShutdown(resolve);
+      });
       d.startUnexpectedShutdown(new Error('We crashed'));
-      await d.onUnexpectedShutdown.should.be.rejectedWith(/We crashed/);
+      await p;
       await d.executeCommand('getSession').should.be.rejectedWith(/shut down/);
     });
 
@@ -114,8 +122,12 @@ function baseDriverUnitTests (DriverClass, defaultCaps = {}) {
 
       let caps = _.clone(defaultCaps);
       await d.createSession(caps);
+      const p = new B((resolve, reject) => {
+        setTimeout(() => reject(new Error('onUnexpectedShutdown event is expected to be fired within 5 seconds timeout')), 5000);
+        d.onUnexpectedShutdown(resolve);
+      });
       d.startUnexpectedShutdown(new Error('We crashed'));
-      await d.onUnexpectedShutdown.should.be.rejectedWith(/We crashed/);
+      await p;
 
       await d.executeCommand('getSession').should.be.rejectedWith(/shut down/);
       await B.delay(500);