feat: add a timeout options to RequestClient that creates a custom ht…

…tps agent (#775) * Add a timeout options to RequestClient that creates a custom https agent * Remove per-request agent
twilio · Jul 14, 2022 · e24f3fc · e24f3fc
1 parent 795bc09
commit e24f3fc
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 25 deletions.
diff --git a/README.md b/README.md
@@ -138,7 +138,7 @@ npm test
 To run just one specific test file instead of the whole suite, provide a JavaScript regular expression that will match your spec file's name, like:
 
 ```bash
-npm run test -- -m .\*client.\*
+npm run test:javascript -- -m .\*client.\*
 ```
 
 [apidocs]: https://www.twilio.com/docs/api

diff --git a/lib/base/RequestClient.d.ts b/lib/base/RequestClient.d.ts
@@ -1,13 +1,15 @@
-import { HttpMethod } from '../interfaces';
-import Response = require('../http/response');
+import { HttpMethod } from "../interfaces";
+import Response = require("../http/response");
 
 declare class RequestClient {
-  constructor();
+  constructor(opts?: RequestClient.RequestClientOptions);
   /**
    * Make an HTTP request
    * @param opts The request options
    */
-  request<TData>(opts: RequestClient.RequestOptions<TData>): Promise<Response<TData>>;
+  request<TData>(
+    opts: RequestClient.RequestOptions<TData>
+  ): Promise<Response<TData>>;
 }
 
 declare namespace RequestClient {
@@ -54,9 +56,17 @@ declare namespace RequestClient {
     forever?: boolean;
   }
 
+  export interface RequestClientOptions {
+    /**
+     * A timeout in milliseconds. This will be used as the HTTPS agent's socket
+     * timeout, and as the default request timeout.
+     */
+    timeout?: number;
+  }
+
   export interface Headers {
     [header: string]: string;
   }
 }
 
-export = RequestClient;
+export = RequestClient;
diff --git a/lib/base/RequestClient.js b/lib/base/RequestClient.js
@@ -6,12 +6,42 @@ var fs = require('fs');
 var HttpsProxyAgent = require('https-proxy-agent');
 var Q = require('q');
 var qs = require('qs');
+var url = require('url');
+var https = require('https');
 var Response = require('../http/response');
 var Request = require('../http/request');
 
-axios.defaults.headers.post['Content-Type'] = 'application/x-www-form-urlencoded';
+const DEFAULT_CONTENT_TYPE = 'application/x-www-form-urlencoded';
+const DEFAULT_TIMEOUT = 30000;
 
-var RequestClient = function () { };
+/**
+ * Make http request
+ * @param {object} opts - The options argument
+ * @param {string} opts.timeout - A custom timeout to use. This will be used as the socket timeout, and as the default request timeout.
+ */
+var RequestClient = function (opts) {
+  opts = opts || {};
+  this.defaultTimeout = opts.timeout || DEFAULT_TIMEOUT;
+
+  // construct an https agent
+  let agentOpts = {
+    timeout: this.defaultTimeout,
+  };
+
+  let agent;
+  if (process.env.HTTP_PROXY) {
+    // Note: if process.env.HTTP_PROXY is set, we're not able to apply the given
+    // socket timeout. See: https://github.com/TooTallNate/node-https-proxy-agent/pull/96
+    agent = new HttpsProxyAgent(process.env.HTTP_PROXY);
+  } else {
+    agent = new https.Agent(agentOpts);
+  }
+
+  // construct an axios instance
+  this.axios = axios.create();
+  this.axios.defaults.headers.post['Content-Type'] = DEFAULT_CONTENT_TYPE;
+  this.axios.defaults.httpsAgent = agent;
+};
 
 /**
  * Make http request
@@ -53,12 +83,11 @@ RequestClient.prototype.request = function (opts) {
   }
 
   var options = {
-    timeout: opts.timeout || 30000,
+    timeout: opts.timeout || this.defaultTimeout,
     maxRedirects: opts.allowRedirects ? 10 : 0, // Same number of allowed redirects as request module default
     url: opts.uri,
     method: opts.method,
     headers: opts.headers,
-    httpsAgent: process.env.HTTP_PROXY ? new HttpsProxyAgent(process.env.HTTP_PROXY) : undefined,
     proxy: false,
     validateStatus: status => status >= 100 && status < 600,
   };
@@ -99,7 +128,7 @@ RequestClient.prototype.request = function (opts) {
   this.lastResponse = undefined;
   this.lastRequest = new Request(optionsRequest);
 
-  axios(options).then((response) => {
+  this.axios(options).then((response) => {
     if (opts.logLevel === 'debug') {
       console.log(`response.statusCode: ${response.status}`)
       console.log(`response.headers: ${JSON.stringify(response.headers)}`)
@@ -122,7 +151,7 @@ RequestClient.prototype.filterLoggingHeaders = function (headers){
   return Object.keys(headers).filter((header) => {
     return !'authorization'.includes(header.toLowerCase());
   });
-}
+};
 
 RequestClient.prototype.logRequest = function (options){
   console.log('-- BEGIN Twilio API Request --');
@@ -140,6 +169,6 @@ RequestClient.prototype.logRequest = function (options){
   }
 
   console.log('-- END Twilio API Request --');
-}
+};
 
 module.exports = RequestClient;
diff --git a/spec/unit/base/RequestClient.spec.js b/spec/unit/base/RequestClient.spec.js
@@ -3,17 +3,89 @@
 const mockfs = require('mock-fs');
 const proxyquire = require('proxyquire');
 const Q = require('q');
+const HttpsProxyAgent = require('https-proxy-agent');
+
+function createMockAxios(promiseHandler) {
+  return {
+    create: function(createOptions) {
+      let instance = function(requestOptions) {
+        const deferred = Q.defer();
+        promiseHandler(deferred, { createOptions: createOptions, requestOptions: requestOptions });
+        return deferred.promise;
+      };
+
+      instance.defaults = {
+        headers: {
+          post: {},
+        },
+      };
+
+      return instance;
+    },
+  };
+}
+
+describe('RequestClient constructor', function() {
+  let RequestClientMock;
+  let initialHttpProxyValue = process.env.HTTP_PROXY;
+
+  beforeEach(function() {
+    RequestClientMock = proxyquire('../../../lib/base/RequestClient', {
+      axios: createMockAxios(function(deferred) {
+        deferred.resolve({status: 200, data: 'voltron', headers: {response: 'header'}});
+      }),
+    });
+  });
+
+  afterEach(function() {
+    if (initialHttpProxyValue) {
+      process.env.HTTP_PROXY = initialHttpProxyValue;
+    } else {
+      delete process.env.HTTP_PROXY;
+    }
+  });
+
+  it('should initialize with default values', function() {
+    const requestClient = new RequestClientMock();
+    expect(requestClient.defaultTimeout).toEqual(30000);
+    expect(requestClient.axios.defaults.headers.post).toEqual({
+      'Content-Type': 'application/x-www-form-urlencoded',
+    });
+    expect(requestClient.axios.defaults.httpsAgent).not.toBeInstanceOf(HttpsProxyAgent);
+    expect(requestClient.axios.defaults.httpsAgent.options.timeout).toEqual(30000);
+  });
+
+  it('should initialize with a proxy', function() {
+    process.env.HTTP_PROXY = 'http://example.com:8080';
+
+    const requestClient = new RequestClientMock();
+    expect(requestClient.defaultTimeout).toEqual(30000);
+    expect(requestClient.axios.defaults.headers.post).toEqual({
+      'Content-Type': 'application/x-www-form-urlencoded',
+    });
+    expect(requestClient.axios.defaults.httpsAgent).toBeInstanceOf(HttpsProxyAgent);
+    expect(requestClient.axios.defaults.httpsAgent.proxy.host).toEqual('example.com');
+  });
+
+  it('should initialize with a timeout', function() {
+    const requestClient = new RequestClientMock({ timeout: 5000 });
+    expect(requestClient.defaultTimeout).toEqual(5000);
+    expect(requestClient.axios.defaults.headers.post).toEqual({
+      'Content-Type': 'application/x-www-form-urlencoded',
+    });
+    expect(requestClient.axios.defaults.httpsAgent).not.toBeInstanceOf(HttpsProxyAgent);
+    expect(requestClient.axios.defaults.httpsAgent.options.timeout).toEqual(5000);
+  });
+});
 
 describe('lastResponse and lastRequest defined', function() {
   let client;
   let response;
   beforeEach(function() {
     const RequestClientMock = proxyquire('../../../lib/base/RequestClient', {
-      axios: function(options) {
-        const deferred = Q.defer();
+      axios: createMockAxios(function(deferred) {
         deferred.resolve({status: 200, data: 'voltron', headers: {response: 'header'}});
-        return deferred.promise;
-      },
+      }),
     });
 
     client = new RequestClientMock();
@@ -69,11 +141,9 @@ describe('lastRequest defined, lastResponse undefined', function() {
   let options;
   beforeEach(function() {
     const RequestClientMock = proxyquire('../../../lib/base/RequestClient', {
-      axios: function(options) {
-        const deferred = Q.defer();
+      axios: createMockAxios(function(deferred) {
         deferred.reject('failed');
-        return deferred.promise;
-      },
+      }),
     });
 
     client = new RequestClientMock();
@@ -112,11 +182,9 @@ describe('User specified CA bundle', function() {
   let options;
   beforeEach(function() {
     const RequestClientMock = proxyquire('../../../lib/base/RequestClient', {
-      axios: function (options) {
-        const deferred = Q.defer();
+      axios: createMockAxios(function(deferred) {
         deferred.reject('failed');
-        return deferred.promise;
-      },
+      }),
     });
 
     client = new RequestClientMock();