Fix protocol detection inside the routing function
MODIFY
lib/basedriver/capabilities.js
lib/basedriver/capabilities.js
@@ -199,7 +199,7 @@ function processCapabilities (caps, constraints = {}, shouldValidateCaps = true)
const {matchedCaps, validationErrors} = parseCaps(caps, constraints, shouldValidateCaps);
// If we found an error throw an exception
- if (_.isNull(matchedCaps)) {
+ if (!util.hasValue(matchedCaps)) {
if (_.isArray(caps.firstMatch) && caps.firstMatch.length > 1) {
// If there was more than one 'firstMatch' cap, indicate that we couldn't find a matching capabilities set and show all the errors
throw new errors.InvalidArgumentError(`Could not find matching capabilities from ${JSON.stringify(caps)}:\n ${validationErrors.join('\n')}`);

MODIFY
lib/basedriver/commands/session.js
lib/basedriver/commands/session.js
@@ -38,7 +38,7 @@ commands.createSession = async function (jsonwpDesiredCapabilities, jsonwpRequir
} else {
this.setProtocolMJSONWP();
log.debug(`Creating session with MJSONWP desired capabilities: ${_.truncate(JSON.stringify(jsonwpDesiredCapabilities))}`);
- caps = jsonwpDesiredCapabilities;
+ caps = jsonwpDesiredCapabilities || {};
}
caps = fixCaps(caps, this.desiredCapConstraints);

MODIFY
lib/protocol/errors.js
lib/protocol/errors.js
@@ -6,12 +6,14 @@ import HTTPStatusCodes from 'http-status-codes';
const mjsonwpLog = logger.getLogger('MJSONWP');
const w3cLog = logger.getLogger('W3C');
+const W3C_UNKNOWN_ERROR = 'unknown error';
+
// base error class for all of our errors
class ProtocolError extends ES6Error {
constructor (msg, jsonwpCode, w3cStatus, error) {
super(msg);
this.jsonwpCode = jsonwpCode;
- this.error = error || 'An unknown error has occurred';
+ this.error = error || W3C_UNKNOWN_ERROR;
if (this.jsonwpCode === null) {
this.jsonwpCode = 13;
}
@@ -160,7 +162,7 @@ class UnknownError extends ProtocolError {
return HTTPStatusCodes.INTERNAL_SERVER_ERROR;
}
static error () {
- return 'unknown error';
+ return W3C_UNKNOWN_ERROR;
}
constructor (originalError) {
let origMessage = originalError;

MODIFY
lib/protocol/protocol.js
lib/protocol/protocol.js
@@ -7,10 +7,11 @@ import { METHOD_MAP, NO_SESSION_ID_COMMANDS } from './routes';
import { renameKey } from '../basedriver/helpers';
import B from 'bluebird';
import BaseDriver from '../basedriver/driver';
-
+import LRU from 'lru-cache';
const mjsonwpLog = logger.getLogger('MJSONWP');
const w3cLog = logger.getLogger('W3C');
+const genericProtocolLog = logger.getLogger('GENERIC');
const JSONWP_SUCCESS_STATUS_CODE = 0;
// TODO: Make this value configurable as a server side capability
@@ -20,6 +21,9 @@ const MJSONWP_ELEMENT_KEY = 'ELEMENT';
const W3C_ELEMENT_KEY = 'element-6066-11e4-a52e-4f735466cecf';
const IMAGE_ELEMENT_PREFIX = 'appium-image-element-';
+const CREATE_SESSION_COMMAND = 'createSession';
+const DELETE_SESSION_COMMAND = 'deleteSession';
+
const IMG_EL_BODY_RE = new RegExp(
`"(${W3C_ELEMENT_KEY}|${MJSONWP_ELEMENT_KEY})":\s*` +
`"${IMAGE_ELEMENT_PREFIX}[^"]+"`
@@ -31,8 +35,46 @@ const IMG_EL_URL_RE = new RegExp(
class Protocol {}
-function getLogByProtocol (driver) {
- return driver.isW3CProtocol() ? w3cLog : mjsonwpLog;
+function getLogByProtocol (protocol) {
+ switch (protocol) {
+ case BaseDriver.DRIVER_PROTOCOL.W3C:
+ return w3cLog;
+ case BaseDriver.DRIVER_PROTOCOL.MJSONWP:
+ return mjsonwpLog;
+ default:
+ return genericProtocolLog;
+ }
+}
+
+// This cache is useful when a session gets terminated
+// and removed from the sessions list in the umbrella driver,
+// but the client still tries to send a command to this session id.
+// So we know how to properly wrap the error message for it
+const PROTOCOLS_MAPPING_CACHE = new LRU({
+ max: 100,
+});
+
+function cacheProtocolValue (value, sessionId) {
+ if (sessionId && value) {
+ PROTOCOLS_MAPPING_CACHE.set(sessionId, value);
+ }
+ return value;
+}
+
+function extractProtocol (driver, sessionId = null) {
+ if (!_.isFunction(driver.driverForSession) || driver.driverForSession(sessionId) === driver) {
+ // Shortcircuit if the driver instance is not an umbrella driver
+ // or it is Fake driver instance, where `driver.driverForSession`
+ // always returns self instance
+ return driver.protocol;
+ }
+ if (!sessionId) {
+ return null;
+ }
+
+ // Extract the protocol for the current session if the given driver is the umbrella one
+ const dstDriver = driver.driverForSession(sessionId);
+ return dstDriver ? dstDriver.protocol : PROTOCOLS_MAPPING_CACHE.get(sessionId);
}
function isSessionCommand (command) {
@@ -210,11 +252,10 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
let httpResBody = {};
let httpStatus = 200;
let newSessionId;
- let isW3C;
- let isMJSONWP;
+ let currentProtocol = extractProtocol(driver, req.params.sessionId);
// get the appropriate logger depending on the protocol that is being used
- const log = getLogByProtocol(driver);
+ const log = getLogByProtocol(currentProtocol);
try {
// if this is a session command but we don't have a session,
@@ -250,13 +291,19 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
jsonObj = unwrapParams(spec.payloadParams, jsonObj);
}
+ if (spec.command === CREATE_SESSION_COMMAND) {
+ // try to determine protocol by session creation args, so we can throw a
+ // properly formatted error if arguments validation fails
+ currentProtocol = BaseDriver.determineProtocol(...(makeArgs(
+ req.params, jsonObj, spec.payloadParams || {}, currentProtocol)));
+ }
+
// ensure that the json payload conforms to the spec
- checkParams(spec.payloadParams, jsonObj, driver.protocol);
- // ensure the session the user is trying to use is valid
+ checkParams(spec.payloadParams, jsonObj, currentProtocol);
// turn the command and json payload into an argument list for
// the driver methods
- let args = makeArgs(req.params, jsonObj, spec.payloadParams || {}, driver.protocol);
+ let args = makeArgs(req.params, jsonObj, spec.payloadParams || {}, currentProtocol);
let driverRes;
// validate command args according to MJSONWP
if (validators[spec.command]) {
@@ -273,42 +320,40 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
driverRes = await driver.execute(spec.command, ...args);
}
- // Get the protocol after executeCommand (when command is `createSession`, protocol is assigned within
- // createSession function)
- isW3C = driver.isW3CProtocol();
- isMJSONWP = driver.isMjsonwpProtocol();
+ // Get the protocol after executeCommand
+ currentProtocol = extractProtocol(driver, req.params.sessionId) || currentProtocol;
// If `executeCommand` was overridden and the method returns an object
// with a protocol and value/error property, re-assign the protocol
const parsedDriverRes = parseProtocol(driverRes);
if (parsedDriverRes) {
- isW3C = parsedDriverRes.isW3C;
- isMJSONWP = parsedDriverRes.isMJSONWP;
+ currentProtocol = parsedDriverRes.protocol || currentProtocol;
if (parsedDriverRes.error) {
throw parsedDriverRes.error;
}
driverRes = parsedDriverRes.value;
}
- // Use the logger that's specific to this response
- const protocolLogger = isW3C ? w3cLog : mjsonwpLog;
-
// unpack createSession response
- if (spec.command === 'createSession') {
+ if (spec.command === CREATE_SESSION_COMMAND) {
newSessionId = driverRes[0];
- if (isMJSONWP) {
+ cacheProtocolValue(currentProtocol, newSessionId);
+ if (currentProtocol === BaseDriver.DRIVER_PROTOCOL.MJSONWP) {
driverRes = driverRes[1];
- } else if (isW3C) {
+ } else if (currentProtocol === BaseDriver.DRIVER_PROTOCOL.W3C) {
driverRes = {
capabilities: driverRes[1],
};
}
}
+ // Use the logger that's specific to this response
+ const protocolLogger = getLogByProtocol(currentProtocol);
+
// If the MJSONWP element key format (ELEMENT) was provided translate it to W3C element key format (element-6066-11e4-a52e-4f735466cecf)
// and vice-versa
if (driverRes) {
- if (isW3C) {
+ if (currentProtocol === BaseDriver.DRIVER_PROTOCOL.W3C) {
driverRes = renameKey(driverRes, MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY);
} else {
driverRes = renameKey(driverRes, W3C_ELEMENT_KEY, MJSONWP_ELEMENT_KEY);
@@ -321,7 +366,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
}
// delete should not return anything even if successful
- if (spec.command === 'deleteSession') {
+ if (spec.command === DELETE_SESSION_COMMAND) {
protocolLogger.debug(`Received response: ${_.truncate(JSON.stringify(driverRes), {length: LOG_OBJ_LENGTH})}`);
protocolLogger.debug('But deleting session, so not returning');
driverRes = null;
@@ -337,7 +382,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
}
// Response status should be the status set by the driver response.
- if (!isW3C) {
+ if (currentProtocol !== BaseDriver.DRIVER_PROTOCOL.W3C) {
httpResBody.status = (_.isNil(driverRes) || _.isUndefined(driverRes.status)) ? JSONWP_SUCCESS_STATUS_CODE : driverRes.status;
}
httpResBody.value = driverRes;
@@ -348,16 +393,10 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
// based on the type of error that we encountered
let actualErr = err;
- if (_.isUndefined(isMJSONWP)) {
- isMJSONWP = driver.isMjsonwpProtocol();
- }
-
- if (_.isUndefined(isW3C)) {
- isW3C = driver.isW3CProtocol();
- }
+ currentProtocol = extractProtocol(driver, req.params.sessionId || newSessionId) || currentProtocol;
// Use the logger that's specific to this response
- const protocolLogger = isW3C ? w3cLog : mjsonwpLog;
+ const protocolLogger = getLogByProtocol(currentProtocol);
let errMsg = err.stacktrace || err.stack;
if (!errMsg.includes(err.message)) {
@@ -370,9 +409,9 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
actualErr = err.getActualError();
}
- if (isW3C) {
+ if (currentProtocol === BaseDriver.DRIVER_PROTOCOL.W3C) {
[httpStatus, httpResBody] = getResponseForW3CError(actualErr);
- } else if (isMJSONWP) {
+ } else if (currentProtocol === BaseDriver.DRIVER_PROTOCOL.MJSONWP) {
[httpStatus, httpResBody] = getResponseForJsonwpError(actualErr);
} else {
// If it's unknown what the protocol is (like if it's `getStatus` prior to `createSession`), merge the responses
@@ -395,7 +434,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
res.status(httpStatus).send(httpResBody);
} else {
if (newSessionId) {
- if (isW3C) {
+ if (currentProtocol === BaseDriver.DRIVER_PROTOCOL.W3C) {
httpResBody.value.sessionId = newSessionId;
} else {
httpResBody.sessionId = newSessionId;
@@ -405,7 +444,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
}
// Don't include sessionId in W3C responses
- if (isW3C) {
+ if (currentProtocol === BaseDriver.DRIVER_PROTOCOL.W3C) {
delete httpResBody.sessionId;
}
res.status(httpStatus).json(httpResBody);
@@ -457,7 +496,8 @@ function driverShouldDoJwpProxy (driver, req, command) {
}
async function doJwpProxy (driver, req, res) {
- getLogByProtocol(driver).info('Driver proxy active, passing request on via HTTP proxy');
+ const log = getLogByProtocol(extractProtocol(driver, req.params.sessionId));
+ log.info('Driver proxy active, passing request on via HTTP proxy');
// check that the inner driver has a proxy function
if (!driver.canProxy(req.params.sessionId)) {

MODIFY
test/protocol/fake-driver.js
test/protocol/fake-driver.js
import { errors, BaseDriver } from '../..';
import _ from 'lodash';
+import UUID from 'uuid-js';
class FakeDriver extends BaseDriver {
@@ -22,7 +23,8 @@ class FakeDriver extends BaseDriver {
}
async createSession (desiredCapabilities, requiredCapabilities, capabilities) {
- this.sessionId = "1234";
+ // Use a counter to make sure each session has a unique id
+ this.sessionId = `fakeSession_${UUID.create().hex}`;
if (capabilities) {
return [this.sessionId, capabilities];
} else {

MODIFY
test/protocol/protocol-e2e-specs.js
test/protocol/protocol-e2e-specs.js
@@ -456,7 +456,7 @@ describe('Protocol', async function () {
let sessionUrl;
beforeEach(async function () {
- // Start a session
+ // Start a W3C session
let {value} = await request.post(`${baseUrl}/session`, {
json: {
capabilities: w3cCaps,
@@ -903,7 +903,7 @@ describe('Protocol', async function () {
});
should.exist(res.sessionId);
- res.sessionId.should.equal('1234');
+ res.sessionId.indexOf('fakeSession_').should.equal(0);
res.value.should.eql({greeting: 'hello', valediction: 'bye'});
});
});
@@ -988,7 +988,9 @@ describe('Protocol', async function () {
res.statusCode.should.equal(500);
res.body.should.eql({
status: 35,
- "value": { "message": "No such context found."},
+ value: {
+ message: "No such context found."
+ },
sessionId: "foo"
});
});

Fix protocol detection inside the routing function (#257)