refactor: Prefer event emitting to promises cancelation
BREAKING CHANGE: The current refactoring is breaking and changes `onUnexpectedShutdown` property to a function, which accepts a handler that is invoked by the driver when it gets shut down unexpectedly
MODIFY
lib/basedriver/README.md
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?

MODIFY
lib/basedriver/commands/session.js
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];

MODIFY
lib/basedriver/driver.js
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);

MODIFY
test/basedriver/driver-e2e-tests.js
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;
});
});

MODIFY
test/basedriver/driver-tests.js
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);

refactor: Prefer event emitting to promises cancelation (#370)

BREAKING CHANGE: The current refactoring is breaking and changes `onUnexpectedShutdown` property to a function, which accepts a handler that is invoked by the driver when it gets shut down unexpectedly