Clean up socket in case of error (#98)

.github/workflows/build.yml

@@ -27,6 +27,10 @@ jobs:
       run: |
         npm install
 
-    - name: Test
+    - name: Test / 1
       run: |
         npm run test-ci
+
+    - name: Test / 2
+      run: |
+        ./test/hang-socket/runner.sh

index.js

@@ -43,6 +43,7 @@ class HttpProxyAgent extends http.Agent {
       if (response.statusCode === 200) {
         callback(null, socket)
       } else {
+        socket.destroy()
         callback(new Error(`Bad response: ${response.statusCode}`), null)
       }
     })
@@ -101,6 +102,7 @@ class HttpsProxyAgent extends https.Agent {
         const secureSocket = super.createConnection({ ...options, socket })
         callback(null, secureSocket)
       } else {
+        socket.destroy()
         callback(new Error(`Bad response: ${response.statusCode}`), null)
       }
     })

package.json

@@ -13,7 +13,7 @@
     "./*": "./*.js"
   },
   "scripts": {
-    "test": "standard && NODE_EXTRA_CA_CERTS=test/fixtures/certs_unit_test.pem ava -v test/*.test.js && tsd",
+    "test": "standard && NODE_EXTRA_CA_CERTS=test/fixtures/certs_unit_test.pem ava -v test/*.test.js && NODE_EXTRA_CA_CERTS=test/fixtures/certs_unit_test.pem ./test/hang-socket/runner.sh && tsd",
     "test-ci": "standard && ava -v test/*.test.js && tsd"
   },
   "engines": {

test/hang-socket/http.js

@@ -0,0 +1,67 @@
+'use strict'
+
+const http = require('http')
+const { createServer, SERVER_HOSTNAME } = require('../utils')
+const { HttpProxyAgent } = require('../../')
+
+function request (opts) {
+  return new Promise((resolve, reject) => {
+    const req = http.request(opts, resolve)
+    req.on('error', reject)
+    req.end(opts.body)
+  })
+}
+
+const timeout = setTimeout(() => {
+  console.log('The http agent is not cleaning up hanging sockets')
+  process.exit(1)
+}, 5000)
+
+async function run () {
+  const server = await createServer()
+  server.on('connect', (request, socket, head) => {
+    socket.on('end', () => {
+      clearTimeout(timeout)
+    })
+    const lines = [
+      'HTTP/1.1 403 FORBIDDEN',
+      '',
+      'Forbidden'
+    ]
+    socket.write(lines.join('\r\n'))
+  })
+
+  const agent = new HttpProxyAgent({
+    keepAlive: true,
+    keepAliveMsecs: 1000,
+    maxSockets: 256,
+    maxFreeSockets: 256,
+    scheduling: 'lifo',
+    timeout: 500,
+    proxy: `http://${SERVER_HOSTNAME}:${server.address().port}`
+  })
+
+  try {
+    await request({
+      method: 'GET',
+      hostname: 'www.example.com',
+      port: '',
+      path: '/',
+      agent
+    })
+    console.error(new Error('Should throw'))
+    process.exit(1)
+  } catch (err) {
+    if (err.message !== 'Bad response: 403') {
+      console.error(new Error('Expected a different error'))
+      process.exit(1)
+    }
+  }
+
+  server.close()
+}
+
+run().catch(err => {
+  console.error(err)
+  process.exit(1)
+})

test/hang-socket/https.js

@@ -0,0 +1,67 @@
+'use strict'
+
+const https = require('https')
+const { createSecureServer, SERVER_HOSTNAME } = require('../utils')
+const { HttpsProxyAgent } = require('../../')
+
+function request (opts) {
+  return new Promise((resolve, reject) => {
+    const req = https.request(opts, resolve)
+    req.on('error', reject)
+    req.end(opts.body)
+  })
+}
+
+const timeout = setTimeout(() => {
+  console.log('The https agent is not cleaning up hanging sockets')
+  process.exit(1)
+}, 5000)
+
+async function run () {
+  const server = await createSecureServer()
+  server.on('connect', (request, socket, head) => {
+    socket.on('end', () => {
+      clearTimeout(timeout)
+    })
+    const lines = [
+      'HTTP/1.1 403 FORBIDDEN',
+      '',
+      'Forbidden'
+    ]
+    socket.write(lines.join('\r\n'))
+  })
+
+  const agent = new HttpsProxyAgent({
+    keepAlive: true,
+    keepAliveMsecs: 1000,
+    maxSockets: 256,
+    maxFreeSockets: 256,
+    scheduling: 'lifo',
+    timeout: 500,
+    proxy: `https://${SERVER_HOSTNAME}:${server.address().port}`
+  })
+
+  try {
+    await request({
+      method: 'GET',
+      hostname: 'www.example.com',
+      port: '',
+      path: '/',
+      agent
+    })
+    console.error(new Error('Should throw'))
+    process.exit(1)
+  } catch (err) {
+    if (err.message !== 'Bad response: 403') {
+      console.error(new Error('Expected a different error, got:', err.message))
+      process.exit(1)
+    }
+  }
+
+  server.close()
+}
+
+run().catch(err => {
+  console.error(err)
+  process.exit(1)
+})

test/hang-socket/runner.sh

@@ -0,0 +1,6 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+node test/hang-socket/http.js
+node test/hang-socket/https.js