chore: Replace requests usage with axios (appium#14269)

zgordon02 · May 5, 2020 · da83532 · da83532
1 parent c2ff85c
commit da83532
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 117 deletions.
diff --git a/lib/config.js b/lib/config.js
@@ -1,7 +1,7 @@
 import _ from 'lodash';
 import path from 'path';
-import { mkdirp, fs, system, util } from 'appium-support';
-import request from 'request-promise';
+import { mkdirp, fs, system } from 'appium-support';
+import axios from 'axios';
 import { exec } from 'teen_process';
 import { rootDir } from './utils';
 import logger from './logger';
@@ -51,12 +51,11 @@ async function getGitRev (useGithubApiFallback = false) {
   }
 
   try {
-    const response = await request.get(`${GITHUB_API}/tags`, {
+    const resBodyObj = (await axios.get(`${GITHUB_API}/tags`, {
       headers: {
         'User-Agent': `Appium ${APPIUM_VER}`
       }
-    });
-    const resBodyObj = util.safeJsonParse(response);
+    })).data;
     if (_.isArray(resBodyObj)) {
       for (const {name, commit} of resBodyObj) {
         if (name === `v${APPIUM_VER}` && commit && commit.sha) {
@@ -83,12 +82,11 @@ async function getGitTimestamp (commitSha, useGithubApiFallback = false) {
   }
 
   try {
-    const response = await request.get(`${GITHUB_API}/commits/${commitSha}`, {
+    const resBodyObj = (await axios.get(`${GITHUB_API}/commits/${commitSha}`, {
       headers: {
         'User-Agent': `Appium ${APPIUM_VER}`
       }
-    });
-    const resBodyObj = util.safeJsonParse(response);
+    })).data;
     if (resBodyObj && resBodyObj.commit) {
       if (resBodyObj.commit.committer && resBodyObj.commit.committer.date) {
         return resBodyObj.commit.committer.date;

diff --git a/lib/grid-register.js b/lib/grid-register.js
@@ -1,4 +1,4 @@
-import request from 'request-promise';
+import axios from 'axios';
 import { fs } from 'appium-support';
 import logger from './logger';
 import _ from 'lodash';
@@ -26,116 +26,104 @@ async function registerNode (configFile, addr, port) {
   postRequest(data, addr, port);
 }
 
-async function registerToGrid (options_post, jsonObject) {
+async function registerToGrid (postOptions, configHolder) {
   try {
-    let response = await request(options_post);
-    if (response === undefined || response.statusCode !== 200) {
-      throw new Error('Request failed');
+    const {status} = await axios(postOptions);
+    if (status !== 200) {
+      throw new Error(`Request failed with code ${status}`);
     }
-    let logMessage = `Appium successfully registered with the grid on ${hubUri(jsonObject.configuration)}`;
-    logger.debug(logMessage);
+    logger.debug(`Appium successfully registered with the the grid on ` +
+      hubUri(configHolder.configuration));
   } catch (err) {
-    logger.error(`Request to register with grid was unsuccessful: ${err.message}`);
+    logger.error(`An attempt to register with the grid was unsuccessful: ${err.message}`);
   }
 }
 
 function postRequest (data, addr, port) {
   // parse json to get hub host and port
-  let jsonObject;
+  let configHolder;
   try {
-    jsonObject = JSON.parse(data);
+    configHolder = JSON.parse(data);
   } catch (err) {
     logger.errorAndThrow(`Syntax error in node configuration file: ${err.message}`);
   }
 
   // Move Selenium 3 configuration properties to configuration object
-  if (!_.has(jsonObject, 'configuration')) {
+  if (!_.has(configHolder, 'configuration')) {
     let configuration = {};
-    for (const property in jsonObject) {
-      if (_.has(jsonObject, property) && property !== 'capabilities') {
-        configuration[property] = jsonObject[property];
-        delete jsonObject[property];
+    for (const property in configHolder) {
+      if (_.has(configHolder, property) && property !== 'capabilities') {
+        configuration[property] = configHolder[property];
+        delete configHolder[property];
       }
     }
-    jsonObject.configuration = configuration;
+    configHolder.configuration = configuration;
   }
 
   // if the node config does not have the appium/webdriver url, host, and port,
   // automatically add it based on how appium was initialized
   // otherwise, we will take whatever the user setup
   // because we will always set localhost/127.0.0.1. this won't work if your
   // node and grid aren't in the same place
-  if (!jsonObject.configuration.url || !jsonObject.configuration.host || !jsonObject.configuration.port) {
-    jsonObject.configuration.url = `http://${addr}:${port}/wd/hub`;
-    jsonObject.configuration.host = addr;
-    jsonObject.configuration.port = port;
+  if (!configHolder.configuration.url || !configHolder.configuration.host || !configHolder.configuration.port) {
+    configHolder.configuration.url = `http://${addr}:${port}/wd/hub`;
+    configHolder.configuration.host = addr;
+    configHolder.configuration.port = port;
   }
   // if the node config does not have id automatically add it
-  if (!jsonObject.configuration.id) {
-    jsonObject.configuration.id = `http://${jsonObject.configuration.host}:${jsonObject.configuration.port}`;
+  if (!configHolder.configuration.id) {
+    configHolder.configuration.id = `http://${configHolder.configuration.host}:${configHolder.configuration.port}`;
   }
 
-  // re-serialize the configuration with the auto populated data
-  data = JSON.stringify(jsonObject);
-
-  // prepare the header
-  let post_headers = {
-    'Content-Type': 'application/json',
-    'Content-Length': data.length
-  };
   // the post options
-  let post_options = {
-    url: `${hubUri(jsonObject.configuration)}/grid/register`,
+  const regRequest = {
+    url: `${hubUri(configHolder.configuration)}/grid/register`,
     method: 'POST',
-    body: data,
-    headers: post_headers,
-    resolveWithFullResponse: true // return the full response, not just the body
+    data: configHolder,
   };
 
-  if (jsonObject.configuration.register !== true) {
-    logger.debug(`No registration sent (${jsonObject.configuration.register} = false)`);
+  if (configHolder.configuration.register !== true) {
+    logger.debug(`No registration sent (${configHolder.configuration.register} = false)`);
     return;
   }
 
-  let registerCycleTime = jsonObject.configuration.registerCycle;
-  if (registerCycleTime !== null && registerCycleTime > 0) {
-    // initiate a new Thread
-    let first = true;
-    logger.debug(`Starting auto register thread for grid. Will try to register every ${registerCycleTime} ms.`);
-    setInterval(async function registerRetry () {
-      if (first !== true) {
-        let isRegistered = await isAlreadyRegistered(jsonObject);
-        if (isRegistered !== null && isRegistered !== true) {
-          // make the http POST to the grid for registration
-          await registerToGrid(post_options, jsonObject);
-        }
-      } else {
-        first = false;
-        await registerToGrid(post_options, jsonObject);
-      }
-    }, registerCycleTime);
+  const registerCycleInterval = configHolder.configuration.registerCycle;
+  if (isNaN(registerCycleInterval) || registerCycleInterval <= 0) {
+    logger.warn(`'registerCycle' is not a valid positive number. ` +
+      `No registration request will be sent to the grid.`);
+    return;
   }
+  // initiate a new Thread
+  let first = true;
+  logger.debug(`Starting auto register thread for the grid. ` +
+    `Will try to register every ${registerCycleInterval} ms.`);
+  setInterval(async function registerRetry () {
+    if (first) {
+      first = false;
+      await registerToGrid(regRequest, configHolder);
+    } else if (!await isAlreadyRegistered(configHolder)) {
+      // make the http POST to the grid for registration
+      await registerToGrid(regRequest, configHolder);
+    }
+  }, registerCycleInterval);
 }
 
-async function isAlreadyRegistered (jsonObject) {
+async function isAlreadyRegistered (configHolder) {
   //check if node is already registered
-  let id = jsonObject.configuration.id;
+  const id = configHolder.configuration.id;
   try {
-    let response = await request({
-      uri: `${hubUri(jsonObject.configuration)}/grid/api/proxy?id=${id}`,
-      method: 'GET',
+    const {data, status} = await axios({
+      url: `${hubUri(configHolder.configuration)}/grid/api/proxy?id=${id}`,
       timeout: 10000,
-      resolveWithFullResponse: true // return the full response, not just the body
     });
-    if (response === undefined || response.statusCode !== 200) {
-      throw new Error(`Request failed`);
+    if (status !== 200) {
+      throw new Error(`Request failed with code ${status}`);
     }
-    let responseData = JSON.parse(response.body);
-    if (responseData.success !== true) {
+    if (!data.success) {
       // if register fail, print the debug msg
-      logger.debug(`Grid registration error: ${responseData.msg}`);
+      logger.debug(`Grid registration error: ${data.msg}`);
     }
-    return responseData.success;
+    return data.success;
   } catch (err) {
     logger.debug(`Hub down or not responding: ${err.message}`);
   }

diff --git a/package.json b/package.json
@@ -58,15 +58,14 @@
     "argparse": "^1.0.10",
     "async-lock": "^1.0.0",
     "asyncbox": "2.x",
+    "axios": "^0.19.2",
     "bluebird": "3.x",
     "continuation-local-storage": "3.x",
     "dateformat": "^3.0.3",
     "find-root": "^1.1.0",
     "lodash": "^4.17.11",
     "longjohn": "^0.2.12",
     "npmlog": "4.x",
-    "request": "^2.81.0",
-    "request-promise": "4.x",
     "semver": "^7.0.0",
     "source-map-support": "0.x",
     "teen_process": "1.x",

diff --git a/test/config-specs.js b/test/config-specs.js
@@ -11,7 +11,7 @@ import { getGitRev, getBuildInfo, checkNodeOk, warnNodeDeprecations,
 import getParser from '../lib/parser';
 import logger from '../lib/logger';
 import { fs } from 'appium-support';
-import request from 'request-promise';
+import axios from 'axios';
 
 let should = chai.should();
 chai.use(chaiAsPromised);
@@ -45,7 +45,7 @@ describe('Config', function () {
       let getStub;
       beforeEach(function () {
         mockFs = sinon.mock(fs);
-        getStub = sinon.stub(request, 'get');
+        getStub = sinon.stub(axios, 'get');
       });
       afterEach(function () {
         getStub.restore();
@@ -56,7 +56,7 @@ describe('Config', function () {
         await verifyBuildInfoUpdate(true);
       });
       it('should get a configuration object if the local git metadata is not present', async function () {
-        getStub.onCall(0).returns([
+        getStub.onCall(0).returns({data: [
           {
             'name': `v${APPIUM_VER}`,
             'zipball_url': 'https://api.github.com/repos/appium/appium/zipball/v1.9.0-beta.1',
@@ -77,8 +77,8 @@ describe('Config', function () {
             },
             'node_id': 'MDM6UmVmNzUzMDU3MDp2MS44LjItYmV0YQ=='
           }
-        ]);
-        getStub.onCall(1).returns({
+        ]});
+        getStub.onCall(1).returns({data: {
           'sha': '3c2752f9f9c56000705a4ae15b3ba68a5d2e644c',
           'node_id': 'MDY6Q29tbWl0NzUzMDU3MDozYzI3NTJmOWY5YzU2MDAwNzA1YTRhZTE1YjNiYTY4YTVkMmU2NDRj',
           'commit': {
@@ -109,7 +109,7 @@ describe('Config', function () {
           'url': 'https://api.github.com/repos/appium/appium/commits/3c2752f9f9c56000705a4ae15b3ba68a5d2e644c',
           'html_url': 'https://github.com/appium/appium/commit/3c2752f9f9c56000705a4ae15b3ba68a5d2e644c',
           'comments_url': 'https://api.github.com/repos/appium/appium/commits/3c2752f9f9c56000705a4ae15b3ba68a5d2e644c/comments',
-        });
+        }});
         await verifyBuildInfoUpdate(false);
       });
     });