diff --git a/lib/basedriver/README.md b/lib/basedriver/README.md index d0d34a775..b38e154b3 100644 --- a/lib/basedriver/README.md +++ b/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? diff --git a/lib/basedriver/commands/session.js b/lib/basedriver/commands/session.js index 53a6d3411..c5b9f2195 100644 --- a/lib/basedriver/commands/session.js +++ b/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]; diff --git a/lib/basedriver/driver.js b/lib/basedriver/driver.js index 769e020d4..021fdcc2d 100644 --- a/lib/basedriver/driver.js +++ b/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); diff --git a/test/basedriver/driver-e2e-tests.js b/test/basedriver/driver-e2e-tests.js index e404788db..1569c4a02 100644 --- a/test/basedriver/driver-e2e-tests.js +++ b/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; }); }); diff --git a/test/basedriver/driver-tests.js b/test/basedriver/driver-tests.js index 1abcc83f1..9fb49af57 100644 --- a/test/basedriver/driver-tests.js +++ b/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);