Clean up client handling of end/error for searches

Author: pfmooney

lib/client/client.js

@@ -1375,7 +1375,7 @@ Client.prototype._sendSocket = function _sendSocket(message,
   var timer = false;
   var sentEmitter = false;
 
-  function _done(event, obj) {
+  function sendResult(event, obj) {
     if (event === 'error' && self.listeners('resultError')) {
       self.emit('resultError', obj);
     }
@@ -1385,19 +1385,15 @@ Client.prototype._sendSocket = function _sendSocket(message,
         // Execute callback with the error instead.
         if (!sentEmitter)
           return callback(obj);
-        emitter.removeAllListeners('end');
       }
-      if (event === 'end')
-        emitter.removeAllListeners('error');
-
       return emitter.emit(event, obj);
     }
 
     if (event === 'error')
       return callback(obj);
 
     return callback(null, obj);
-  } // end function _done(event, obj)
+  }
 
   function messageCallback(msg) {
     if (timer)
@@ -1407,29 +1403,29 @@ Client.prototype._sendSocket = function _sendSocket(message,
       log.trace({msg: msg ? msg.json : null}, 'response received');
 
     if (expect === 'abandon')
-      return _done('end', null);
+      return sendResult('end', null);
 
     if (msg instanceof SearchEntry || msg instanceof SearchReference) {
       var event = msg.constructor.name;
       event = event[0].toLowerCase() + event.slice(1);
-      return _done(event, msg);
+      return sendResult(event, msg);
     } else {
       tracker.remove(message.messageID);
       // Potentially mark client as idle
       self._updateIdle();
 
       if (msg instanceof LDAPResult) {
-        if (expect.indexOf(msg.status) === -1)
-          return _done('error', errors.getError(msg));
-
-        return _done('end', msg);
+        if (expect.indexOf(msg.status) === -1) {
+          return sendResult('error', errors.getError(msg));
+        }
+        return sendResult('end', msg);
       } else if (msg instanceof Error) {
-        return _done('error', msg);
+        return sendResult('error', msg);
       } else {
-        return _done('error', new errors.ProtocolError(msg.type));
+        return sendResult('error', new errors.ProtocolError(msg.type));
       }
     }
-  } // end function messageCallback(msg)
+  }
 
   function onRequestTimeout() {
     self.emit('timeout', message);
@@ -1438,7 +1434,7 @@ Client.prototype._sendSocket = function _sendSocket(message,
       //FIXME: the timed-out request should be abandoned
       cb(new errors.TimeoutError('request timeout (client interrupt)'));
     }
-  } // end function onRequestTimeout()
+  }
 
   function writeCallback() {
     if (expect === 'abandon') {
@@ -1461,7 +1457,7 @@ Client.prototype._sendSocket = function _sendSocket(message,
       return callback(null, emitter);
     }
     return false;
-  } // end writeCallback()
+  }
 
   // Start actually doing something...
   tracker.track(message, messageCallback);

test/client.test.js

@@ -215,6 +215,37 @@ test('setup', function (t) {
     }
   });
 
+  server.search('cn=pagederr', function (req, res, next) {
+    var cookie = null;
+    req.controls.forEach(function (control) {
+      if (control.type === ldap.PagedResultsControl.OID) {
+        cookie = control.value.cookie;
+      }
+    });
+    if (cookie && Buffer.isBuffer(cookie) && cookie.length === 0) {
+      // send first "page"
+        res.send({
+          dn: util.format('o=result, cn=pagederr'),
+          attributes: {
+            o: 'result',
+            objectclass: ['pagedResult']
+          }
+        });
+        res.controls.push(new ldap.PagedResultsControl({
+          value: {
+            size: 2,
+            cookie: new Buffer('a')
+          }
+        }));
+        res.end();
+        return next();
+    } else {
+      // send error instead of second page
+        res.end(ldap.LDAP_SIZE_LIMIT_EXCEEDED);
+        return next();
+    }
+  });
+
   server.search('dc=empty', function (req, res, next) {
     res.send({
       dn: 'dc=empty',
@@ -694,6 +725,30 @@ test('search paged', function (t) {
     }
   });
 
+  t.test('paged - handle later error', function (t2) {
+    var countEntries = 0;
+    var countPages = 0;
+    client.search('cn=pagederr', {
+      paged: { pageSize: 1 }
+    }, function (err, res) {
+      t2.ifError(err);
+      res.on('searchEntry', function () {
+        t2.ok(++countEntries);
+      });
+      res.on('page', function () {
+        t2.ok(++countPages);
+      });
+      res.on('error', function (error) {
+        t2.equal(countEntries, 1);
+        t2.equal(countPages, 1);
+        t2.end();
+      });
+      res.on('end', function () {
+        t2.fail('should not be reached');
+      });
+    });
+  });
+
   t.end();
 });