refactor: Use axios for the actual proxy implementation instead of the deprecated request module
BREAKING CHANGE: The request method of Proxy class now accepts and returns axios-specific values
BREAKING CHANGE: The default value of keepAlive setting has been changed to true
MODIFY
.gitignore
.gitignore
@@ -5,3 +5,4 @@ build
coverage
.idea/
package-lock.json*
+.vscode

MODIFY
lib/express/server.js
lib/express/server.js
@@ -20,6 +20,7 @@ import {
} from './websocket';
import B from 'bluebird';
import { DEFAULT_BASE_PATH } from '../protocol';
+import { EventEmitter } from 'events';
const KEEP_ALIVE_TIMEOUT_MS = 60 * 10 * 1000; // 10 minutes
@@ -37,6 +38,8 @@ async function server (opts = {}) {
// create the actual http server
const app = express();
let httpServer = http.createServer(app);
+ const serverStateNotifier = new EventEmitter();
+ let isServerClosed = false;
httpServer.addWebSocketHandler = addWebSocketHandler;
httpServer.removeWebSocketHandler = removeWebSocketHandler;
httpServer.removeAllWebSocketHandlers = removeAllWebSocketHandlers;
@@ -46,6 +49,9 @@ async function server (opts = {}) {
// all connections are closed and the `close` event is emitted
const close = httpServer.close.bind(httpServer);
httpServer.close = async () => await new B((resolve, reject) => {
+ // https://github.com/nodejs/node-v0.x-archive/issues/9066#issuecomment-124210576
+ isServerClosed = true;
+ serverStateNotifier.emit('shutdown');
httpServer.on('close', resolve);
close((err) => {
if (err) reject(err); // eslint-disable-line curly
@@ -67,6 +73,25 @@ async function server (opts = {}) {
httpServer.on('connection', (socket) => {
socket.setTimeout(KEEP_ALIVE_TIMEOUT_MS);
socket.on('error', reject);
+
+ function destroy () {
+ if (socket._openReqCount === 0) {
+ socket.destroy();
+ }
+ }
+ socket._openReqCount = 0;
+ socket.once('close', () => serverStateNotifier.removeListener('shutdown', destroy));
+ serverStateNotifier.once('shutdown', destroy);
+ });
+ httpServer.on('request', function (req, res) {
+ const socket = req.connection || req.socket;
+ socket._openReqCount++;
+ res.on('finish', function () {
+ socket._openReqCount--;
+ if (isServerClosed && socket._openReqCount === 0) {
+ socket.destroy();
+ }
+ });
});
configureServer(app, routeConfiguringFunction, allowCors, basePath);

MODIFY
lib/jsonwp-proxy/proxy.js
lib/jsonwp-proxy/proxy.js
import _ from 'lodash';
import { logger, util } from 'appium-support';
-import request from 'request-promise';
+import axios from 'axios';
import { getSummaryByCode } from '../jsonwp-status/status';
import {
errors, isErrorType, errorFromMJSONWPStatusCode, errorFromW3CJsonCode
@@ -9,6 +9,9 @@ import { PROTOCOLS, routeToCommandName, DEFAULT_BASE_PATH } from '../protocol';
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';
+
const log = logger.getLogger('WD Proxy');
// TODO: Make this value configurable as a server side capability
@@ -19,7 +22,7 @@ const {MJSONWP, W3C} = PROTOCOLS;
class JWProxy {
constructor (opts = {}) {
- Object.assign(this, {
+ _.defaults(this, opts, {
scheme: 'http',
server: 'localhost',
port: 4444,
@@ -27,20 +30,32 @@ class JWProxy {
reqBasePath: DEFAULT_BASE_PATH,
sessionId: null,
timeout: DEFAULT_REQUEST_TIMEOUT,
- keepAlive: false,
- }, opts);
+ keepAlive: true,
+ });
this.scheme = this.scheme.toLowerCase();
this._activeRequests = [];
+ this._reqCancelSource = axios.CancelToken.source();
this._downstreamProtocol = null;
this.protocolConverter = new ProtocolConverter(this.proxy.bind(this));
}
- // abstract the call behind a member function
- // so that we can mock it in tests
- async request (...args) {
- const currentRequest = request(...args);
- this._activeRequests.push(currentRequest);
- return await currentRequest.finally(() => _.pull(this._activeRequests, currentRequest));
+ /**
+ * Performs requests to the downstream server
+ *
+ * @private - Do not call this method directly,
+ * it uses client-specific arguments and responses!
+ *
+ * @param {AxiosRequestConfig} requestConfig
+ * @returns {AxiosResponse}
+ */
+ async request (requestConfig) {
+ const reqPromise = axios(requestConfig);
+ this._activeRequests.push(reqPromise);
+ try {
+ return await reqPromise;
+ } finally {
+ _.pull(this._activeRequests, reqPromise);
+ }
}
getActiveRequestsCount () {
@@ -49,9 +64,7 @@ class JWProxy {
cancelActiveRequests () {
try {
- for (let r of this._activeRequests) {
- r.cancel();
- }
+ this._reqCancelSource.cancel('The request has been canceled');
} finally {
this._activeRequests = [];
}
@@ -132,91 +145,79 @@ class JWProxy {
'user-agent': 'appium',
accept: 'application/json, */*',
},
- resolveWithFullResponse: true,
+ validateStatus: null,
timeout: this.timeout,
+ httpAgent: new http.Agent({ keepAlive: this.keepAlive }),
+ httpsAgent: new https.Agent({ keepAlive: this.keepAlive }),
+ cancelToken: this._reqCancelSource.token,
};
- if (this.keepAlive) {
- reqOpts.forever = true;
- // Setting `forever` option to `true` implicitly assigns
- // `agent` option to `http(s).Agent({keepAlive:true})`
- } else {
- // Setting `agent` option to `false` prevents the requests
- // module from using any connection pools, which enforces
- // new socket creation for each HTTP(S) request
- // (the default HTTP 1.0 behavior)
- reqOpts.agent = false;
- }
- if (util.hasValue(body)) {
+ // GET methods shouldn't have any body. Most servers are OK with this, but WebDriverAgent throws 400 errors
+ if (util.hasValue(body) && method !== 'GET') {
if (typeof body !== 'object') {
try {
- reqOpts.json = JSON.parse(body);
+ reqOpts.data = JSON.parse(body);
} catch (e) {
throw new Error(`Cannot interpret the request body as valid JSON: ${truncateBody(body)}`);
}
} else {
- reqOpts.json = body;
- }
+ reqOpts.data = body;
}
-
- // GET methods shouldn't have any body. Most servers are OK with this, but WebDriverAgent throws 400 errors
- if (method === 'GET') {
- reqOpts.json = null;
}
log.debug(`Proxying [${method} ${url || '/'}] to [${method} ${newUrl}] ` +
- (body ? `with body: ${truncateBody(body)}` : 'with no body'));
+ (reqOpts.data ? `with body: ${truncateBody(reqOpts.data)}` : 'with no body'));
const throwProxyError = (error) => {
- const message = `The request to ${url} has failed`;
- const err = new Error(message);
- err.message = message;
- err.error = error;
- err.statusCode = 500;
+ const err = new Error(`The request to ${url} has failed`);
+ err.response = {
+ data: error,
+ status: 500
+ };
throw err;
};
let isResponseLogged = false;
try {
- const res = await this.request(reqOpts);
- // `res.body` might be really big
+ const {data, status, headers} = await this.request(reqOpts);
+ // `data` might be really big
// Be careful while handling it to avoid memory leaks
- const resBodyObj = util.safeJsonParse(res.body);
- if (!_.isPlainObject(resBodyObj)) {
+ if (!_.isPlainObject(data)) {
// The response should be a valid JSON object
// If it cannot be coerced to an object then the response is wrong
- throwProxyError(res.body);
+ throwProxyError(data);
}
- log.debug(`Got response with status ${res.statusCode}: ${truncateBody(res.body)}`);
+ log.debug(`Got response with status ${status}: ${truncateBody(data)}`);
isResponseLogged = true;
const isSessionCreationRequest = /\/session$/.test(url) && method === 'POST';
if (isSessionCreationRequest) {
- if (res.statusCode === 200) {
- this.sessionId = resBodyObj.sessionId || (resBodyObj.value || {}).sessionId;
+ if (status === 200) {
+ this.sessionId = data.sessionId || (data.value || {}).sessionId;
}
- this.downstreamProtocol = this.getProtocolFromResBody(resBodyObj);
+ this.downstreamProtocol = this.getProtocolFromResBody(data);
log.info(`Determined the downstream protocol as '${this.downstreamProtocol}'`);
}
- if (res.statusCode < 400 && _.has(resBodyObj, 'status') && parseInt(resBodyObj.status, 10) !== 0) {
+ if (status < 400 && _.has(data, 'status') && parseInt(data.status, 10) !== 0) {
// Some servers, like chromedriver may return response code 200 for non-zero JSONWP statuses
- throwProxyError(resBodyObj);
+ throwProxyError(data);
}
- return [res, resBodyObj];
+ const res = {statusCode: status, headers, body: data};
+ return [res, data];
} catch (e) {
// We only consider an error unexpected if this was not
// an async request module error or if the response cannot be cast to
// a valid JSON
- if (util.hasValue(e.error)) {
+ if (util.hasValue(e.response)) {
if (!isResponseLogged) {
- const error = truncateBody(e.error);
- log.info(util.hasValue(e.statusCode)
- ? `Got response with status ${e.statusCode}: ${error}`
+ const error = truncateBody(e.response.data);
+ log.info(util.hasValue(e.response.status)
+ ? `Got response with status ${e.response.status}: ${error}`
: `Got response with unknown status: ${error}`);
}
} else {
log.warn(e.message);
log.debug(e.stack);
}
- throw new errors.ProxyRequestError(`Could not proxy command to remote server. ` +
- `Original error: ${e.message}`, e.error, e.statusCode);
+ throw new errors.ProxyRequestError(`Could not proxy command to the remote server. ` +
+ `Original error: ${e.message}`, e.response?.data, e.response?.status);
}
}

MODIFY
package.json
package.json
"lru-cache": "^5.0.0",
"method-override": "^3.0.0",
"morgan": "^1.9.0",
- "request": "^2.88.2",
- "request-promise": "^4.2.5",
"serve-favicon": "^2.4.5",
"source-map-support": "^0.5.5",
"validate.js": "^0.13.0",

MODIFY
test/jsonwp-proxy/mock-request.js
test/jsonwp-proxy/mock-request.js
@@ -21,17 +21,17 @@ function resFixture (url, method) {
}
async function request (opts) { // eslint-disable-line require-await
- if (/badurl$/.test(opts.url)) {
+ const {url, method, json} = opts;
+ if (/badurl$/.test(url)) {
throw new Error('noworky');
}
- let [statusCode, body] = resFixture(opts.url, opts.method, opts.json);
- let response = {
- statusCode,
+ const [status, data] = resFixture(url, method, json);
+ return {
+ status,
headers: {'content-type': 'application/json; charset=utf-8'},
- body
+ data,
};
- return response;
}
export default request;

MODIFY
test/protocol/protocol-e2e-specs.js
test/protocol/protocol-e2e-specs.js
@@ -396,7 +396,6 @@ describe('Protocol', function () {
}).should.eventually.be.rejectedWith(/400/);
});
it('should allow create session with capabilities (W3C)', async function () {
- // let {status, value, sessionId} = await request({
const {data} = await axios({
url: `${baseUrl}/session`,
method: 'POST',

refactor: Use axios for the actual proxy implementation instead of the deprecated request module (#409)

BREAKING CHANGE: The request method of Proxy class now accepts and returns axios-specific values
BREAKING CHANGE: The default value of keepAlive setting has been changed to true