Skip to content

Commit

Permalink
Allow disabling connectTimeout, and disable that in all tests. Additi…
Browse files Browse the repository at this point in the history 
…onally, actually emit 'error', instead of an object
  • Loading branch information
Mark Cavage committed Sep 7, 2012
1 parent ca4b9bf commit 733906c
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 23 deletions.
27 changes: 18 additions & 9 deletions lib/client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,17 @@ function ZKClient(options) {
assert.object(options, 'options');
assert.object(options.log, 'options.log');
assert.arrayOfObject(options.servers, 'options.servers');
assert.optionalNumber(options.timeout, 'options.timeout');
assert.optionalNumber(options.pollInterval, 'options.pollInterval');
assert.optionalNumber(options.connectTimeout, 'options.connectTimeout');
assert.optionalNumber(options.timeout, 'options.timeout');

EventEmitter.call(this);

var _connect = [];
var self = this;

this.autoReconnect = options.autoReconnect;
this.connectTimeout = options.connectTimeout || 2000;
this.connectTimeout = (options.connectTimeout !== undefined ?
options.connectTimeout : 2000);
this.log = options.log.child({component: 'ZKPlus'}, true);
this.pollInterval = options.pollInterval || 0;
this.pollIntervalId = null;
Expand Down Expand Up @@ -111,7 +111,7 @@ ZKClient.prototype.onClose = function onClose() {
var log = this.log;
if (this.closedByClient) {
this.closedByClient = false;
log.debug('The zookeeper session is closed by the client');
log.debug('zookeeper session closed by caller');
this.emit('close');
} else {
// the session has expired:
Expand Down Expand Up @@ -158,8 +158,10 @@ ZKClient.prototype.connect = function connect(callback) {
callback = function () {};
}

var connectTimeout = this.connectTimeout;
var log = this.log;
var self = this;
var timer;
var zk = this.zk;

switch (this.getState()) {
Expand Down Expand Up @@ -187,11 +189,18 @@ ZKClient.prototype.connect = function connect(callback) {
break;
}

var timer = setTimeout(function () {
self.emit(new ZKError(ZK.ZSYSTEMERROR,
'(zkplus) connect timeout reached'));

}, this.connectTimeout);
if (connectTimeout !== false) {
timer = setTimeout(function onConnectTimeout() {
if (self.getState() !== 'connected') {
self.emit('error',
new ZKError(ZK.ZSYSTEMERROR,
'connect timeout (' +
connectTimeout +
'ms)'));
}
}, connectTimeout);
timer.stack = new Error().stack;
}

log.trace('connecting');
zk.connect(function connectCallback(err) {
Expand Down
1 change: 1 addition & 0 deletions lib/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function createLogger() {
function ZKPlus(options) {
options = options || {};
var opts = {
connectTimeout: options.connectTimeout,
connect: (options.connect !== undefined ?
options.connect : true),
servers: [],
Expand Down
8 changes: 2 additions & 6 deletions test/client.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ var PATH = ROOT + '/' + uuid().substr(0, 7);
var FILE = PATH + '/unit_test.json';
var SUBDIR = PATH + '/foo/bar/baz';
var ZK;
var connectTimeout;

var HOST = process.env.ZK_HOST || 'localhost';
var PORT = parseInt(process.env.ZK_PORT, 10) || 2181;
Expand All @@ -32,11 +31,8 @@ var PORT = parseInt(process.env.ZK_PORT, 10) || 2181;

before(function (callback) {
try {
connectTimeout = setTimeout(function () {
console.error('Could not connect to a ZK instance');
}, 1500);

This comment has been minimized.

Sign in to view

Copy link
@vvo

vvo Sep 7, 2012

Contributor

ahh now I remember, I had done this only for testing, and it became an option :)
having it in tests was good because otherwise you won't know why your npm test had failed.
ZK = zk.createClient({
connectTimeout: false,
log: helper.createLogger('zk.client.test.js'),
servers: [ {
host: HOST,
Expand All @@ -46,7 +42,6 @@ before(function (callback) {
pollInterval: 200
});
ZK.once('connect', function () {
clearTimeout(connectTimeout);
ZK.mkdirp(PATH, function (err) {
if (err) {
console.error(err.stack);
Expand Down Expand Up @@ -255,6 +250,7 @@ test('watch (data+initialRead)', function (t) {

test('trigger sessionExpired', function (t) {
var ZK2 = zk.createClient({
connectTimeout: false,
log: helper.createLogger('zk.client.test.js'),
servers: [ {
host: (process.env.ZK_HOST || 'localhost'),
Expand Down
1 change: 1 addition & 0 deletions test/election.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var PORT = parseInt(process.env.ZK_PORT, 10) || 2181;
before(function (callback) {
try {
ZK = zk.createClient({
connectTimeout: false,
log: LOG,
servers: [ {
host: HOST,
Expand Down
1 change: 1 addition & 0 deletions test/generic-election.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var PORT = parseInt(process.env.ZK_PORT, 10) || 2181;
test('beforeClass', function (t) {
try {
ZK = zk.createClient({
connectTimeout: false,
pollInterval: 200,
log: LOG,
servers: [ {
Expand Down
13 changes: 5 additions & 8 deletions test/server-string.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,16 @@ var connectTimeout;
///--- Tests
before(function (callback) {
try {
connectTimeout = setTimeout(function () {
console.error('Could not connect to a ZK instance');
process.exit();
}, 1500);

ZK = zk.createClient({
connectTimeout: false,
log: helper.createLogger('zk.server-string.test.js'),
servers: [process.env.ZK_HOST || 'localhost' + ':' +
process.env.ZK_PORT || '2181'],
servers: [{
host: process.env.ZK_HOST || '127.0.0.1',
port: parseInt(process.env.ZK_PORT || 2181, 10)
} ],
timeout: 1000
});
ZK.on('connect', function () {
clearTimeout(connectTimeout);
ZK.mkdirp(PATH, function (err) {
if (err) {
console.error(err.stack);
Expand Down

0 comments on commit 733906c

Please sign in to comment.