Update response handling for proxyReqRes call
MODIFY
lib/jsonwp-proxy/proxy.js
lib/jsonwp-proxy/proxy.js
@@ -2,11 +2,14 @@ import _ from 'lodash';
import { logger, util } from 'appium-support';
import request from 'request-promise';
import { getSummaryByCode } from '../jsonwp-status/status';
-import { errors, isErrorType, errorFromMJSONWPStatusCode, errorFromW3CJsonCode } from '../protocol/errors';
+import {
+ errors, isErrorType, errorFromMJSONWPStatusCode, errorFromW3CJsonCode
+} from '../protocol/errors';
import BaseDriver from '../basedriver/driver';
import { routeToCommandName } from '../protocol/routes';
import ProtocolConverter from './protocol-converter';
-
+import { formatResponseValue, formatStatus } from '../protocol/helpers';
+import SESSIONS_CACHE from '../protocol/sessions-cache';
const log = logger.getLogger('WD Proxy');
// TODO: Make this value configurable as a server side capability
@@ -219,10 +222,10 @@ class JWProxy {
}
getProtocolFromResBody (resObj) {
- if (util.hasValue(resObj.status)) {
+ if (_.isInteger(resObj.status)) {
return MJSONWP;
}
- if (util.hasValue(resObj.value)) {
+ if (!_.isUndefined(resObj.value)) {
return W3C;
}
}
@@ -306,21 +309,19 @@ class JWProxy {
// if the proxied response contains a sessionId that the downstream
// driver has generated, we don't want to return that to the client.
// Instead, return the id from the request or from current session
- let isResBodyModified = false;
if (_.has(resBodyObj, 'sessionId')) {
const reqSessionId = this.getSessionIdFromUrl(req.originalUrl);
if (reqSessionId) {
log.info(`Replacing sessionId ${resBodyObj.sessionId} with ${reqSessionId}`);
resBodyObj.sessionId = reqSessionId;
- isResBodyModified = true;
} else if (this.sessionId) {
log.info(`Replacing sessionId ${resBodyObj.sessionId} with ${this.sessionId}`);
resBodyObj.sessionId = this.sessionId;
- isResBodyModified = true;
}
}
- res.status(response.statusCode)
- .send(isResBodyModified ? JSON.stringify(resBodyObj) : response.body);
+ resBodyObj.value = formatResponseValue(resBodyObj.value);
+ formatStatus(resBodyObj, SESSIONS_CACHE.getProtocol(resBodyObj.sessionId));
+ res.status(response.statusCode).send(JSON.stringify(resBodyObj));
}
}

ADD
lib/protocol/helpers.js
+import _ from 'lodash';
+import { duplicateKeys } from '../basedriver/helpers';
+import BaseDriver from '../basedriver/driver';
+
+
+const MJSONWP_ELEMENT_KEY = 'ELEMENT';
+const W3C_ELEMENT_KEY = 'element-6066-11e4-a52e-4f735466cecf';
+const JSONWP_SUCCESS_STATUS_CODE = 0;
+
+/**
+ * Preprocesses the resulting value for API responses,
+ * so they have keys for both W3C and JSONWP protocols.
+ * The argument value is NOT mutated
+ *
+ * @param {?Object} resValue The actual response value
+ * @returns {?Object} Either modified value or the same one if
+ * nothing has been modified
+ */
+function formatResponseValue (resValue) {
+ if (_.isUndefined(resValue)) {
+ // convert undefined to null
+ return null;
+ }
+ // If the MJSONWP element key format (ELEMENT) was provided, add a duplicate key (element-6066-11e4-a52e-4f735466cecf)
+ // If the W3C element key format (element-6066-11e4-a52e-4f735466cecf) was provided, add a duplicate key (ELEMENT)
+ return duplicateKeys(resValue, MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY);
+}
+
+/**
+ * Properly formats the status for API responses,
+ * so they are correct for both W3C and JSONWP protocols.
+ * This method DOES mutate the `responseBody` argument if needed
+ *
+ * @param {Object} responseBody
+ * @param {?string} protocol The name of the protocol, either
+ * `BaseDriver.DRIVER_PROTOCOL.W3C` or `BaseDriver.DRIVER_PROTOCOL.MJSONWP`
+ * @returns {Object} The fixed response body
+ */
+function formatStatus (responseBody, protocol = null) {
+ if (!_.isPlainObject(responseBody) || !protocol) {
+ return responseBody;
+ }
+ if (protocol === BaseDriver.DRIVER_PROTOCOL.MJSONWP && !_.isInteger(responseBody.status)) {
+ // Response status should be the status set by the driver response.
+ responseBody.status = JSONWP_SUCCESS_STATUS_CODE;
+ }
+ if (protocol === BaseDriver.DRIVER_PROTOCOL.W3C && !_.isUndefined(responseBody.status)) {
+ delete responseBody.status;
+ }
+ return responseBody;
+}
+
+
+export {
+ MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, formatResponseValue,
+ JSONWP_SUCCESS_STATUS_CODE, formatStatus,
+};

MODIFY
lib/protocol/protocol.js
lib/protocol/protocol.js
import _ from 'lodash';
-import { logger, util } from 'appium-support';
+import { util } from 'appium-support';
import { validators } from './validators';
-import { errors, isErrorType, errorFromMJSONWPStatusCode, errorFromW3CJsonCode,
- getResponseForW3CError, getResponseForJsonwpError } from './errors';
+import {
+ errors, isErrorType, getResponseForW3CError, getResponseForJsonwpError,
+ errorFromMJSONWPStatusCode, errorFromW3CJsonCode,
+} from './errors';
import { METHOD_MAP, NO_SESSION_ID_COMMANDS } from './routes';
-import { duplicateKeys } from '../basedriver/helpers';
import B from 'bluebird';
import BaseDriver from '../basedriver/driver';
-import LRU from 'lru-cache';
+import {
+ MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, formatResponseValue, formatStatus,
+} from './helpers';
+import SESSIONS_CACHE from './sessions-cache';
-const GENERIC_PROTOCOL = 'GENERIC';
-const mjsonwpLog = logger.getLogger('MJSONWP');
-const w3cLog = logger.getLogger('W3C');
-const genericProtocolLog = logger.getLogger(GENERIC_PROTOCOL);
-
-const JSONWP_SUCCESS_STATUS_CODE = 0;
// TODO: Make this value configurable as a server side capability
const LOG_OBJ_LENGTH = 1024; // MAX LENGTH Logged to file / console
-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';
@@ -36,69 +32,6 @@ const IMG_EL_URL_RE = new RegExp(
class Protocol {}
-class SessionsCache {
- constructor (max) {
- this._cache = new LRU({ max });
- }
-
- getLogger (sessionId, protocol) {
- if (sessionId) {
- if (this._cache.has(sessionId)) {
- const value = this._cache.get(sessionId);
- if (value.logger) {
- return value.logger;
- }
- protocol = protocol || value.protocol;
- }
- // Always create a new logger instance for ids
- // that are not in the current sessions list,
- // so we can still see such ids as prefixes
- return logger.getLogger(`${protocol || GENERIC_PROTOCOL} ` +
- `(${sessionId.substring(0, Math.min(sessionId.length, 8))})`);
- }
-
- // Fall back to protocol name-only logger if session id is unknown
- switch (protocol) {
- case BaseDriver.DRIVER_PROTOCOL.W3C:
- return w3cLog;
- case BaseDriver.DRIVER_PROTOCOL.MJSONWP:
- return mjsonwpLog;
- default:
- return genericProtocolLog;
- }
- }
-
- getProtocol (sessionId) {
- return (this._cache.get(sessionId) || {}).protocol;
- }
-
- putSession (sessionId, value) {
- if (sessionId && value) {
- this._cache.set(sessionId, {
- protocol: value,
- // We don't want to cache the logger instance for each random session id in the cache
- // in order to save memory. Instead we only cache loggers for valid ids that
- // are returned by `createSession` call and reset them after `deleteSession` is called
- logger: this.getLogger(sessionId, value),
- });
- }
- return value;
- }
-
- resetLogger (sessionId) {
- if (this._cache.has(sessionId)) {
- this._cache.get(sessionId).logger = null;
- }
- }
-}
-
-// 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 SESSIONS_CACHE = new SessionsCache(100);
-
-
function extractProtocol (driver, sessionId = null) {
const dstDriver = _.isFunction(driver.driverForSession)
? driver.driverForSession(sessionId)
@@ -382,16 +315,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
}
}
- // If the MJSONWP element key format (ELEMENT) was provided, add a duplicate key (element-6066-11e4-a52e-4f735466cecf)
- // If the W3C element key format (element-6066-11e4-a52e-4f735466cecf) was provided, add a duplicate key (ELEMENT)
- if (driverRes) {
- driverRes = duplicateKeys(driverRes, MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY);
- }
-
- // convert undefined to null, but leave all other values the same
- if (_.isUndefined(driverRes)) {
- driverRes = null;
- }
+ driverRes = formatResponseValue(driverRes);
// delete should not return anything even if successful
if (spec.command === DELETE_SESSION_COMMAND) {
@@ -410,10 +334,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
}
}
- // Response status should be the status set by the driver response.
- if (currentProtocol !== BaseDriver.DRIVER_PROTOCOL.W3C) {
- httpResBody.status = (_.isNil(driverRes) || _.isUndefined(driverRes.status)) ? JSONWP_SUCCESS_STATUS_CODE : driverRes.status;
- }
+ httpResBody = formatStatus(httpResBody, currentProtocol);
httpResBody.value = driverRes;
SESSIONS_CACHE.getLogger(req.params.sessionId || newSessionId, currentProtocol).debug(`Responding ` +
`to client with driver.${spec.command}() result: ${_.truncate(JSON.stringify(driverRes), {length: LOG_OBJ_LENGTH})}`);

ADD
lib/protocol/sessions-cache.js
+import LRU from 'lru-cache';
+import { logger } from 'appium-support';
+import BaseDriver from '../basedriver/driver';
+
+
+const GENERIC_PROTOCOL = 'GENERIC';
+const mjsonwpLog = logger.getLogger('MJSONWP');
+const w3cLog = logger.getLogger('W3C');
+const genericProtocolLog = logger.getLogger(GENERIC_PROTOCOL);
+
+
+class SessionsCache {
+ constructor (max) {
+ this._cache = new LRU({ max });
+ }
+
+ getLogger (sessionId, protocol) {
+ if (sessionId) {
+ if (this._cache.has(sessionId)) {
+ const value = this._cache.get(sessionId);
+ if (value.logger) {
+ return value.logger;
+ }
+ protocol = protocol || value.protocol;
+ }
+ // Always create a new logger instance for ids
+ // that are not in the current sessions list,
+ // so we can still see such ids as prefixes
+ return logger.getLogger(`${protocol || GENERIC_PROTOCOL} ` +
+ `(${sessionId.substring(0, Math.min(sessionId.length, 8))})`);
+ }
+
+ // Fall back to protocol name-only logger if session id is unknown
+ switch (protocol) {
+ case BaseDriver.DRIVER_PROTOCOL.W3C:
+ return w3cLog;
+ case BaseDriver.DRIVER_PROTOCOL.MJSONWP:
+ return mjsonwpLog;
+ default:
+ return genericProtocolLog;
+ }
+ }
+
+ getProtocol (sessionId) {
+ return (this._cache.get(sessionId) || {}).protocol;
+ }
+
+ putSession (sessionId, value) {
+ if (sessionId && value) {
+ this._cache.set(sessionId, {
+ protocol: value,
+ // We don't want to cache the logger instance for each random session id in the cache
+ // in order to save memory. Instead we only cache loggers for valid ids that
+ // are returned by `createSession` call and reset them after `deleteSession` is called
+ logger: this.getLogger(sessionId, value),
+ });
+ }
+ return value;
+ }
+
+ resetLogger (sessionId) {
+ if (this._cache.has(sessionId)) {
+ this._cache.get(sessionId).logger = null;
+ }
+ }
+}
+
+// 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 SESSIONS_CACHE = new SessionsCache(100);
+
+export default SESSIONS_CACHE;

Update response handling for proxyReqRes call (#344)