refactor(base-driver): Always assume the returned responses are in W3C format
MODIFY
package.json
package.json
"mjpeg-server": "0.3.1",
"mocha": "9.1.3",
"pre-commit": "1.2.2",
- "querystring": "0.2.1",
"rewiremock": "3.14.3",
"serve-static": "1.14.1",
"sinon": "12.0.1",

MODIFY
packages/base-driver/lib/jsonwp-proxy/protocol-converter.js
packages/base-driver/lib/jsonwp-proxy/protocol-converter.js
@@ -4,7 +4,6 @@ import { duplicateKeys } from '../basedriver/helpers';
import {
MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, PROTOCOLS
} from '../constants';
-import { formatStatus } from '../protocol/helpers';
const log = logger.getLogger('Protocol Converter');
@@ -206,10 +205,7 @@ class ProtocolConverter {
*/
async convertAndProxy (commandName, url, method, body) {
if (!this.downstreamProtocol) {
- // Patch calls with GENERIC protocol
- // to preserve the backward compatibility
- const [res, resBodyObj] = await this.proxyFunc(url, method, body);
- return [res, formatStatus(resBodyObj, res.statusCode)];
+ return await this.proxyFunc(url, method, body);
}
// Same url, but different arguments

MODIFY
packages/base-driver/lib/jsonwp-proxy/proxy.js
packages/base-driver/lib/jsonwp-proxy/proxy.js
@@ -9,7 +9,6 @@ import { routeToCommandName } from '../protocol';
import { MAX_LOG_BODY_LENGTH, DEFAULT_BASE_PATH, PROTOCOLS } from '../constants';
import ProtocolConverter from './protocol-converter';
import { formatResponseValue, formatStatus } from '../protocol/helpers';
-import SESSIONS_CACHE from '../protocol/sessions-cache';
import http from 'http';
import https from 'https';
@@ -327,8 +326,7 @@ class JWProxy {
}
}
resBodyObj.value = formatResponseValue(resBodyObj.value);
- formatStatus(resBodyObj, res.statusCode, SESSIONS_CACHE.getProtocol(reqSessionId));
- res.status(response.statusCode).send(JSON.stringify(resBodyObj));
+ res.status(response.statusCode).send(JSON.stringify(formatStatus(resBodyObj)));
}
}

MODIFY
packages/base-driver/lib/protocol/errors.js
packages/base-driver/lib/protocol/errors.js
@@ -657,7 +657,7 @@ class ProxyRequestError extends ES6Error {
getActualError () {
// If it's MJSONWP error, returns actual error cause for request failure based on `jsonwp.status`
- if (util.hasValue(this.jsonwp) && util.hasValue(this.jsonwp.status) && util.hasValue(this.jsonwp.value)) {
+ if (util.hasValue(this.jsonwp?.status) && util.hasValue(this.jsonwp?.value)) {
return errorFromMJSONWPStatusCode(this.jsonwp.status, this.jsonwp.value);
} else if (util.hasValue(this.w3c) && _.isNumber(this.w3cStatus) && this.w3cStatus >= 300) {
return errorFromW3CJsonCode(this.w3c.error, this.w3c.message || this.message, this.w3c.stacktrace);

MODIFY
packages/base-driver/lib/protocol/helpers.js
packages/base-driver/lib/protocol/helpers.js
import _ from 'lodash';
import { duplicateKeys } from '../basedriver/helpers';
-import { MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, PROTOCOLS } from '../constants';
-
-const JSONWP_SUCCESS_STATUS_CODE = 0;
-const JSONWP_UNKNOWN_ERROR_STATUS_CODE = 13;
+import { MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY } from '../constants';
/**
* Preprocesses the resulting value for API responses,
@@ -26,33 +23,16 @@ function formatResponseValue (resValue) {
/**
* 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
+ * so they are correct for the W3C protocol.
*
* @param {Object} responseBody
- * @param {number} responseCode the HTTP response code
- * @param {?string} protocol The name of the protocol, either
- * `PROTOCOLS.W3C` or `PROTOCOLS.MJSONWP`
* @returns {Object} The fixed response body
*/
-function formatStatus (responseBody, responseCode = 200, protocol = null) {
- if (!_.isPlainObject(responseBody)) {
- return responseBody;
- }
- const isError = _.has(responseBody.value, 'error') || responseCode >= 400;
- if ((protocol === PROTOCOLS.MJSONWP && !_.isInteger(responseBody.status))
- || (!protocol && !_.has(responseBody, 'status'))) {
- responseBody.status = isError
- ? JSONWP_UNKNOWN_ERROR_STATUS_CODE
- : JSONWP_SUCCESS_STATUS_CODE;
- } else if (protocol === PROTOCOLS.W3C && _.has(responseBody, 'status')) {
- delete responseBody.status;
- }
- return responseBody;
+function formatStatus (responseBody) {
+ return _.isPlainObject(responseBody) ? _.omit(responseBody, ['status']) : responseBody;
}
export {
- MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, formatResponseValue,
- JSONWP_SUCCESS_STATUS_CODE, formatStatus,
+ MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY, formatResponseValue, formatStatus
};

MODIFY
packages/base-driver/lib/protocol/protocol.js
packages/base-driver/lib/protocol/protocol.js
@@ -2,7 +2,7 @@ import _ from 'lodash';
import { util } from '@appium/support';
import { validators } from './validators';
import {
- errors, isErrorType, getResponseForW3CError, getResponseForJsonwpError,
+ errors, isErrorType, getResponseForW3CError,
errorFromMJSONWPStatusCode, errorFromW3CJsonCode,
} from './errors';
import { METHOD_MAP, NO_SESSION_ID_COMMANDS } from './routes';
@@ -377,24 +377,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
.debug(`Encountered internal error running command: ${errMsg}`);
}
- if (currentProtocol === PROTOCOLS.W3C) {
[httpStatus, httpResBody] = getResponseForW3CError(actualErr);
- } else if (currentProtocol === PROTOCOLS.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
- // together to be protocol-agnostic
- let jsonwpRes = getResponseForJsonwpError(actualErr);
- let w3cRes = getResponseForW3CError(actualErr);
-
- httpResBody = {
- ...jsonwpRes[1],
- ...w3cRes[1],
- };
-
- // Use the JSONWP status code (which is usually 500)
- httpStatus = jsonwpRes[0];
- }
}
// decode the response, which is either a string or json
@@ -415,7 +398,7 @@ function buildHandler (app, method, path, spec, driver, isSessCmd) {
delete httpResBody.sessionId;
}
- httpResBody = formatStatus(httpResBody, httpStatus, currentProtocol);
+ httpResBody = formatStatus(httpResBody);
res.status(httpStatus).json(httpResBody);
}
};
@@ -452,11 +435,10 @@ async function doJwpProxy (driver, req, res) {
// check that the inner driver has a proxy function
if (!driver.canProxy(req.params.sessionId)) {
- throw new Error('Trying to proxy to a JSONWP server but driver is unable to proxy');
+ throw new Error('Trying to proxy to a server but the driver is unable to proxy');
}
try {
- const proxiedRes = await driver.executeCommand('proxyReqRes', req, res, req.params.sessionId);
- if (proxiedRes && proxiedRes.error) throw proxiedRes.error; // eslint-disable-line curly
+ await driver.executeCommand('proxyReqRes', req, res, req.params.sessionId);
} catch (err) {
if (isErrorType(err, errors.ProxyRequestError)) {
throw err;

MODIFY
packages/base-driver/test/jsonwp-proxy/proxy-e2e-specs.js
packages/base-driver/test/jsonwp-proxy/proxy-e2e-specs.js
@@ -17,7 +17,6 @@ describe('proxy', function () {
it('should proxy status straight', async function () {
let [res, resBody] = await jwproxy.proxy('/status', 'GET');
res.statusCode.should.equal(200);
- resBody.status.should.equal(0);
resBody.value.should.equal(`I'm fine`);
});
it('should proxy status as command', async function () {

MODIFY
packages/base-driver/test/jsonwp-proxy/proxy-specs.js
packages/base-driver/test/jsonwp-proxy/proxy-specs.js
@@ -169,32 +169,32 @@ describe('proxy', function () {
await j.proxyReqRes(req, res);
res.headers['content-type'].should.equal('application/json; charset=utf-8');
res.sentCode.should.equal(200);
- res.sentBody.should.eql({status: 0, value: {foo: 'bar'}});
+ res.sentBody.should.eql({value: {foo: 'bar'}});
});
it('should rewrite the inner session id so it doesnt change', async function () {
let j = mockProxy({sessionId: '123'});
let [req, res] = buildReqRes('/element/200/value', 'GET');
await j.proxyReqRes(req, res);
- res.sentBody.should.eql({status: 0, value: 'foobar', sessionId: '123'});
+ res.sentBody.should.eql({value: 'foobar', sessionId: '123'});
});
it('should rewrite the inner session id with sessionId in url', async function () {
let j = mockProxy({sessionId: '123'});
let [req, res] = buildReqRes('/session/456/element/200/value', 'POST');
await j.proxyReqRes(req, res);
- res.sentBody.should.eql({status: 0, value: 'foobar', sessionId: '456'});
+ res.sentBody.should.eql({value: 'foobar', sessionId: '456'});
});
it('should pass through urls that do not require session IDs', async function () {
let j = mockProxy({sessionId: '123'});
let [req, res] = buildReqRes('/status', 'GET');
await j.proxyReqRes(req, res);
- res.sentBody.should.eql({status: 0, value: {'foo': 'bar'}});
+ res.sentBody.should.eql({value: {'foo': 'bar'}});
});
it('should proxy strange responses', async function () {
let j = mockProxy({sessionId: '123'});
let [req, res] = buildReqRes('/nochrome', 'GET');
await j.proxyReqRes(req, res);
res.sentCode.should.equal(100);
- res.sentBody.should.eql({status: 0, value: {message: 'chrome not reachable'}});
+ res.sentBody.should.eql({value: {message: 'chrome not reachable'}});
});
});
});

MODIFY
packages/base-driver/test/protocol/protocol-e2e-specs.js
packages/base-driver/test/protocol/protocol-e2e-specs.js
@@ -11,7 +11,6 @@ import { createProxyServer } from './helpers';
import {
MJSONWP_ELEMENT_KEY, W3C_ELEMENT_KEY
} from '../../lib/constants';
-import qs from 'querystring';
import {TEST_HOST, getTestPort} from '../helpers';
let port;
@@ -58,7 +57,6 @@ describe('Protocol', function () {
data: {url: 'http://google.com'}
});
data.should.eql({
- status: 0,
value: 'Navigated to: http://google.com',
sessionId: 'foo'
});
@@ -71,25 +69,23 @@ describe('Protocol', function () {
data: {url: 'http://google.com'},
});
data.should.eql({
- status: 0,
value: 'Navigated to: http://google.com',
sessionId: 'foo'
});
});
it('should respond to x-www-form-urlencoded as well as json requests', async function () {
+ const reqData = new URLSearchParams();
+ reqData.set('url', 'http://google.com');
const {data} = await axios({
url: `${baseUrl}/session/foo/url`,
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
},
method: 'POST',
- data: qs.stringify({
- url: 'http://google.com',
- }),
+ data: reqData.toString(),
});
data.should.eql({
- status: 0,
value: 'Navigated to: http://google.com',
sessionId: 'foo'
});
@@ -102,7 +98,6 @@ describe('Protocol', function () {
data: {},
});
data.should.eql({
- status: 0,
value: 'foo',
sessionId: 'foo'
});
@@ -114,7 +109,6 @@ describe('Protocol', function () {
method: 'POST',
data: {},
});
- data.status.should.equal(0);
data.value.should.eql(['bar', 'foo']);
});
@@ -122,7 +116,6 @@ describe('Protocol', function () {
const {data} = await axios({
url: `${baseUrl}/session/foo/element/bar/attribute/baz`,
});
- data.status.should.equal(0);
data.value.should.eql(['baz', 'bar', 'foo']);
});
@@ -150,7 +143,6 @@ describe('Protocol', function () {
data: {url: 'http://google.com'}
});
data.should.eql({
- status: 0,
value: 'Navigated to: http://google.com',
sessionId: 'foo'
});
@@ -178,14 +170,10 @@ describe('Protocol', function () {
validateStatus: null,
});
- status.should.equal(501);
- data.should.eql({
- status: 405,
- value: {
- message: 'Method has not yet been implemented'
- },
- sessionId: 'foo'
- });
+ status.should.equal(405);
+ data.value.error.should.eql('unknown method');
+ data.value.message.should.eql('Method has not yet been implemented');
+ data.sessionId.should.eql('foo');
});
it('should throw not implemented for ignored commands', async function () {
@@ -196,14 +184,10 @@ describe('Protocol', function () {
data: {},
});
- status.should.equal(501);
- data.should.eql({
- status: 405,
- value: {
- message: 'Method has not yet been implemented'
- },
- sessionId: 'foo'
- });
+ status.should.equal(405);
+ data.value.error.should.eql('unknown method');
+ data.value.message.should.eql('Method has not yet been implemented');
+ data.sessionId.should.eql('foo');
});
it('should get 400 for bad parameters', async function () {
@@ -244,14 +228,10 @@ describe('Protocol', function () {
validateStatus: null,
});
status.should.equal(500);
- data.should.eql({
- status: 13,
- value: {
- message: 'An unknown server-side error occurred while processing ' +
- 'the command. Original error: Mishandled Driver Error'
- },
- sessionId: 'foo'
- });
+ data.value.error.should.eql('unknown error');
+ data.value.message.should.eql('An unknown server-side error occurred while processing ' +
+ 'the command. Original error: Mishandled Driver Error');
+ data.sessionId.should.eql('foo');
});
describe('w3c sendkeys migration', function () {
@@ -261,7 +241,6 @@ describe('Protocol', function () {
method: 'POST',
data: {value: 'text to type'}
});
- data.status.should.equal(0);
data.value.should.eql(['text to type', 'bar']);
});
it('should accept text for sendkeys', async function () {
@@ -270,7 +249,6 @@ describe('Protocol', function () {
method: 'POST',
data: {text: 'text to type'}
});
- data.status.should.equal(0);
data.value.should.eql(['text to type', 'bar']);
});
it('should accept value and text for sendkeys, and use value', async function () {
@@ -279,7 +257,6 @@ describe('Protocol', function () {
method: 'POST',
data: {value: 'text to type', text: 'text to ignore'}
});
- data.status.should.equal(0);
data.value.should.eql(['text to type', 'bar']);
});
});
@@ -292,7 +269,6 @@ describe('Protocol', function () {
method: 'POST',
data: {element: '3'}
});
- data.status.should.equal(0);
data.value.should.eql(['3', null, null]);
});
it('should allow moveto with xoffset/yoffset', async function () {
@@ -301,7 +277,6 @@ describe('Protocol', function () {
method: 'POST',
data: {xoffset: 42, yoffset: 17}
});
- data.status.should.equal(0);
data.value.should.eql([null, 42, 17]);
});
});
@@ -312,7 +287,6 @@ describe('Protocol', function () {
method: 'POST',
data: {appId: 42}
});
- data.status.should.equal(0);
data.value.should.eql(42);
});
it('should allow removeApp with bundleId', async function () {
@@ -321,7 +295,6 @@ describe('Protocol', function () {
method: 'POST',
data: {bundleId: 42}
});
- data.status.should.equal(0);
data.value.should.eql(42);
});
});
@@ -728,7 +701,6 @@ describe('Protocol', function () {
method: 'POST',
});
data.should.eql({
- status: 0,
value: null,
sessionId: 'foo'
});
@@ -739,7 +711,6 @@ describe('Protocol', function () {
url: `${baseUrl}/session/foo/element/bar/text`,
});
data.should.eql({
- status: 0,
value: '',
sessionId: 'foo'
});
@@ -753,14 +724,10 @@ describe('Protocol', function () {
});
status.should.equal(500);
- data.should.eql({
- status: 13,
- value: {
- message: 'An unknown server-side error occurred while processing ' +
- 'the command. Original error: Too Fresh!'
- },
- sessionId: 'foo'
- });
+ data.value.error.should.eql('unknown error');
+ data.value.message.should.eql('An unknown server-side error occurred while processing ' +
+ 'the command. Original error: Too Fresh!');
+ data.sessionId.should.eql('foo');
});
it('should not throw UnknownError when known', async function () {
@@ -770,13 +737,9 @@ describe('Protocol', function () {
});
status.should.equal(404);
- data.should.eql({
- status: 6,
- value: {
- message: 'A session is either terminated or not started'
- },
- sessionId: 'foo'
- });
+ data.value.error.should.eql('invalid session id');
+ data.value.message.should.eql('A session is either terminated or not started');
+ data.sessionId.should.eql('foo');
});
});
@@ -832,7 +795,6 @@ describe('Protocol', function () {
});
status.should.equal(404);
- data.status.should.equal(6);
data.value.message.should.contain('session');
});
@@ -848,7 +810,6 @@ describe('Protocol', function () {
});
status.should.equal(404);
- data.status.should.equal(6);
data.value.message.should.contain('session');
});
@@ -863,14 +824,10 @@ describe('Protocol', function () {
});
status.should.equal(500);
- data.should.eql({
- status: 13,
- value: {
- message: 'An unknown server-side error occurred while processing ' +
- 'the command. Original error: Too Fresh!'
- },
- sessionId
- });
+ data.value.error.should.eql('unknown error');
+ data.value.message.should.eql('An unknown server-side error occurred while processing ' +
+ 'the command. Original error: Too Fresh!');
+ data.sessionId.should.eql('Vader Sessions');
});
it('should return a new session ID on create', async function () {
@@ -918,15 +875,11 @@ describe('Protocol', function () {
});
status.should.equal(500);
- data.should.eql({
- status: 13,
- value: {
- message: 'An unknown server-side error occurred while processing ' +
- 'the command. Original error: Trying to proxy to a JSONWP ' +
- 'server but driver is unable to proxy'
- },
- sessionId
- });
+ data.value.error.should.eql('unknown error');
+ data.value.message.should.eql('An unknown server-side error occurred while processing ' +
+ 'the command. Original error: Trying to proxy to a ' +
+ 'server but the driver is unable to proxy');
+ data.sessionId.should.eql('foo');
});
it('should pass on any errors in proxying', async function () {
@@ -941,15 +894,10 @@ describe('Protocol', function () {
});
status.should.equal(500);
- data.should.eql({
- status: 13,
- value: {
- message: 'An unknown server-side error occurred while processing ' +
- 'the command. Original error: Could not proxy. Proxy ' +
- 'error: foo'
- },
- sessionId
- });
+ data.value.error.should.eql('unknown error');
+ data.value.message.should.eql('An unknown server-side error occurred while processing ' +
+ 'the command. Original error: Could not proxy. Proxy error: foo');
+ data.sessionId.should.eql('foo');
});
it('should able to throw ProxyRequestError in proxying', async function () {
@@ -964,14 +912,10 @@ describe('Protocol', function () {
data: {url: 'http://google.com'},
});
- status.should.equal(500);
- data.should.eql({
- status: 35,
- value: {
- message: 'No such context found.'
- },
- sessionId: 'foo'
- });
+ status.should.equal(400);
+ data.value.error.should.eql('unknown error');
+ data.value.message.should.eql('No such context found.');
+ data.sessionId.should.eql('foo');
});
it('should let the proxy handle req/res', async function () {
@@ -998,7 +942,6 @@ describe('Protocol', function () {
status.should.equal(200);
data.should.eql({
- status: 0,
value: 'Navigated to: http://google.com',
sessionId
});
@@ -1015,7 +958,6 @@ describe('Protocol', function () {
});
status.should.equal(500);
- data.status.should.equal(13);
data.value.message.should.contain('roxy');
}
const lists = [
@@ -1038,7 +980,6 @@ describe('Protocol', function () {
status.should.equal(200);
data.should.eql({
- status: 0,
value: "I'm fine",
sessionId: null
});
@@ -1063,7 +1004,6 @@ describe('Protocol', function () {
status.should.equal(200);
data.should.eql({
- status: 0,
value: 'This was not proxied',
sessionId
});

refactor(base-driver): Always assume the returned responses are in W3C format (#16175)