use http-transport-proxy instead of https-transport-proxy, leave body…

… as undefined instead of converting to empty string
bbc · Oct 5, 2023 · eed2dda · eed2dda
1 parent 7846433
commit eed2dda
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 21 deletions.
diff --git a/lib/transport/node-fetch.js b/lib/transport/node-fetch.js
@@ -3,12 +3,10 @@
 const Transport = require('./transport');
 const https = require('node:https');
 const http = require('node:http');
-const HttpsProxyAgent = require('https-proxy-agent').HttpsProxyAgent;
+const HttpProxyAgent = require('http-proxy-agent').HttpProxyAgent;
 const fetch = require('node-fetch');
 
 const REQUIRED_PROPERTIES = [
-  'body',
-  'elapsedTime',
   'url',
   'statusCode',
   'headers'
@@ -23,7 +21,7 @@ class FetchTransport extends Transport {
     }
 
     if (options?.defaults?.proxy) {
-      this._httpsProxyAgent = new HttpsProxyAgent(options.defaults.proxy, options?.agentOpts);
+      this._httpProxyAgent = new HttpProxyAgent(options.defaults.proxy, options?.agentOpts);
     }
 
     this.defaults = options?.defaults;
@@ -72,23 +70,27 @@ class FetchTransport extends Transport {
     to.headers = Object.fromEntries(from.headers.entries());
 
     // currently supports json and text formats only
-    to.body = await from.text();
-    const contentType = to.headers['content-type'];
-    if (this.parseAsJson(ctx, contentType)) {
-      try {
-        to.body = JSON.parse(to.body);
-      } catch {} // If body is not parseable, leave as text
+    const text = await from.text();
+
+    if (text) {
+      to.body = text;
+      const contentType = to.headers['content-type'];
+      if (this.parseAsJson(ctx, contentType)) {
+        try {
+          to.body = JSON.parse(to.body);
+        } catch {} // If body is not parseable, leave as text
+      }
     }
 
     to.httpResponse = from;
     return to;
   }
 
   selectAgent(ctx) {
-    if (this._httpsProxyAgent) return this._httpsProxyAgent;
+    if (this?.defaults?.proxy) return this._httpProxyAgent;
 
-    const protocol = new URL(ctx.req.getUrl()).protocol;
-    return protocol === 'http:' ? this._httpAgent : this._httpsAgent;
+    const http = new URL(ctx.req.getUrl()).protocol === 'http:';
+    return http ? this._httpAgent : this._httpsAgent;
   }
 
   async makeRequest(ctx, opts) {

diff --git a/package.json b/package.json
@@ -48,6 +48,7 @@
     "sinon": "^1.15.3"
   },
   "dependencies": {
+    "http-proxy-agent": "^7.0.0",
     "https-proxy-agent": "^7.0.2",
     "koa-compose": "^4.0.0",
     "lodash": "^4.17.4",

diff --git a/test/client.js b/test/client.js
@@ -83,7 +83,7 @@ describe('HttpTransportClient', () => {
         .get(url)
         .asResponse();
 
-      assert.deepEqual(res.body, '');
+      assert.deepEqual(res.body, undefined);
     });
 
     it('sets a default User-agent for every request', async () => {
@@ -554,22 +554,21 @@ describe('HttpTransportClient', () => {
     describe('setContextProperty', () => {
       it('sets an option in the context', async () => {
         nock.cleanAll();
-        api.get(path).reply(200, responseBody);
+        api.get(path).reply(200, '1234');
 
         const client = HttpTransport.createBuilder()
-          .use(toJson())
           .createClient();
 
         const res = await client
           .use(setContextProperty({
-            time: false
+            json: true
           },
           'opts'
           ))
           .get(url)
           .asResponse();
 
-        assert.isUndefined(res.elapsedTime);
+        assert.strictEqual(res.body, 1234);
       });
 
       it('sets an explict key on the context', async () => {

diff --git a/test/transport/node-fetch.js b/test/transport/node-fetch.js
@@ -369,7 +369,7 @@ describe('Request HTTP transport', () => {
           });
       });
 
-      it('selects proxy httpsAgent when protocol proxy has been provided', () => {
+      it('selects httpProxyAgent when proxy has been provided', () => {
         const ctx = createContext(url);
         const options = {
           defaults: {
@@ -386,12 +386,13 @@ describe('Request HTTP transport', () => {
           .catch(assert.ifError)
           .then(() => {
             sinon.assert.calledWithMatch(spy, url, { agent: {
-              proxy: new URL(proxyUrl)
+              proxy: new URL(proxyUrl),
+              protocol: 'http:'
             } });
           });
       });
 
-      it('selects proxy httpsAgent when protocol proxy has been provided and applies agent options', () => {
+      it('selects httpProxyAgent when proxy has been provided and applies agent options', () => {
         const ctx = createContext(url);
         const options = {
           agentOpts: {