Skip to content

Commit

Permalink
Reject on autodiscovery failure
Browse files Browse the repository at this point in the history
When autodiscovery fails cache operations are silently queued and
are never dequeued. This leaves the client in an unusable state and
only warns the consumer if onNetError is provided. Since failed
autodiscovery is not recoverable all queued operations should be
rejected.

Related to victorquinn#36.
  • Loading branch information
Jeff Jagoda committed Jun 19, 2018
1 parent 9a63c1e commit e79bdab
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 7 deletions.
31 changes: 24 additions & 7 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ Client.prototype.disconnect = function(opts) {
*/
Client.prototype.getHostList = function() {

var client = this;
var connections = {};
// Promise.any because we don't care which completes first, as soon as we get
// a list of hosts we can stop
Expand All @@ -165,15 +166,24 @@ Client.prototype.getHostList = function() {
// Do the autodiscovery, then resolve with hosts
return deferred.resolve(this.autodiscovery());
},
onError: this.onNetError
onError: function (err) {
client.onNetError(err);
deferred.reject(err);
}
});

return deferred.promise;
}, this)).bind(this).then(function(hosts) {
this.hosts = hosts;
this.connectToHosts();
this.flushBuffer();
});
}, this)).bind(this).then(
function(hosts) {
this.hosts = hosts;
this.connectToHosts();
this.flushBuffer();
},
function (err) {
var wrappedError = new Error('Autodiscovery failed. Errors were:\n' + err.join('\n---\n'));
this.flushBuffer(wrappedError);
}
);
};

/**
Expand Down Expand Up @@ -209,14 +219,21 @@ Client.prototype.connectToHosts = function() {
*
* @api private
*/
Client.prototype.flushBuffer = function() {
Client.prototype.flushBuffer = function(err) {
if (this.buffer && this.buffer.size > 0) {
debug('flushing client write buffer');
// @todo Watch out for and handle how this behaves with a very long buffer
while(this.buffer.size > 0) {
var item = this.buffer.first();
this.buffer = this.buffer.shift();

// Something bad happened before things got a chonce to run. We
// need to cancel all pending operations.
if (err) {
item.deferred.reject(err);
continue;
}

// First, retrieve the correct connection out of the hashring
var connection = this.connections[this.ring.get(item.key)];

Expand Down
13 changes: 13 additions & 0 deletions test/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,19 @@ describe('Client', function() {
});
});
*/

it('throws on autodiscovery failure', function() {
var cache = new Client({ hosts: ['badserver:11211'], autodiscover: true });
var val = chance.word();

return cache.set('test', val)
.then(function () { throw new Error('should not get here'); })
.catch(function(err) {
err.should.be.ok;
err.should.be.an.instanceof(Error);
err.message.should.match(/Autodiscovery failed/);
});
});
});

describe('set and get', function() {
Expand Down

0 comments on commit e79bdab

Please sign in to comment.