From f7276475b9d6e7c4dc2a7adb6d9861338faacd70 Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Thu, 19 Jan 2012 18:02:10 -0800 Subject: [PATCH] GH-51 Timeout support in client library --- bin/ldapjs-search | 10 ++- docs/client.md | 11 +++- lib/client.js | 145 +++++++++++++++++++++++++++----------------- tst/client.test.js | 13 ++++ tst/laundry.test.js | 2 +- 5 files changed, 122 insertions(+), 59 deletions(-) diff --git a/bin/ldapjs-search b/bin/ldapjs-search index e0d7391..06fd75a 100755 --- a/bin/ldapjs-search +++ b/bin/ldapjs-search @@ -38,6 +38,7 @@ var opts = { 'password': String, 'persistent': Boolean, 'scope': String, + 'timeout': Number, 'url': url }; @@ -49,6 +50,7 @@ var shortOpts = { 'w': ['--password'], 'p': ['--persistent'], 's': ['--scope'], + 't': ['--timeout'], 'u': ['--url'] }; @@ -129,13 +131,19 @@ if (!parsed.persistent) var client = ldap.createClient({ url: parsed.url, - log4js: log4js + log4js: log4js, + timeout: parsed.timeout || false }); client.on('error', function(err) { perror(err); }); +client.on('timeout', function(req) { + process.stderr.write('Timeout reached\n'); + process.exit(1); +}); + client.bind(parsed.binddn, parsed.password, function(err, res) { if (err) perror(err); diff --git a/docs/client.md b/docs/client.md index fbbda2e..b38b551 100644 --- a/docs/client.md +++ b/docs/client.md @@ -28,7 +28,8 @@ client is: ||url|| a valid LDAP url.|| ||socketPath|| If you're running an LDAP server over a Unix Domain Socket, use this.|| ||log4js|| You can optionally pass in a log4js instance the client will use to acquire a logger. The client logs all messages at the `Trace` level.|| -||numConnections||The size of the connection pool. Default is 1.|| +||timeout||How long the client should let operations live for before timing out. Default is Infinity.|| +||connectTimeout||How long the client should wait before timing out on TCP connections. Default is up to the OS.|| ||reconnect||Whether or not to automatically reconnect (and rebind) on socket errors. Takes amount of time in millliseconds. Default is 1000. 0/false will disable altogether.|| ## Connection management @@ -45,6 +46,14 @@ operations be allowed back through; in the meantime all callbacks will receive a `DisconnectedError`. If you never called `bind`, the client will allow operations when the socket is connected. +Also, note that the client will emit a `timeout` event if an operation +times out, and you'll be passed in the request object that was offending. You +probably don't _need_ to listen on it, as the client will also return an error +in the callback of that request. However, it is useful if you want to have a +catch-all. An event of `connectTimout` will be emitted when the client fails to +get a socket in time; there are no arguments. Note that this event will be +emitted (potentially) in reconnect scenarios as well. + ## Common patterns The last two parameters in every API are `controls` and `callback`. `controls` diff --git a/lib/client.js b/lib/client.js index a47da62..9f0618f 100644 --- a/lib/client.js +++ b/lib/client.js @@ -124,13 +124,18 @@ function Client(options) { this.secure = this.url.secure; } - this.log4js = options.log4js || logStub; + this.connection = null; + this.connectTimeout = options.connectTimeout || false; this.connectOptions = { port: self.url ? self.url.port : options.socketPath, host: self.url ? self.url.hostname : undefined, socketPath: options.socketPath || undefined }; + this.log4js = options.log4js || logStub; + this.reconnect = (typeof(options.reconnect) === 'number' ? + options.reconnect : 1000); this.shutdown = false; + this.timeout = options.timeout || false; this.__defineGetter__('log', function() { if (!self._log) @@ -139,11 +144,7 @@ function Client(options) { return self._log; }); - this.reconnect = (typeof(options.reconnect) === 'number' ? - options.reconnect : 1000); - - this.connection = null; - this.connect(); + return this.connect(function() {}); } util.inherits(Client, EventEmitter); module.exports = Client; @@ -157,24 +158,39 @@ module.exports = Client; */ Client.prototype.connect = function(callback) { if (this.connection) - return; + return callback(); var self = this; - var connection = this.connection = this._newConnection(); + var timer = false; + if (this.connectTimeout) { + timer = setTimeout(function() { + if (self.connection) + self.connection.destroy(); + + var err = new ConnectionError('timeout'); + self.emit('connectTimeout'); + return callback(err); + }, this.connectTimeout); + } + + this.connection = this._newConnection(); function reconnect() { self.connection = null; if (self.reconnect) - setTimeout(function() { self.connect(); }, self.reconnect); + setTimeout(function() { self.connect(function() {}); }, self.reconnect); } - this.connection.on('close', function(had_err) { + self.connection.on('close', function(had_err) { self.emit('close', had_err); reconnect(); }); - this.connection.on('connect', function() { + self.connection.on('connect', function() { + if (timer) + clearTimeout(timer); + if (self._bindDN && self._credentials) return self.bind(self._bindDN, self._credentials, function(err) { if (err) { @@ -183,9 +199,11 @@ Client.prototype.connect = function(callback) { } self.emit('connect'); + return callback(); }); self.emit('connect'); + return callback(); }); }; @@ -214,24 +232,25 @@ Client.prototype.bind = function(name, credentials, controls, callback, conn) { if (typeof(callback) !== 'function') throw new TypeError('callback (function) required'); - this.connect(); + var self = this; + this.connect(function() { + var req = new BindRequest({ + name: name || '', + authentication: 'Simple', + credentials: credentials || '', + controls: controls + }); - var req = new BindRequest({ - name: name || '', - authentication: 'Simple', - credentials: credentials || '', - controls: controls + return self._send(req, [errors.LDAP_SUCCESS], function(err, res) { + if (!err) { // In case we need to reconnect later + self._bindDN = name; + self._credentials = credentials; + } + + return callback(err, res); + }, conn); }); - - return self._send(req, [errors.LDAP_SUCCESS], function(err, res) { - if (!err) { // In case we need to reconnect later - self._bindDN = name; - self._credentials = credentials; - } - - return callback(err, res); - }, conn); }; @@ -691,7 +710,7 @@ Client.prototype.unbind = function(callback) { this._bindDN = null; this._credentials = null; - if (!this.connect) + if (!this.connection) return callback(); var req = new UnbindRequest(); @@ -708,21 +727,22 @@ Client.prototype._send = function(message, expect, callback, connection) { var conn = connection || this.connection; var self = this; + var timer; function closeConn(err) { + if (timer) + clearTimeout(timer); + var err = err || new ConnectionError('no connection'); - // This is lame, but we want to send the original error back, whereas - // this will trigger a connection event - process.nextTick(function() { - if (conn) - conn.destroy(); - }); + if (typeof(callback) === 'function') { + callback(err); + } else { + callback.emit('error', err); + } - if (typeof(callback) === 'function') - return callback(err); - - return callback.emit('error', err); + if (conn) + conn.destroy(); } if (!conn) @@ -731,7 +751,10 @@ Client.prototype._send = function(message, expect, callback, connection) { // Now set up the callback in the messages table message.messageID = conn.ldap.nextMessageID; if (expect !== 'abandon') { - conn.ldap.messages[message.messageID] = function(res) { + conn.ldap.messages[message.messageID] = function _sendCallback(res) { + if (timer) + clearTimeout(timer); + if (self.log.isDebugEnabled()) self.log.debug('%s: response received: %j', conn.ldap.id, res.json); @@ -778,27 +801,36 @@ Client.prototype._send = function(message, expect, callback, connection) { }; } - // Finally send some data - if (this.log.isDebugEnabled()) - this.log.debug('%s: sending request: %j', conn.ldap.id, message.json); - - // Note if this was an unbind, we just go ahead and end, since there - // will never be a response - var _writeCb = null; - if (expect === 'abandon') { - _writeCb = function() { - return callback(); - }; - } else if (expect === 'unbind') { - _writeCb = function() { - conn.unbindMessageID = message.id; - conn.end(); - }; - } else { - // noop + // If there's a user specified timeout, pick that up + if (this.timeout) { + timer = setTimeout(function() { + self.emit('timeout', message); + if (conn.ldap.messages[message.messageID]) + return conn.ldap.messages[message.messageID](new LDAPResult({ + status: 80, // LDAP_OTHER + errorMessage: 'request timeout (client interrupt)' + })); + }, this.timeout); } try { + // Note if this was an unbind, we just go ahead and end, since there + // will never be a response + var _writeCb = null; + if (expect === 'abandon') { + _writeCb = function() { + return callback(); + }; + } else if (expect === 'unbind') { + _writeCb = function() { + conn.unbindMessageID = message.id; + conn.end(); + }; + } + + // Finally send some data + if (this.log.isDebugEnabled()) + this.log.debug('%s: sending request: %j', conn.ldap.id, message.json); return conn.write(message.toBer(), _writeCb); } catch (e) { return closeConn(e); @@ -906,6 +938,7 @@ Client.prototype._newConnection = function() { if (log.isTraceEnabled()) log.trace('%s timeout event=%s', c.ldap.id); + self.emit('timeout'); c.end(); }); diff --git a/tst/client.test.js b/tst/client.test.js index ec69f0d..84cbec2 100644 --- a/tst/client.test.js +++ b/tst/client.test.js @@ -75,6 +75,10 @@ test('setup', function(t) { return next(); }); + server.search('dc=timeout', function(req, res, next) { + // Haha client! + }); + server.search(SUFFIX, function(req, res, next) { if (req.dn.equals('cn=ref,' + SUFFIX)) { @@ -575,6 +579,15 @@ test('unbind (GH-30)', function(t) { }); +test('search timeout (GH-51)', function(t) { + client.timeout = 250; + client.search('dc=timeout', 'objectclass=*', function(err, res) { + t.ok(err); + t.end(); + }); +}); + + test('shutdown', function(t) { client.unbind(function(err) { server.on('close', function() { diff --git a/tst/laundry.test.js b/tst/laundry.test.js index 32d64ba..4b6401b 100644 --- a/tst/laundry.test.js +++ b/tst/laundry.test.js @@ -63,7 +63,7 @@ test('setup', function(t) { mail: uuid() + '@pogostick.org' } }; - + if (req.filter.matches(entry.attributes)) res.send(entry);