fix: remove unnecessary utils and cleanup destructuring

npm · Oct 2, 2023 · d2608ea · d2608ea
1 parent e24eb6c
commit d2608ea
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 57 deletions.
diff --git a/lib/agents.js b/lib/agents.js
@@ -4,7 +4,6 @@ const net = require('net')
 const tls = require('tls')
 const { once } = require('events')
 const timers = require('timers/promises')
-const { urlify, appendPort } = require('./util')
 const { normalizeOptions, cacheOptions } = require('./options')
 const { getProxy, getProxyAgent, proxyCache } = require('./proxy.js')
 const Errors = require('./errors.js')
@@ -14,8 +13,10 @@ module.exports = class Agent extends AgentBase {
   #options
   #timeouts
   #proxy
+  #noProxy
+  #ProxyAgent
 
-  constructor (options) {
+  constructor (options = {}) {
     const { timeouts, proxy, noProxy, ...normalizedOptions } = normalizeOptions(options)
 
     super(normalizedOptions)
@@ -24,26 +25,24 @@ module.exports = class Agent extends AgentBase {
     this.#timeouts = timeouts
 
     if (proxy) {
-      this.#proxy = {
-        proxy: urlify(proxy),
-        noProxy,
-        Agent: getProxyAgent(proxy),
-      }
+      this.#proxy = new URL(proxy)
+      this.#noProxy = noProxy
+      this.#ProxyAgent = getProxyAgent(proxy)
     }
   }
 
   get proxy () {
-    return this.#proxy ? { url: this.#proxy.proxy } : {}
+    return this.#proxy ? { url: this.#proxy } : {}
   }
 
   #getProxy (options) {
     if (!this.#proxy) {
       return
     }
 
-    const proxy = getProxy(appendPort(`${options.protocol}//${options.host}`, options.port), {
-      proxy: this.#proxy.proxy,
-      noProxy: this.#proxy.noProxy,
+    const proxy = getProxy(`${options.protocol}//${options.host}:${options.port}`, {
+      proxy: this.#proxy,
+      noProxy: this.#noProxy,
     })
 
     if (!proxy) {
@@ -61,7 +60,7 @@ module.exports = class Agent extends AgentBase {
       return proxyCache.get(cacheKey)
     }
 
-    let { Agent: ProxyAgent } = this.#proxy
+    let ProxyAgent = this.#ProxyAgent
     if (Array.isArray(ProxyAgent)) {
       ProxyAgent = options.secureEndpoint ? ProxyAgent[1] : ProxyAgent[0]
     }
@@ -79,7 +78,7 @@ module.exports = class Agent extends AgentBase {
     if (timeout) {
       const connectionTimeout = timers.setTimeout(timeout, null, { signal: ac.signal })
         .then(() => {
-          throw new Errors.ConnectionTimeoutError(options)
+          throw new Errors.ConnectionTimeoutError(`${options.host}:${options.port}`)
         }).catch((err) => {
           if (err.name === 'AbortError') {
             return
@@ -151,7 +150,7 @@ module.exports = class Agent extends AgentBase {
 
     if (this.#timeouts.idle) {
       socket.setTimeout(this.#timeouts.idle, () => {
-        socket.destroy(new Errors.IdleTimeoutError(options))
+        socket.destroy(new Errors.IdleTimeoutError(`${options.host}:${options.port}`))
       })
     }
 
@@ -178,7 +177,7 @@ module.exports = class Agent extends AgentBase {
       let responseTimeout
       request.once('finish', () => {
         setTimeout(() => {
-          request.destroy(new Errors.ResponseTimeoutError(request, this.proxy?.url))
+          request.destroy(new Errors.ResponseTimeoutError(request, this.#proxy))
         }, this.#timeouts.response)
       })
       request.once('response', () => {
@@ -190,7 +189,7 @@ module.exports = class Agent extends AgentBase {
       let transferTimeout
       request.once('response', (res) => {
         setTimeout(() => {
-          res.destroy(new Errors.TransferTimeoutError(request, this.proxy?.url))
+          res.destroy(new Errors.TransferTimeoutError(request, this.#proxy))
         }, this.#timeouts.transfer)
         res.once('close', () => {
           clearTimeout(transferTimeout)

diff --git a/lib/errors.js b/lib/errors.js
@@ -1,7 +1,5 @@
 'use strict'
 
-const { appendPort } = require('./util')
-
 class InvalidProxyProtocolError extends Error {
   constructor (url) {
     super(`Invalid protocol \`${url.protocol}\` connecting to proxy \`${url.host}\``)
@@ -11,17 +9,15 @@ class InvalidProxyProtocolError extends Error {
 }
 
 class ConnectionTimeoutError extends Error {
-  constructor ({ host, port }) {
-    host = appendPort(host, port)
+  constructor (host) {
     super(`Timeout connecting to host \`${host}\``)
     this.code = 'ECONNECTIONTIMEOUT'
     this.host = host
   }
 }
 
 class IdleTimeoutError extends Error {
-  constructor ({ host, port }) {
-    host = appendPort(host, port)
+  constructor (host) {
     super(`Idle timeout reached for host \`${host}\``)
     this.code = 'EIDLETIMEOUT'
     this.host = host

diff --git a/lib/index.js b/lib/index.js
@@ -1,7 +1,6 @@
 'use strict'
 
 const { LRUCache } = require('lru-cache')
-const { urlify } = require('./util')
 const { normalizeOptions, cacheOptions } = require('./options')
 const { getProxy, proxyCache } = require('./proxy.js')
 const dns = require('./dns.js')
@@ -15,7 +14,7 @@ const getAgent = (url, { agent, proxy, noProxy, ...options } = {}) => {
     return agent
   }
 
-  url = urlify(url)
+  url = new URL(url)
 
   const proxyForUrl = getProxy(url, { proxy, noProxy })
   const normalizedOptions = {

diff --git a/lib/proxy.js b/lib/proxy.js
@@ -5,26 +5,23 @@ const { HttpsProxyAgent } = require('https-proxy-agent')
 const { SocksProxyAgent } = require('socks-proxy-agent')
 const { LRUCache } = require('lru-cache')
 const { InvalidProxyProtocolError } = require('./errors.js')
-const { urlify } = require('./util.js')
 
 const PROXY_CACHE = new LRUCache({ max: 20 })
 
-const PROXY_ENV = (() => {
-  const keys = new Set(['https_proxy', 'http_proxy', 'proxy', 'no_proxy'])
-  const values = {}
-  for (let [key, value] of Object.entries(process.env)) {
-    key = key.toLowerCase()
-    if (keys.has(key)) {
-      values[key] = value
-    }
-  }
-  return values
-})()
-
 const SOCKS_PROTOCOLS = new Set(SocksProxyAgent.protocols)
 
+const PROXY_ENV_KEYS = new Set(['https_proxy', 'http_proxy', 'proxy', 'no_proxy'])
+
+const PROXY_ENV = Object.entries(process.env).reduce((acc, [key, value]) => {
+  key = key.toLowerCase()
+  if (PROXY_ENV_KEYS.has(key)) {
+    acc[key] = value
+  }
+  return acc
+}, {})
+
 const getProxyAgent = (url) => {
-  url = urlify(url)
+  url = new URL(url)
 
   const protocol = url.protocol.slice(0, -1)
   if (SOCKS_PROTOCOLS.has(protocol)) {
@@ -65,7 +62,7 @@ const isNoProxy = (url, noProxy) => {
 }
 
 const getProxy = (url, { proxy, noProxy }) => {
-  url = urlify(url)
+  url = new URL(url)
 
   if (!proxy) {
     proxy = url.protocol === 'https:'
@@ -81,7 +78,7 @@ const getProxy = (url, { proxy, noProxy }) => {
     return null
   }
 
-  return urlify(proxy)
+  return new URL(proxy)
 }
 
 module.exports = {

diff --git a/lib/util.js b/lib/util.js
diff --git a/test/index.js b/test/index.js
@@ -271,6 +271,10 @@ t.test('getAgent', (t) => {
 })
 
 t.test('http agent', (t) => {
+  t.test('does not throw with no args', async (t) => {
+    t.ok(new HttpAgent())
+  })
+
   t.test('throws for incompatible proxy protocols', async (t) => {
     t.throws(() => {
       new HttpAgent({ proxy: 'foo://not-supported' })