node-ldapjs icon indicating copy to clipboard operation
node-ldapjs copied to clipboard

Client timeout doesn't work, searches hang forever if remote server goes down

Open glennschmidt opened this issue 7 years ago • 1 comments

To reproduce:

  1. Create a client, setting the timeout option (eg. 4 seconds):
ldap.createClient({url: 'ldap://directory:389', timeout: 4000, connectTimeout: 4000});
  1. starttls then bind
  2. search -> The search is successful
  3. Shut down the server (slapd), then start it up again
  4. Issue another search -> The callback is never called, the search never succeeds or fails, and the client doesn't emit any 'error' event

Ideally, ldapjs should notice that its TCP connection has gone down (due to the server restarting), and would automatically reconnect when the next query is issued.

But at minimum, it should notice that it hasn't received a response to the search within the 4-second timeout window, and emit an error so that I can reconnect manually.

glennschmidt avatar Jan 17 '18 13:01 glennschmidt

Concerning the issue of the never resolving promise, I use the workaround of wrapping the res EventEmitter in a bluebird promise:

const resultPromise = new Promise((resolve, reject) => {
  const entries = [];
  res
  .on('searchEntry', ({object: entry }) => {
    entries.push(entry);
  })
  .on('error', err )> {
    reject(err);
  })
  .on('end', ({status}) => {
    if (status !== 0) {
      return reject(new Error(`LDAP error: ${status});
    }
    resolve(entries);
  });
}).timeout(4 * 1000, 'LDAP search timed out');

This solution is not clean as we don't know the state of the TCP connection at this point but it works.

We found out that a concurrency issue (solved with https://github.com/joyent/node-ldapjs/pull/424) cause the EventEmitter to hang forever without emitting any error. The timeout & connectTimeout did not seems to work neither.

Hope it helps.

rdubigny avatar Feb 01 '18 12:02 rdubigny

👋

On February 22, 2023, we released version 3 of this library. As a result, we are closing this issue/pull request.

Please see issue #839 for more information, including how to proceed if you feel this closure is in error.

jsumners avatar Feb 22 '23 19:02 jsumners