Return last error when PoolCluster exhausts connection retries

fixes #818

Author: dougwilson

Changes.md

@@ -11,6 +11,7 @@ you spot any mistakes.
 * Clone connection config for new pool connections
 * Default `connectTimeout` to 2 minutes
 * Reject unauthorized SSL connections (use `ssl.rejectUnauthorized` to override) #816
+* Return last error when PoolCluster exhausts connection retries #818
 * Throw on unknown SSL profile name #817
 * User newer TLS functions when available #809
 

lib/PoolCluster.js

@@ -151,12 +151,8 @@ PoolCluster.prototype._getConnection = function(node, cb) {
   node.pool.getConnection(function (err, connection) {
     if (err) {
       self._increaseErrorCount(node);
-
-      if (self._canRetry) {
-        return cb(null, 'retry');
-      } else {
-        return cb(err);
-      }
+      cb(err);
+      return;
     } else {
       self._decreaseErrorCount(node);
     }
@@ -182,34 +178,48 @@ function PoolNamespace(cluster, pattern, selector) {
 
 PoolNamespace.prototype.getConnection = function(cb) {
   var clusterNode = this._getClusterNode();
+  var cluster     = this._cluster;
+  var namespace   = this;
 
   if (clusterNode === null) {
     return cb(new Error('Pool does not exist.'));
   }
 
-  this._cluster._getConnection(clusterNode, function(err, connection) {
-    if (err) {
-      return cb(err);
+  cluster._getConnection(clusterNode, function(err, connection) {
+    var retry = err && cluster._canRetry
+      && cluster._findNodeIds(namespace._pattern).length !== 0;
+
+    if (retry) {
+      return namespace.getConnection(cb);
     }
 
-    if (connection === 'retry') {
-      return this.getConnection(cb);
+    if (err) {
+      return cb(err);
     }
 
     cb(null, connection);
-  }.bind(this));
+  });
 };
 
-PoolNamespace.prototype._getClusterNode = function() {
+PoolNamespace.prototype._getClusterNode = function _getClusterNode() {
   var foundNodeIds = this._cluster._findNodeIds(this._pattern);
-
-  if (foundNodeIds.length === 0) {
-      return null;
+  var nodeId;
+
+  switch (foundNodeIds.length) {
+    case 0:
+      nodeId = null;
+      break;
+    case 1:
+      nodeId = foundNodeIds[0];
+      break;
+    default:
+      nodeId = this._selector(foundNodeIds);
+      break;
   }
 
-  var nodeId = (foundNodeIds.length === 1) ? foundNodeIds[0] : this._selector(foundNodeIds);
-
-  return this._cluster._getNode(nodeId);
+  return nodeId !== null
+    ? this._cluster._getNode(nodeId)
+    : null;
 };
 
 /**

test/integration/pool/test-cluster.js

@@ -221,7 +221,7 @@ function createPoolCluster(clusterConfig, poolConfig) {
   cluster.of('*', 'RR').getConnection(function (err, connection) {
     cluster.end();
 
-    assert.ok(err === null);
+    assert.ifError(err);
     assert.equal(connection._clusterId, 'CORRECT');
 
     assert.equal(removedNodeId, 'ERROR');

test/unit/pool-cluster/test-connection-error.js

@@ -0,0 +1,29 @@
+var assert  = require('assert');
+var common  = require('../../common');
+var cluster = common.createPoolCluster({
+  canRetry             : true,
+  removeNodeErrorCount : 5
+});
+var server  = common.createFakeServer();
+
+var connCount  = 0;
+var poolConfig = common.getTestConfig({port: common.fakeServerPort});
+cluster.add('MASTER', poolConfig);
+
+server.listen(common.fakeServerPort, function(err) {
+  assert.ifError(err);
+
+  cluster.getConnection('MASTER', function(err, conn){
+    assert.ok(err);
+    assert.equal(err.code, 'ER_HOST_NOT_PRIVILEGED');
+    assert.equal(err.fatal, true);
+    assert.equal(connCount, 5);
+    server.destroy();
+  });
+});
+
+server.on('connection', function(incomingConnection) {
+  var errno = 1130; // ER_HOST_NOT_PRIVILEGED
+  connCount += 1;
+  incomingConnection.deny('You suck.', errno);
+});