Merge pull request #607 from sidorares/connect-time-errors

Fix crash when first packet from server is error packet

Author: sidorares

lib/connection.js

@@ -87,7 +87,7 @@ function Connection(opts) {
 
   this.clientEncoding = CharsetToEncoding[this.config.charsetNumber];
 
-  this.stream.once('error', connection._handleNetworkError.bind(this));
+  this.stream.on('error', connection._handleNetworkError.bind(this));
 
   // see https://gist.github.com/khoomeister/4985691#use-that-instead-of-bind
   this.packetParser = new PacketParser(function(p) {
@@ -123,11 +123,17 @@ function Connection(opts) {
   if (!this.config.isServer) {
     handshakeCommand = new Commands.ClientHandshake(this.config.clientFlags);
     handshakeCommand.on('end', function() {
+      // this happens when handshake finishes early and first packet is error
+      // and not server hello ( for example, 'Too many connactions' error)
+      if (!handshakeCommand.handshake) {
+        return;
+      }
       connection._handshakePacket = handshakeCommand.handshake;
       connection.threadId = handshakeCommand.handshake.connectionId;
       connection.emit('connect', handshakeCommand.handshake);
     });
     handshakeCommand.on('error', function(err) {
+      connection._closing = true;
       connection._notifyError(err);
     });
     this.addCommand(handshakeCommand);
@@ -212,7 +218,16 @@ Connection.prototype._notifyError = function(err) {
     connection._command.onResult(err);
     connection._command = null;
   } else {
-    bubbleErrorToConnection = true;
+    // connection handshake is special because we allow it to be implicit
+    // if error happened during handshake, but there are others commands in queue
+    // then bubble error to other commands and not to connection
+    if (
+      !(connection._command &&
+        connection._command.constructor == Commands.ClientHandshake &&
+        connection._commands.length > 0)
+    ) {
+      bubbleErrorToConnection = true;
+    }
   }
   while ((command = connection._commands.shift())) {
     if (command.onResult) {
@@ -791,6 +806,10 @@ Connection.prototype.destroy = function() {
 };
 
 Connection.prototype.close = function() {
+  if (this.connectTimeout) {
+    Timers.clearTimeout(this.connectTimeout);
+    this.connectTimeout = null;
+  }
   this._closing = true;
   this.stream.end();
   var connection = this;
@@ -900,7 +919,7 @@ Connection.prototype.writeOk = function(args) {
 Connection.prototype.writeError = function(args) {
   // if we want to send error before initial hello was sent, use default encoding
   var encoding = this.serverConfig ? this.serverConfig.encoding : 'cesu8';
-  this.writePacket(Packets.Error.toPacket(args, this.serverConfig.encoding));
+  this.writePacket(Packets.Error.toPacket(args, encoding));
 };
 
 Connection.prototype.serverHandshake = function serverHandshake(args) {

test/integration/connection/test-connect-time-error.js

@@ -0,0 +1,36 @@
+var mysql = require('../../../index.js');
+var assert = require('assert');
+var Buffer = require('safe-buffer').Buffer;
+
+var server;
+
+const ERROR_TEXT = 'test error';
+
+var portfinder = require('portfinder');
+portfinder.getPort(function(err, port) {
+  var server = mysql.createServer();
+  server.listen(port);
+  server.on('connection', function(conn) {
+    console.log('Here!');
+    conn.writeError(new Error(ERROR_TEXT));
+    conn.close();
+  });
+
+  var connection = mysql.createConnection({
+    host: 'localhost',
+    port: port,
+    user: 'testuser',
+    database: 'testdatabase',
+    password: 'testpassword'
+  });
+
+  connection.query('select 1+1', function(err) {
+    assert.equal(err.message, ERROR_TEXT);
+  });
+
+  connection.query('select 1+2', function(err) {
+    assert.equal(err.message, ERROR_TEXT);
+    connection.close();
+    server._server.close();
+  });
+});