Removed default session timeout from HTTP2 agent

lib/http2/http2Agent.js

@@ -12,18 +12,12 @@ class Http2Agent extends EventEmitter {
     const _options = Object.assign({}, options, this.options)
     const name = this.getName(_options)
     let connection = this.connections[name]
-    if (_options.sessionIdleTimeout === undefined || _options.sessionIdleTimeout === null) {
-      _options.sessionIdleTimeout = 5000
-    }
 
     if (!connection || connection.destroyed || connection.closed) {
-      // check if a socket is supplied
-
-      let connectionOptions = {}
-
-      // Omitting create connections since there is a signature mismatch b/w http1 and http2 and we don't want to mess with it.
-      connectionOptions = Object.assign({}, options, { createConnection: undefined, port: _options.port || 443 })
+      const connectionOptions = Object.assign({}, options,
+        { createConnection: undefined, port: _options.port || 443 })
 
+      // check if a socket is supplied
       if (socket) {
         connectionOptions.createConnection = () => socket
       }
@@ -42,21 +36,28 @@ class Http2Agent extends EventEmitter {
       connection.ref = function () {
         this.refCount++
         oldRef.call(this)
-        connection.setTimeout(0)
         connection.removeAllListeners('timeout')
+        connection.setTimeout(0)
       }
       const connectionsMap = this.connections
       connection.unref = function () {
         this.refCount--
         if (this.refCount === 0) {
           oldUnref.call(this)
-          connection.setTimeout(_options.sessionIdleTimeout, () => {
-            connection.close()
-            delete connectionsMap[name]
-          })
+          if (_options.sessionIdleTimeout) {
+            connection.setTimeout(_options.sessionIdleTimeout, () => {
+              connection.close()
+              delete connectionsMap[name]
+            })
+          }
         }
       }
 
+      // Add a default error listener to HTTP2 session object to transparently swallow errors incase no streams are active
+      connection.on('error', () => {
+        // Do nothing
+      })
+
       this.connections[name] = connection
     }
     connection.ref()

lib/http2/request.js

@@ -70,10 +70,15 @@ class Http2Request extends EventEmitter {
 
   registerListeners () {
     this.stream.on('drain', () => this.emit('drain', arguments))
-    this.stream.on('error', (e) => this.emit('error', e))
+    this.stream.on('error', (e) => {
+      this.emit('error', e)
+    })
 
     this.stream.on('close', (...args) => {
       this.emit('close', args)
+      this._client.off('error', this.onError)
+      this.stream.removeAllListeners()
+      this.removeAllListeners()
     })
 
     this._client.once('error', this.onError)
@@ -82,7 +87,7 @@ class Http2Request extends EventEmitter {
     })
 
     this.stream.on('end', () => {
-      this._client.off('error', this.onError)
+      this.emit('end')
     })
   }
 

tests/test-timeout-http2.js

@@ -27,9 +27,14 @@ var tape = require('tape')
 var s = server.createHttp2Server()
 destroyable(s)
 
+var streams = []
 // Request that waits for 200ms
 s.on('/timeout', function (req, res) {
+  streams.push(req.stream)
   setTimeout(function () {
+    if (res.stream.closed) {
+      return
+    }
     res.writeHead(200, {'content-type': 'text/plain'})
     res.write('waited')
     res.end()
@@ -156,6 +161,18 @@ tape('float timeout', function (t) { // should be rounded by setTimeout anyway
 })
 
 tape('cleanup', function (t) {
+  const sessions = []
+
+  streams.forEach((stream) => {
+    sessions.push(stream.session)
+    stream.destroy()
+  })
+
+  sessions.forEach((session) => {
+    if (!session) { return }
+    session.close()
+  })
+
   s.close(function () {
     t.end()
   })