HTTPS connection failing because socket has been deleted

[othiym23]

Here are two stacktraces that I have collected within the last few hours from two separate npm users. This failure has been seen on all of OS X, Windows, and Linux.:

104310 verbose stack AssertionError: null == true
104310 verbose stack     at TLSSocket.socketOnData (_http_client.js:308:3)
104310 verbose stack     at TLSSocket.emit (events.js:107:17)
104310 verbose stack     at TLSSocket.Readable.read (_stream_readable.js:373:10)
104310 verbose stack     at TLSSocket.socketCloseListener (_http_client.js:229:10)
104310 verbose stack     at TLSSocket.emit (events.js:129:20)
104310 verbose stack     at TCP.close (net.js:476:12)

102672 verbose stack AssertionError: null == true
102672 verbose stack     at TLSSocket.socketOnData (_http_client.js:308:3)
102672 verbose stack     at TLSSocket.emit (events.js:107:17)
102672 verbose stack     at TLSSocket.Readable.read (_stream_readable.js:373:10)
102672 verbose stack     at TLSSocket.socketCloseListener (_http_client.js:229:10)
102672 verbose stack     at TLSSocket.emit (events.js:129:20)
102672 verbose stack     at TCP.close (net.js:476:12)

See also npm/npm#7349 for more discussion and further links to people encountering this issue, which ultimately traces back to this bare assert in _http_client.js. It's rare enough that it's probably a race condition, and I seem to recall a similar issue getting fixed elsewhere in the not too distant pass. @indutny? Anybody?

[indutny]

The error seems to be originating from socket.parser === null, which may happen on:

• close event on socket
• parser error
• upgrade event without listener
• 100 status code
• error event
• end event

I don't think that upgrade or 100 status code may actually happen on npm. So this seems to be an OOB data after 'close'/'error'/'end' event.

[othiym23]

The relevant code in npm uses a custom http.Agent (to enable keep-alive) passed to request, and there are some issues we've been seeing with talking to mochiweb, the CouchDB HTTP server, that affect both node and iojs. My hypothesis is that something weird is happening around error-handling in the new agent keep-alive code, but I haven't had a chance to wireshark it or produce a reduced test case. Not sure the two issues are related, but it sure would be convenient for me if it were. ;)

[indutny]

Well, if the error event happens - socket will be destroyed immediately. Anyway, do you know any way to reproduce it? Even some script that does it in like 10000 runs would suffice.

[othiym23]

I'm trying, over on npm/npm#7349. ;) At this point we know about the same amount about what's going on. I'm also away from my computer, so I can't bang on this myself until tonight.

[indutny]

Test case:

var net = require('net');
var http = require('http');
var util = require('util');

function Agent() {
  http.Agent.call(this);
}
util.inherits(Agent, http.Agent);

Agent.prototype.createConnection = function() {
  var self = this;
  var socket = new net.Socket();

  socket.on('error', function() {
    socket.push('OH GOSH');
  });

  socket.on('newListener', function onNewListener(name) {
    if (name !== 'error')
      return;
    socket.removeListener('newListener', onNewListener);

    // Let other listeners to be set up too
    process.nextTick(function() {
      self.breakSocket(socket);
    });
  });

  return socket;
};

Agent.prototype.breakSocket = function breakSocket(socket) {
  socket.emit('error', new Error('Intentional error'));
};

var agent = new Agent();

http.request({
  agent: agent
}).once('error', function() {
  console.log('ignore');
});

[indutny]

Patch is coming soon.

[indutny]

Suggested fix https://github.com/iojs/io.js/pull/1103/files

[indutny]

The fix has been landed in io.js: nodejs/node@1a3ca82, please backport

[misterdjules]

Thank you @othiym23 and @indutny, added to the 0.12.1 milestone.

[misterdjules]

Thank you very much @indutny!

nodejs/node@1a3ca82 LGTM, and I would land it. However, I just want to wait for a while to see if we get any feedback on npm/npm#7349 (comment) before landing it as a fix for this issue.

[misterdjules]

Just for the record, this issue is fixed by nodejs/node@1a3ca82, see npm/npm#7349 (comment).

[misterdjules]

Landed in d6484f3, thank you @indutny and @othiym23!