Merge pull request #38 from smartsheet/revert-36-personal/mmeyerink/u…

…pdate-http-request-library Revert "remove request in favor of axios"
smartsheet · Aug 22, 2023 · ae4ff5a · ae4ff5a
2 parents ff70b88 + 3bceed1
commit ae4ff5a
Show file tree

Hide file tree

Showing 10 changed files with 176 additions and 282 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,10 +4,6 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
-## [3.2.0] - 2023-08-17
-### Security
-- Removed `request` dependency and replaced it with `Axios`
-
 ## [3.1.2] - 2023-06-13
 ### Fixed
 - Added missing euBaseURI constant for smartsheet.eu 

diff --git a/lib/utils/httpRequestor.js b/lib/utils/httpRequestor.js
@@ -11,7 +11,7 @@ exports.create = function(requestorConfig) {
     : requestLogger.empty;
 
   var request = requestorConfig.request ||
-                Promise.promisifyAll(require("axios"), {multiArgs: true});
+                Promise.promisifyAll(require("request"), {multiArgs: true});
 
   var handleResponse = requestorConfig.handleResponse ||
                        require('./responseHandler');
@@ -83,12 +83,12 @@ exports.create = function(requestorConfig) {
   };
 
   var get = (options, callback) =>
-    methodRequest(options, request.get, 'GET', callback);
+    methodRequest(options, request.getAsync, 'GET', callback);
 
   var post = (options, callback) => {
     var requestExtension = { body: JSON.stringify(options.body) };
 
-    return methodRequest(options, request.post, 'POST', callback, requestExtension);
+    return methodRequest(options, request.postAsync, 'POST', callback, requestExtension);
   };
 
   var getFileBody = (options) => {
@@ -102,67 +102,56 @@ exports.create = function(requestorConfig) {
   var postFile = (options, callback) => {
     var requestExtension = { body: getFileBody(options) };
 
-    return methodRequest(options, request.post, 'POST', callback, requestExtension);
+    return methodRequest(options, request.postAsync, 'POST', callback, requestExtension);
   };
 
   var deleteFunc = (options, callback)  =>
-    methodRequest(options, request.delete, 'DELETE', callback);
+    methodRequest(options, request.delAsync, 'DELETE', callback);
 
   var put = (options, callback) => {
     var requestExtension = { body: JSON.stringify(options.body) };
 
-    return methodRequest(options, request.put, 'PUT', callback, requestExtension);
+    return methodRequest(options, request.putAsync, 'PUT', callback, requestExtension);
   };
 
   var methodRequest = (options, method, methodName, callback, requestExtension) => {
     var baseRequestOptions = {
       url: buildUrl(options),
       headers: buildHeaders(options),
-      params: options.queryParameters,
-      responseEncoding: options.encoding
+      qs: options.queryParameters,
+      encoding: options.encoding,
+      gzip: true
     };
-    var url = buildUrl(options);
     var requestOptions = _.extend(baseRequestOptions, requestExtension);
 
     var retryOptions = _.pick(options, 'maxRetryDurationMillis', 'calcRetryBackoff');
 
     logger.logRequest(methodName, requestOptions);
 
-    return makeRequestWithRetries(url, method, methodName, requestOptions, retryOptions, callback);
+    return makeRequestWithRetries(method, methodName, requestOptions, retryOptions, callback);
   };
 
-  var makeRequestWithRetries = (url, method, methodName, requestOptions, retryOptions, callback) => {
+  var makeRequestWithRetries = (method, methodName, requestOptions, retryOptions, callback) => {
     var effectiveRetryOptions = _.defaults(retryOptions, defaultRetryOptions);
 
     effectiveRetryOptions.endRetryTime = Date.now() + effectiveRetryOptions.maxRetryDurationMillis;
 
-    return retryHelper(url, method, methodName, requestOptions, effectiveRetryOptions, 0)
-      .then((response) => {
-        logger.logSuccessfulResponse(response);
-
-        if (callback) {
-          callback(undefined, response.content);
-        }
-
-        return response.content;
-      })
-      .catch((error) => {
-        logger.logErrorResponse(methodName, requestOptions, error);
-
-        if (callback) {
-          callback(error, undefined);
-        }
-
-        return new Promise.reject(_.omit(error, 'headers', 'body'));
-      });
+    return retryHelper(method, methodName, requestOptions, effectiveRetryOptions, 0)
+      .tap(logger.logSuccessfulResponse)
+      .tapCatch(error => logger.logErrorResponse(methodName, requestOptions, error))
+      .get('content')
+      .catch(error => Promise.reject(_.omit(error, 'headers', 'body')))
+      .nodeify(callback);
   };
 
-  var retryHelper = (url, method, methodName, requestOptions, retryOptions, numRetries) =>
-    method(url, requestOptions)
-      .then(handleResponse)
-      .catch(retryWithBackoffHelper(url, method, methodName, requestOptions, retryOptions, numRetries));
+  var retryHelper = (method, methodName, requestOptions, retryOptions, numRetries) =>
+    makeRequest(method, methodName, requestOptions)
+      .catch(retryWithBackoffHelper(method, methodName, requestOptions, retryOptions, numRetries));
 
-  var retryWithBackoffHelper = (url, method, methodName, requestOptions, retryOptions, numRetries) => {
+  var makeRequest = (method, methodName, requestOptions) =>
+    method(requestOptions).spread(handleResponse);
+
+  var retryWithBackoffHelper = (method, methodName, requestOptions, retryOptions, numRetries) => {
     return error => {
       var backoffMillis = retryOptions.calcRetryBackoff(numRetries, error);
 
@@ -179,7 +168,7 @@ exports.create = function(requestorConfig) {
       var nextRetry = numRetries + 1;
       logger.logRetryAttempt(methodName, requestOptions, error, nextRetry);
       return Promise.delay(backoffMillis)
-        .then(() => retryHelper(url, method, methodName, requestOptions, retryOptions, nextRetry));
+        .then(() => retryHelper(method, methodName, requestOptions, retryOptions, nextRetry));
     };
   };
 

diff --git a/lib/utils/responseHandler.js b/lib/utils/responseHandler.js
@@ -1,30 +1,33 @@
 var Promise = require("bluebird");
 
-module.exports = (response) => {
+module.exports = (response, body) => {
   var outResponse = {
-    statusCode: response.status,
+    statusCode: response.statusCode,
     headers: response.headers,
-    body: response.data
+    body: body
   };
 
-  if (response.status != 200) {
+  if (response.statusCode != 200) {
     var errorResponse = outResponse;
     if (/\bapplication\/json\b/.test(response.headers['content-type'])) {
-      errorResponse.errorCode = response.data.errorCode;
-      errorResponse.message = response.data.message;
-      errorResponse.refId = response.data.refId;
+      var responseBody = JSON.parse(body);
+      errorResponse.errorCode = responseBody.errorCode;
+      errorResponse.message = responseBody.message;
+      errorResponse.refId = responseBody.refId;
 
-      if (response.data.detail !== undefined) {
-        errorResponse.detail = response.data.detail;
+      if (responseBody.detail !== undefined) {
+        errorResponse.detail = responseBody.detail;
       }
     } else {
-      errorResponse.message = response.data;
+      errorResponse.message = body;
     }
 
     return new Promise.reject(errorResponse);
+  } else if (response.headers['content-type'] === 'application/json;charset=UTF-8') {
+    outResponse.content = JSON.parse(body);
+    return outResponse;
+  } else {
+    outResponse.content = body;
+    return outResponse;
   }
-
-  outResponse.content = response.data;
-
-  return outResponse;
 };