Fix the way current protocol is determined from execute response
MODIFY
lib/protocol/protocol.js
lib/protocol/protocol.js
@@ -62,18 +62,17 @@ function cacheProtocolValue (value, sessionId) {
}
function extractProtocol (driver, sessionId = null) {
- if (!_.isFunction(driver.driverForSession) || driver.driverForSession(sessionId) === driver) {
+ const dstDriver = _.isFunction(driver.driverForSession)
+ ? driver.driverForSession(sessionId)
+ : driver;
+ if (dstDriver === 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);
}
@@ -254,9 +253,6 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
let newSessionId;
let currentProtocol = extractProtocol(driver, req.params.sessionId);
- // get the appropriate logger depending on the protocol that is being used
- const log = getLogByProtocol(currentProtocol);
-
try {
// if this is a session command but we don't have a session,
// error out early (especially before proxying)
@@ -294,8 +290,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
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)));
+ currentProtocol = BaseDriver.determineProtocol(...makeArgs(req.params, jsonObj, spec.payloadParams || {}));
}
// ensure that the json payload conforms to the spec
@@ -311,7 +306,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
}
// run the driver command wrapped inside the argument validators
- log.debug(`Calling ${driver.constructor.name}.${spec.command}() with args: ` +
+ getLogByProtocol(currentProtocol).debug(`Calling ${driver.constructor.name}.${spec.command}() with args: ` +
_.truncate(JSON.stringify(args), {length: LOG_OBJ_LENGTH}));
if (driver.executeCommand) {
@@ -325,19 +320,20 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
// 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) {
- currentProtocol = parsedDriverRes.protocol || currentProtocol;
- if (parsedDriverRes.error) {
- throw parsedDriverRes.error;
+ if (_.isPlainObject(driverRes) && _.has(driverRes, 'protocol')) {
+ currentProtocol = driverRes.protocol || currentProtocol;
+ if (driverRes.error) {
+ throw driverRes.error;
}
- driverRes = parsedDriverRes.value;
+ driverRes = driverRes.value;
}
// unpack createSession response
if (spec.command === CREATE_SESSION_COMMAND) {
newSessionId = driverRes[0];
cacheProtocolValue(currentProtocol, newSessionId);
+ getLogByProtocol(currentProtocol)
+ .debug(`Cached the protocol value '${currentProtocol}' for the new session ${newSessionId}`);
if (currentProtocol === BaseDriver.DRIVER_PROTOCOL.MJSONWP) {
driverRes = driverRes[1];
} else if (currentProtocol === BaseDriver.DRIVER_PROTOCOL.W3C) {
@@ -346,10 +342,6 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
};
}
}
-
- // 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) {
@@ -367,8 +359,10 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
// delete should not return anything even if successful
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');
+ getLogByProtocol(currentProtocol)
+ .debug(`Received response: ${_.truncate(JSON.stringify(driverRes), {length: LOG_OBJ_LENGTH})}`);
+ getLogByProtocol(currentProtocol)
+ .debug('But deleting session, so not returning');
driverRes = null;
}
@@ -386,17 +380,14 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
httpResBody.status = (_.isNil(driverRes) || _.isUndefined(driverRes.status)) ? JSONWP_SUCCESS_STATUS_CODE : driverRes.status;
}
httpResBody.value = driverRes;
- protocolLogger.debug(`Responding to client with driver.${spec.command}() ` +
+ getLogByProtocol(currentProtocol).debug(`Responding to client with driver.${spec.command}() ` +
`result: ${_.truncate(JSON.stringify(driverRes), {length: LOG_OBJ_LENGTH})}`);
} catch (err) {
// if anything goes wrong, figure out what our response should be
// based on the type of error that we encountered
let actualErr = err;
- currentProtocol = extractProtocol(driver, req.params.sessionId || newSessionId) || currentProtocol;
-
- // Use the logger that's specific to this response
- const protocolLogger = getLogByProtocol(currentProtocol);
+ currentProtocol = currentProtocol || extractProtocol(driver, req.params.sessionId || newSessionId);
let errMsg = err.stacktrace || err.stack;
if (!errMsg.includes(err.message)) {
@@ -404,7 +395,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
// is the first part of the stack trace
errMsg = `${err.message} ${errMsg}`;
}
- protocolLogger.debug(`Encountered internal error running command: ${errMsg}`);
+ getLogByProtocol(currentProtocol).debug(`Encountered internal error running command: ${errMsg}`);
if (isErrorType(err, errors.ProxyRequestError)) {
actualErr = err.getActualError();
}
@@ -515,31 +506,7 @@ async function doJwpProxy (driver, req, res) {
}
}
-/**
- * Check a driver command respond and see if the protocol and value was passed in
- * @param {Object} driverRes Response returned by `executeCommand` in an inner driver
- * @returns {?Object} Object of the form {isW3C, isMJSONWP, value|error} or null if it isn't parsable
- */
-function parseProtocol (driverRes) {
- if (!_.isPlainObject(driverRes) || !_.isString(driverRes.protocol)) {
- return null;
- }
-
- let isW3C = driverRes.protocol === BaseDriver.DRIVER_PROTOCOL.W3C;
- let isMJSONWP = driverRes.protocol === BaseDriver.DRIVER_PROTOCOL.MJSONWP;
-
- if (_.has(driverRes, 'value')) {
- return {isW3C, isMJSONWP, value: driverRes.value};
- }
-
- if (_.has(driverRes, 'error')) {
- return {isW3C, isMJSONWP, error: driverRes.error};
- }
-
- return null;
-}
-
export { Protocol, routeConfiguringFunction, isSessionCommand,
- MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, IMAGE_ELEMENT_PREFIX, parseProtocol,
+ MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, IMAGE_ELEMENT_PREFIX,
driverShouldDoJwpProxy };

MODIFY
test/protocol/protocol-specs.js
test/protocol/protocol-specs.js
@@ -4,7 +4,7 @@ import '../..'; // NOTE: For some reason this file needs to be imported to preve
import chai from 'chai';
import sinon from 'sinon';
import chaiAsPromised from 'chai-as-promised';
-import { MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, parseProtocol, driverShouldDoJwpProxy, IMAGE_ELEMENT_PREFIX } from '../../lib/protocol/protocol';
+import { MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, driverShouldDoJwpProxy, IMAGE_ELEMENT_PREFIX } from '../../lib/protocol/protocol';
import BaseDriver from '../../lib/basedriver/driver';
chai.should();
@@ -12,32 +12,6 @@ chai.use(chaiAsPromised);
describe('Protocol', async function () {
- describe('#parseProtocol', function () {
- it('should parse {protocol: "MJSONWP"} as MJSONWP', function () {
- parseProtocol({protocol: 'MJSONWP', value: undefined}).should.eql({isW3C: false, isMJSONWP: true, value: undefined});
- });
- it('should parse {protocol: "W3C"} as W3C', function () {
- parseProtocol({protocol: 'W3C', value: undefined}).should.eql({isW3C: true, isMJSONWP: false, value: undefined});
- });
- it('should parse {protocol: "MJSONWP", value: false} as MJSONWP with value: false', function () {
- parseProtocol({protocol: 'MJSONWP', value: false}).should.eql({isW3C: false, isMJSONWP: true, value: false});
- });
- it('should parse {protocol: "W3C", value: 0} as W3C with value: 0', function () {
- parseProtocol({protocol: 'W3C', value: 0}).should.eql({isW3C: true, isMJSONWP: false, value: 0});
- });
- it('should parse {protocol: "MJSONWP", value: "string"}', function () {
- parseProtocol({protocol: 'MJSONWP', value: "string"}).should.eql({isW3C: false, isMJSONWP: true, value: "string"});
- });
- it('should parse {protocol: "W3C", value: {obj}}', function () {
- const value = {hello: 'world', goodbye: 'whirl'};
- parseProtocol({protocol: 'MJSONWP', value}).should.eql({isW3C: false, isMJSONWP: true, value});
- });
- it('should throw if {protocol: "MJSONWP", error}', function () {
- const error = new Error('some error');
- parseProtocol({protocol: 'W3C', error}).error.should.equal(error);
- });
- });
-
describe('#driverShouldDoJwpProxy', function () {
it('should not proxy if an image element is found in request url', function () {
const d = new BaseDriver();

Fix the way current protocol is determined from execute response (#258)