fix: fetch patch ignoring command line arguments

BST-889
Fetch patch was making http requests by using needle directly
Changed to use lib request which uses command line arguments when invoking needle
Added tests for lib request
Changed fetch patch fail test to typescript
Removed patch fetch proxy test (now covered by lib request test)
Added @types/sinon devDependency to help write test with sinon assertions and matchers

package.json

@@ -103,6 +103,7 @@
     "@types/lodash": "^4.14.136",
     "@types/needle": "^2.0.4",
     "@types/node": "^6.14.4",
+    "@types/sinon": "^7.5.0",
     "@typescript-eslint/eslint-plugin": "^2.0.0",
     "@typescript-eslint/parser": "^2.0.0",
     "eslint": "^5.16.0",

src/lib/protect/fetch-patch.ts

@@ -1,57 +1,39 @@
-import * as needle from 'needle';
 import * as fs from 'fs';
 import * as analytics from '../analytics';
 import * as debugModule from 'debug';
-import * as proxyFromEnv from 'proxy-from-env';
-import * as ProxyAgent from 'proxy-agent';
+import request = require('../request');
 
 const debug = debugModule('snyk:fetch-patch');
-const getProxyForUrl = proxyFromEnv.getProxyForUrl;
 
 async function getPatchFile(
   patchUrl: string,
   patchFilename: string,
 ): Promise<string> {
-  let options;
-  const proxyUri = getProxyForUrl(patchUrl);
-  if (proxyUri) {
-    debug('Using proxy: ', proxyUri);
-    options = { agent: new ProxyAgent(proxyUri) };
-  }
-
-  let res: needle.NeedleResponse;
-  let patchData: string;
   try {
-    res = await needle('get', patchUrl, options || {});
-    if (!res || res.statusCode !== 200) {
-      throw res;
+    const response = await request({ url: patchUrl });
+    if (
+      !response ||
+      !response.res ||
+      !response.body ||
+      response.res.statusCode !== 200
+    ) {
+      throw response;
     }
-    patchData = res.body;
+    fs.writeFileSync(patchFilename, response.body);
     debug(
-      `Successfully downloaded patch from ${patchUrl}, patch size ${patchData.length} bytes`,
+      `Fetched patch from ${patchUrl} to ${patchFilename}, patch size ${response.body.length} bytes`,
     );
   } catch (error) {
-    debug(`Failed to download patch from ${patchUrl}`, error);
+    const errorMessage = `Failed to fetch patch from ${patchUrl} to ${patchFilename}`;
+    debug(errorMessage, error);
     analytics.add('patch-fetch-fail', {
-      message: error && (error.message || error.body),
-      code: error && error.statusCode,
-    });
-    throw error;
-  }
-
-  try {
-    fs.writeFileSync(patchFilename, patchData);
-    debug(`Successfully wrote patch to ${patchFilename}`);
-  } catch (error) {
-    debug(`Failed to write patch to ${patchFilename}`, error);
-    analytics.add('patch-fetch-fail', {
-      message: error && error.message,
+      message: (error && error.message) || errorMessage,
+      code: error && error.res && error.res.statusCode,
       patchFilename,
       patchUrl,
     });
-    throw error;
+    throw new Error(errorMessage);
   }
-
   return patchFilename;
 }
 

test/patch-fetch-fail.test.ts

@@ -0,0 +1,131 @@
+import { test } from 'tap';
+import * as proxyquire from 'proxyquire';
+
+let analytics;
+
+const proxyFetchPatch = proxyquire('../src/lib/protect/fetch-patch', {
+  '../request': () => {
+    return Promise.resolve({
+      res: {
+        statusCode: 200,
+      },
+      body: 'patch content',
+    });
+  },
+  fs: {
+    writeFileSync() {
+      return;
+    },
+  },
+  '../analytics': {
+    add(type, data) {
+      analytics = { type, data };
+    },
+  },
+});
+
+const proxyFetchPatchNotFound = proxyquire('../src/lib/protect/fetch-patch', {
+  '../request': () => {
+    return Promise.resolve({
+      res: {
+        statusCode: 404,
+      },
+      body: 'not found',
+    });
+  },
+  '../analytics': {
+    add(type, data) {
+      analytics = { type, data };
+    },
+  },
+});
+
+const proxyFetchPatchErrorWriting = proxyquire(
+  '../src/lib/protect/fetch-patch',
+  {
+    '../request': () => {
+      return Promise.resolve({
+        res: {
+          statusCode: 200,
+        },
+        body: 'patch content',
+      });
+    },
+    fs: {
+      writeFileSync() {
+        throw new Error('Error writing file');
+      },
+    },
+    '../analytics': {
+      add(type, data) {
+        analytics = { type, data };
+      },
+    },
+  },
+);
+
+test('fetch patch returns filename when successful', (t) => {
+  return proxyFetchPatch('https://test.patch.url', 'file.patch')
+    .then((name) => t.is(name, 'file.patch', 'filename returned'))
+    .catch((e) => t.fail('should not throw error', e));
+});
+
+test('fetch patch handles error responses', (t) => {
+  return proxyFetchPatchNotFound('https://test.patch.url', 'file.patch')
+    .then(() => t.fail('should have failed'))
+    .catch(() => {
+      t.is(
+        analytics.type,
+        'patch-fetch-fail',
+        'analytics type is patch-fetch-fail',
+      );
+      t.is(analytics.data.code, 404, 'analytics status code is 404');
+      t.is(
+        analytics.data.message,
+        'Failed to fetch patch from https://test.patch.url to file.patch',
+        'analytics message is expected',
+      );
+      t.is(
+        analytics.data.patchFilename,
+        'file.patch',
+        'analytics patch filename is expected',
+      );
+      t.is(
+        analytics.data.patchUrl,
+        'https://test.patch.url',
+        'analytics patch url is expected',
+      );
+    });
+});
+
+test('fetch patch handles errors thrown while writing file', (t) => {
+  return proxyFetchPatchErrorWriting('https://test.patch.url', 'file.patch')
+    .then(() => t.fail('should have failed'))
+    .catch(() => {
+      t.is(
+        analytics.type,
+        'patch-fetch-fail',
+        'analytics type is patch-fetch-fail',
+      );
+      t.is(
+        analytics.data.code,
+        undefined,
+        'analytics status code is undefined',
+      );
+      t.is(
+        analytics.data.message,
+        'Error writing file',
+        'analytics message indicates error writing file',
+      );
+      t.is(
+        analytics.data.patchFilename,
+        'file.patch',
+        'analytics patch filename is expected',
+      );
+      t.is(
+        analytics.data.patchUrl,
+        'https://test.patch.url',
+        'analytics patch url is expected',
+      );
+    });
+});