GH-51 Timeout support in client library

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);

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`

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' ?
+  return this.connect(function() {});
-                    options.reconnect : 1000);
-
-  this.connection = null;
-  this.connect();
 }
 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,9 +232,9 @@ Client.prototype.bind = function(name, credentials, controls, callback, conn) {
   if (typeof(callback) !== 'function')
     throw new TypeError('callback (function) required');
 
-  this.connect();
-  var self = this;
 
+  var self = this;
+  this.connect(function() {
     var req = new BindRequest({
       name: name || '',
       authentication: 'Simple',
@@ -232,6 +250,7 @@ Client.prototype.bind = function(name, credentials, controls, callback, conn) {
 
       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
+    if (typeof(callback) === 'function') {
-    // this will trigger a connection event
+      callback(err);
-    process.nextTick(function() {
+    } else {
+      callback.emit('error', err);
+    }
+
     if (conn)
       conn.destroy();
-    });
-
-    if (typeof(callback) === 'function')
-      return callback(err);
-
-    return callback.emit('error', err);
   }
 
   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,10 +801,19 @@ Client.prototype._send = function(message, expect, callback, connection) {
     };
   }
 
-  // Finally send some data
+  // If there's a user specified timeout, pick that up
-  if (this.log.isDebugEnabled())
+  if (this.timeout) {
-    this.log.debug('%s: sending request: %j', conn.ldap.id, message.json);
+    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;
@@ -794,11 +826,11 @@ Client.prototype._send = function(message, expect, callback, connection) {
         conn.unbindMessageID = message.id;
         conn.end();
       };
-  } else {
-    // noop
     }
 
-  try {
+    // 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();
   });
 

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() {