refactor: split core function of getAgent for better testing (#3392)

* refactor: split core function of getAgent for better testing

* refactor: use destructing over checking separate props of object

src/lib/http-agent.js

@@ -3,7 +3,7 @@ const { URL } = require('url')
 const { HttpsProxyAgent } = require('https-proxy-agent')
 const waitPort = require('wait-port')
 
-const { log } = require('../utils/command-helpers')
+const { log, exit } = require('../utils/command-helpers')
 const { NETLIFYDEVERR, NETLIFYDEVWARN } = require('../utils/logo')
 
 const fs = require('./fs')
@@ -28,23 +28,21 @@ const DEFAULT_HTTPS_PORT = 443
 // 50 seconds
 const AGENT_PORT_TIMEOUT = 50
 
-const getAgent = async ({ httpProxy, certificateFile, exit }) => {
+const tryGetAgent = async ({ httpProxy, certificateFile }) => {
   if (!httpProxy) {
-    return
+    return {}
   }
 
   let proxyUrl
   try {
     proxyUrl = new URL(httpProxy)
-  } catch (error) {
-    log(NETLIFYDEVERR, `${httpProxy} is not a valid URL`)
-    exit(1)
+  } catch {
+    return { error: `${httpProxy} is not a valid URL` }
   }
 
   const scheme = proxyUrl.protocol.slice(0, -1)
   if (!['http', 'https'].includes(scheme)) {
-    log(NETLIFYDEVERR, `${httpProxy} must have a scheme of http or https`)
-    exit(1)
+    return { error: `${httpProxy} must have a scheme of http or https` }
   }
 
   let open
@@ -57,22 +55,22 @@ const getAgent = async ({ httpProxy, certificateFile, exit }) => {
     })
   } catch (error) {
     // unknown error
-    log(NETLIFYDEVERR, `${httpProxy} is not available.`, error.message)
-    exit(1)
+    return { error: `${httpProxy} is not available.`, message: error.message }
   }
 
   if (!open) {
     // timeout error
-    log(NETLIFYDEVERR, `Could not connect to '${httpProxy}'`)
-    exit(1)
+    return { error: `Could not connect to '${httpProxy}'` }
   }
 
+  let response = {}
+
   let certificate
   if (certificateFile) {
     try {
       certificate = await fs.readFileAsync(certificateFile)
     } catch (error) {
-      log(NETLIFYDEVWARN, `Could not read certificate file '${certificateFile}'.`, error.message)
+      response = { warning: `Could not read certificate file '${certificateFile}'.`, message: error.message }
     }
   }
 
@@ -85,7 +83,20 @@ const getAgent = async ({ httpProxy, certificateFile, exit }) => {
   }
 
   const agent = new HttpsProxyAgentWithCA(opts)
+  response = { ...response, agent }
+  return response
+}
+
+const getAgent = async ({ httpProxy, certificateFile }) => {
+  const { error, warning, agent, message } = await tryGetAgent({ httpProxy, certificateFile })
+  if (error) {
+    log(NETLIFYDEVERR, error, message || '')
+    exit(1)
+  }
+  if (warning) {
+    log(NETLIFYDEVWARN, warning, message || '')
+  }
   return agent
 }
 
-module.exports = { getAgent }
+module.exports = { getAgent, tryGetAgent }

src/lib/http-agent.test.js

@@ -3,36 +3,31 @@ const http = require('http')
 const test = require('ava')
 const { createProxyServer } = require('http-proxy')
 const { HttpsProxyAgent } = require('https-proxy-agent')
-const sinon = require('sinon')
 
-const { getAgent } = require('./http-agent')
+const { tryGetAgent } = require('./http-agent')
 
-test(`should return undefined when there is no httpProxy`, async (t) => {
-  t.is(undefined, await getAgent({}))
+test(`should return an empty object when there is no httpProxy`, async (t) => {
+  t.deepEqual(await tryGetAgent({}), {})
 })
 
-test(`should exit with error on invalid url`, async (t) => {
+test(`should return error on invalid url`, async (t) => {
   const httpProxy = 'invalid_url'
-  const exit = sinon.stub()
-  exit.withArgs(1).throws('error')
-
-  await t.throwsAsync(getAgent({ httpProxy, exit }))
+  const result = await tryGetAgent({ httpProxy })
+  t.truthy(result.error)
 })
 
-test(`should exit with error on when scheme is not http or https`, async (t) => {
+test(`should return error when scheme is not http or https`, async (t) => {
   const httpProxy = 'file://localhost'
-  const exit = sinon.stub()
-  exit.withArgs(1).throws('error')
+  const result = await tryGetAgent({ httpProxy })
 
-  await t.throwsAsync(getAgent({ httpProxy, exit }))
+  t.truthy(result.error)
 })
 
-test(`should exit with error when proxy is not available`, async (t) => {
+test(`should return error when proxy is not available`, async (t) => {
   const httpProxy = 'https://unknown:7979'
-  const exit = sinon.stub()
-  exit.withArgs(1).throws('error')
+  const result = await tryGetAgent({ httpProxy })
 
-  await t.throwsAsync(getAgent({ httpProxy, exit }))
+  t.truthy(result.error)
 })
 
 test(`should return agent for a valid proxy`, async (t) => {
@@ -46,12 +41,9 @@ test(`should return agent for a valid proxy`, async (t) => {
   })
 
   const httpProxyUrl = `http://localhost:${server.address().port}`
-  const exit = sinon.stub()
-  exit.withArgs(1).throws('error')
-
-  const agent = await getAgent({ httpProxy: httpProxyUrl, exit })
+  const result = await tryGetAgent({ httpProxy: httpProxyUrl })
 
-  t.is(agent instanceof HttpsProxyAgent, true)
+  t.is(result.agent instanceof HttpsProxyAgent, true)
 
   server.close()
 })

src/utils/command.js

@@ -9,18 +9,7 @@ const API = require('netlify')
 
 const { getAgent } = require('../lib/http-agent')
 
-const {
-  pollForToken,
-  log,
-  exit,
-  warn,
-  error,
-  getToken,
-  getCwd,
-  argv,
-  normalizeConfig,
-  chalk,
-} = require('./command-helpers')
+const { pollForToken, log, exit, error, getToken, getCwd, argv, normalizeConfig, chalk } = require('./command-helpers')
 const getGlobalConfig = require('./get-global-config')
 const openBrowser = require('./open-browser')
 const StateConfig = require('./state-config')
@@ -62,7 +51,6 @@ class BaseCommand extends TrackedCommand {
     const agent = await getAgent({
       httpProxy: flags.httpProxy,
       certificateFile: flags.httpProxyCertificateFilename,
-      warn,
     })
     const apiOpts = { ...apiUrlOpts, agent }
     const globalConfig = await getGlobalConfig()