chore(appium): some cosmetic improvements to session and proxy naming etc
MODIFY
packages/appium/lib/appium.js
packages/appium/lib/appium.js
@@ -2,7 +2,8 @@ import _ from 'lodash';
import log from './logger';
import { getBuildInfo, updateBuildInfo, APPIUM_VER } from './config';
import { findMatchingDriver } from './drivers';
-import { BaseDriver, errors, isSessionCommand } from '@appium/base-driver';
+import { BaseDriver, errors, isSessionCommand,
+ CREATE_SESSION_COMMAND } from '@appium/base-driver';
import AsyncLock from 'async-lock';
import { parseCapsForInnerDriver, pullSettings } from './utils';
import { util } from '@appium/support';
@@ -282,7 +283,11 @@ class AppiumDriver extends BaseDriver {
for (const plugin of this.sessionPlugins[innerSessionId]) {
if (_.isFunction(plugin.onUnexpectedShutdown)) {
log.debug(`Plugin ${plugin.name} defines an unexpected shutdown handler; calling it now`);
+ try {
plugin.onUnexpectedShutdown(driver, cause);
+ } catch (e) {
+ log.warn(`Got an error when running plugin ${plugin.name} shutdown handler: ${e}`);
+ }
} else {
log.debug(`Plugin ${plugin.name} does not define an unexpected shutdown handler`);
}
@@ -492,12 +497,10 @@ class AppiumDriver extends BaseDriver {
// in case it calls 'await next()'. This requires that the driver have defined
// 'proxyCommand' and not just 'proxyReqRes'.
if (!dstSession.proxyCommand) {
- throw new Error(`The default behavior for this command was to proxy, but the driver ` +
- `did not have the 'proxyCommand' method defined. To fully support ` +
- `plugins, drivers should have 'proxyCommand' set to a jwpProxy object's ` +
- `'command()' method, in addition to the normal 'proxyReqRes'`);
+ throw new NoDriverProxyCommandError();
}
- return await dstSession.proxyCommand(reqForProxy.originalUrl, reqForProxy.method, reqForProxy.body);
+ return await dstSession.proxyCommand(reqForProxy.originalUrl, reqForProxy.method,
+ reqForProxy.body);
}
if (isGetStatus) {
@@ -522,13 +525,13 @@ class AppiumDriver extends BaseDriver {
// if we had plugins, make sure to log out the helpful report about which plugins ended up
// handling the command and which didn't
- plugins.length && this.logPluginHandlerReport({cmd, cmdHandledBy});
+ this.logPluginHandlerReport(plugins, {cmd, cmdHandledBy});
// And finally, if the command was createSession, we want to migrate any plugins which were
// previously sessionless to use the new sessionId, so that plugins can share state between
// their createSession method and other instance methods
- if (cmd === 'createSession' && this.sessionlessPlugins.length && !res.error) {
- const sessionId = res.value[0];
+ if (cmd === CREATE_SESSION_COMMAND && this.sessionlessPlugins.length && !res.error) {
+ const sessionId = _.first(res.value);
log.info(`Promoting ${this.sessionlessPlugins.length} sessionless plugins to be attached ` +
`to session ID ${sessionId}`);
this.sessionPlugins[sessionId] = this.sessionlessPlugins;
@@ -563,7 +566,11 @@ class AppiumDriver extends BaseDriver {
return next;
}
- logPluginHandlerReport ({cmd, cmdHandledBy}) {
+ logPluginHandlerReport (plugins, {cmd, cmdHandledBy}) {
+ if (!plugins.length) {
+ return;
+ }
+
// at the end of the day, we have an object representing which plugins ended up getting
// their code run as part of handling this command. Because plugins can choose *not* to
// pass control to other plugins or to the default driver behavior, this is information
@@ -573,7 +580,7 @@ class AppiumDriver extends BaseDriver {
const didHandle = Object.keys(cmdHandledBy).filter((k) => cmdHandledBy[k]);
const didntHandle = Object.keys(cmdHandledBy).filter((k) => !cmdHandledBy[k]);
if (didntHandle.length > 0) {
- log.info(`Command '${cmd}' was not handled by the following beahviors or plugins, even ` +
+ log.info(`Command '${cmd}' was *not* handled by the following behaviours or plugins, even ` +
`though they were registered to handle it: ${JSON.stringify(didntHandle)}. The ` +
`command *was* handled by these: ${JSON.stringify(didHandle)}.`);
}
@@ -625,4 +632,22 @@ function isAppiumDriverCommand (cmd) {
return !isSessionCommand(cmd) || cmd === 'deleteSession';
}
+/**
+ * Thrown when Appium tried to proxy a command using a driver's `proxyCommand` method but the
+ * method did not exist
+ */
+export class NoDriverProxyCommandError extends Error {
+ /**
+ * @type {Readonly<string>}
+ */
+ code = 'APPIUMERR_NO_DRIVER_PROXYCOMMAND';
+
+ constructor () {
+ super(`The default behavior for this command was to proxy, but the driver ` +
+ `did not have the 'proxyCommand' method defined. To fully support ` +
+ `plugins, drivers should have 'proxyCommand' set to a jwpProxy object's ` +
+ `'command()' method, in addition to the normal 'proxyReqRes'`);
+ }
+}
+
export { AppiumDriver };

MODIFY
packages/base-driver/index.js
packages/base-driver/index.js
@@ -21,7 +21,7 @@ const {
Protocol, routeConfiguringFunction, errors, isErrorType,
errorFromMJSONWPStatusCode, errorFromW3CJsonCode, ALL_COMMANDS, METHOD_MAP,
routeToCommandName, NO_SESSION_ID_COMMANDS, isSessionCommand,
- normalizeBasePath, determineProtocol
+ normalizeBasePath, determineProtocol, CREATE_SESSION_COMMAND, DELETE_SESSION_COMMAND,
} = protocol;
export {
@@ -29,7 +29,7 @@ export {
errorFromMJSONWPStatusCode, errorFromW3CJsonCode, determineProtocol,
errorFromMJSONWPStatusCode as errorFromCode, ALL_COMMANDS, METHOD_MAP,
routeToCommandName, NO_SESSION_ID_COMMANDS, isSessionCommand,
- DEFAULT_BASE_PATH, normalizeBasePath
+ DEFAULT_BASE_PATH, normalizeBasePath, CREATE_SESSION_COMMAND, DELETE_SESSION_COMMAND
};
// Express exports

MODIFY
packages/base-driver/lib/protocol/index.js
packages/base-driver/lib/protocol/index.js
// transpile:main
import {
- Protocol, isSessionCommand, routeConfiguringFunction, determineProtocol
+ Protocol, isSessionCommand, routeConfiguringFunction, determineProtocol,
+ CREATE_SESSION_COMMAND, DELETE_SESSION_COMMAND,
} from './protocol';
import {
NO_SESSION_ID_COMMANDS, ALL_COMMANDS, METHOD_MAP,
@@ -15,4 +16,5 @@ export {
Protocol, routeConfiguringFunction, errors, isErrorType,
errorFromMJSONWPStatusCode, errorFromW3CJsonCode, ALL_COMMANDS, METHOD_MAP,
routeToCommandName, NO_SESSION_ID_COMMANDS, isSessionCommand, determineProtocol,
+ CREATE_SESSION_COMMAND, DELETE_SESSION_COMMAND,
};

MODIFY
packages/base-driver/lib/protocol/protocol.js
packages/base-driver/lib/protocol/protocol.js
@@ -239,7 +239,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
// this command should never be proxied (which is useful for plugin developers who add
// commands and generally would not want that command to be proxied instead of handled by the
// plugin)
- let pluginOverrodeProxy = false;
+ let didPluginOverrideProxy = false;
if (isSessCmd && !spec.neverProxy && driverShouldDoJwpProxy(driver, req, spec.command)) {
if (!driver.pluginsToHandleCmd ||
driver.pluginsToHandleCmd(spec.command, req.params.sessionId).length === 0) {
@@ -249,7 +249,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
SESSIONS_CACHE.getLogger(req.params.sessionId, currentProtocol).debug(`Would have proxied ` +
`command directly, but a plugin exists which might require its value, so will let ` +
`its value be collected internally and made part of plugin chain`);
- pluginOverrodeProxy = true;
+ didPluginOverrideProxy = true;
}
// if a command is not in our method map, it's because we
@@ -291,7 +291,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
`${driver.constructor.name}.${spec.command}() with args: ` +
_.truncate(JSON.stringify(args), {length: MAX_LOG_BODY_LENGTH}));
- if (pluginOverrodeProxy) {
+ if (didPluginOverrideProxy) {
// TODO for now we add this information on the args list, but that's mixing purposes here.
// We really should add another 'options' parameter to 'executeCommand', but this would be
// a breaking change for all drivers so would need to be handled carefully.
@@ -469,5 +469,5 @@ async function doJwpProxy (driver, req, res) {
export {
Protocol, routeConfiguringFunction, isSessionCommand,
- driverShouldDoJwpProxy, determineProtocol
+ driverShouldDoJwpProxy, determineProtocol, CREATE_SESSION_COMMAND, DELETE_SESSION_COMMAND,
};

MODIFY
packages/base-driver/test/basedriver/driver-tests.js
packages/base-driver/test/basedriver/driver-tests.js
@@ -336,7 +336,7 @@ function baseDriverUnitTests (DriverClass, defaultCaps = {}) {
d.canProxy.should.be.an.instanceof(Function);
});
it('should return a boolean from #canProxy', function () {
- [true, false].should.include(d.canProxy(sessId));
+ d.canProxy(sessId).should.be.a('boolean');
});
it('should throw an error when sessionId is wrong', function () {
(() => { d.canProxy(); }).should.throw;

chore(appium): some cosmetic improvements to session and proxy naming etc