io.js v3 behavior change

[dpatti]

I don't know how many people actually run into this, but this was an issue for us when we were upgrading from io.js v2 to v3. Here's the minimal repro:

var mitm = require('mitm');
var http = require('http');

var intercept = mitm();
intercept.on('connection', function(socket){
  socket.destroy();
});

http.request("http://google.com").on('error', console.log);

We were using this pattern in our tests to simulate a network error. The results of the script on my machine:

$ nvm run 2 mitm.js
Running io.js v2.5.0 (npm v2.14.3)
{ [Error: socket hang up] code: 'ECONNRESET' }

$ nvm run 3 mitm.js
Running io.js v3.3.1 (npm v2.14.3)
_http_server.js:331
  var external = socket._handle._externalStream;
                               ^
TypeError: Cannot read property '_externalStream' of null
    at Mitm.connectionListener (_http_server.js:331:32)
    at Mitm.request (index.js:107:35)
    at ClientRequest.onSocket (underscore.js:748:26)
    at Agent.addRequest (_http_agent.js:146:9)
    at new ClientRequest (_http_client.js:133:16)
    at Object.exports.request (http.js:31:10)
    at Object.<anonymous> (mitm.js:9:6)
    at Module._compile (module.js:430:26)
    at Object.Module._extensions..js (module.js:448:10)
    at Module.load (module.js:355:32)

I did some tracing, and it seems to happen because socket.destroy() sets socket._handle to null (and always has), but socket._handle._externalStream is used in Http._connectionListener as of this commit: https://github.com/nodejs/node/commit/1bc446863f9b44dfd40e8c49f489180d53cce80c, which did not land until io.js v3.3.0.

An easy workaround for us is to just destroy the socket on the next tick. I don't know if this would qualify as something you'd want to fix.

As a bonus, this does not crash:

var http = require('http');

var server = http.createServer();
server.on('connection', function(socket){
  socket.destroy();
});

server.listen(8855);

http.request("http://localhost:8855").on('error', console.log);

because Http._connectionListener fires on the connection event, which the above handler is registered after.

[moll]

Thank you for the report! I don't believe there are any tests for destroying sockets, so it might be worthwhile indeed to look into this. I wonder how this will behave in Node v4 and others these days.

[moll]

I'm glad to report this issue has resolved itself over the past years. I did make a change some time ago to fire the connection event in the next tick. Perhaps that fixed it. I finally added a destroy-test to ensure it stays that way: https://github.com/moll/node-mitm/commit/cee7fb2419c417f41a73fc864f1b8ca46d0caba7. :)