Fix "close" event on Node v12.16.3.

Author: moll

.travis.yml

@@ -60,6 +60,12 @@ node_js:
   # returning a string with 2 little-endian bytes in a String.
   # Then again, this could be https://github.com/nodejs/node/pull/27936.
   - "12.4"
+  - "12.16.2"
+
+  # v12.16.3 changed something in InternalSocket.prototype.shutdown or possibly
+  # WritableStream.prototype.end causing the "close" event to not be emitted.
+  # https://github.com/moll/node-mitm/issues/66
+  - "12.16.3"
   - "12"
   - "13"
   - "14"

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## Unreleased
 - Fixes one test for Node v12.4.
+- Fixes the socket "close" event on Node v12.16.3.
 
 ## 1.7.0 (Jan 30, 2019)
 - Adds compatibility with Node v10.15.1.

lib/internal_socket.js

@@ -34,10 +34,14 @@ function InternalSocket(remote) {
   if (remote) this.remote = remote
   this.id = ++uniqueId
 
-  // End is for ReadableStream.prototype.push(null).
-  // Finish is for WritableStream.prototype.end.
+  // The "end" event follows ReadableStream.prototype.push(null).
   this.on("data", readData.bind(this))
   this.on("end", readEof.bind(this))
+
+  // The "finish" event follows  WritableStream.prototype.end.
+  //
+  // There's WritableStream.prototype._final for processing before "finish" is
+  // emitted, but that's only available in Node v8 and later.
   this.on("finish", this._write.bind(this, null, null, noop))
 
   return this.pause(), this
@@ -55,7 +59,6 @@ InternalSocket.prototype = Object.create(DuplexStream.prototype, {
 // v0.11.14 does it in the next tick. For easier sync use, call it here.
 InternalSocket.prototype.readStart = function() { this.resume() }
 InternalSocket.prototype.readStop = function() { this.pause() }
-InternalSocket.prototype.close = InternalSocket.prototype.end
 
 InternalSocket.prototype._read = noop
 InternalSocket.prototype.ref = noop
@@ -122,12 +125,14 @@ InternalSocket.prototype.writeUcs2String = function(req, data) {
 }
 
 // While it seems to have existed since Node v0.10, Node v11.2 requires
-// "shutdown".
+// "shutdown". AFAICT, "shutdown" is for shutting the writable side down and
+// hence the use of WritableStream.prototype.end and waiting for the "finish"
+// event.
 if (
   NODE_11_2_AND_LATER ||
   NODE_10_15_1_AND_MAJOR
 ) InternalSocket.prototype.shutdown = function(req) {
-  this.once("end", req.oncomplete.bind(req, NO_ERROR, req.handle))
+  this.once("finish", req.oncomplete.bind(req, NO_ERROR, req.handle))
   this.end()
 
   // Note v11.8 requires "shutdown" to return an error value, with "1"
@@ -136,6 +141,13 @@ if (
   return 0
 }
 
+// I'm unsure of the relationship between InternalSocket.prototype.shutdown and
+// InternalSocket.prototype.close.
+InternalSocket.prototype.close = function(done) {
+  if (!this._writableState.finished) this.end(done)
+  else if (done) done()
+}
+
 // Node v0.10 will use writeQueueSize to see if it should set write request's
 // "cb" property or write more immediately.
 if (NODE_0_10) InternalSocket.prototype.writeQueueSize = 0

test/index_test.js

@@ -94,6 +94,24 @@ describe("Mitm", function() {
         module.connect(80, "localhost", done.bind(null, null))
       })
 
+      // The "close" event broke on Node v12.16.3 as the
+      // InternalSocket.prototype.close method didn't call back if
+      // the WritableStream had already been closed.
+      it("must emit close on socket if ended immediately", function(done) {
+        this.mitm.on("connection", function(socket) { socket.end() })
+        var socket = module.connect({host: "foo"})
+        socket.on("close", done.bind(null, null))
+      })
+
+      it("must emit close on socket if ended in next tick", function(done) {
+        this.mitm.on("connection", function(socket) {
+          process.nextTick(socket.end.bind(socket))
+        })
+
+        var socket = module.connect({host: "foo"})
+        socket.on("close", done.bind(null, null))
+      })
+
       it("must intercept 127.0.0.1", function(done) {
         var server; this.mitm.on("connection", function(s) { server = s })
         var client = module.connect({host: "127.0.0.1"})