Node-ldapjs: ldapjs is not handling all errors

Created on 26 Nov 2018  路  5Comments  路  Source: ldapjs/node-ldapjs

We've been getting occasional app crashes with vague outputs referring to events.js and sometimes tcp errors or TLSSocket errors. With such little context it could have been anything which makes an outside connection.

The only way we were able to get details was by adding a catch all for uncaughtException

// the ugly catch all
process.on('uncaughtException', (error) => {
    const { message, stack } = error;
    AppLogger.error({ error: { message, stack } });
});

With this we caught this stack trace

Error: unable to get local issuer certificate
    at TLSSocket.<anonymous> (_tls_wrap.js:1105:38)
    at emitNone (events.js:106:13)
    at TLSSocket.emit (events.js:208:7)
    at TLSSocket._finishInit (_tls_wrap.js:639:8)
    at TLSWrap.ssl.onhandshakedone (_tls_wrap.js:469:38)

Still not enough to pinpoint the culprit but it's a start. So with some trial and error running once piece of code at a time we narrowed it down to one line

const ldapClient = ldap.createClient({ url: ldapAddress });

createClient has no error handler and wrapping it in try catch doesn't do it either!

I suspect other functionality of ldapjs is also not handling for all errors. We have other unknown TCP errors being caught globally and after this discover I'd bet it's ldapjs.

question

Most helpful comment

It should emit a connect event, so you could. It should also emit a connectError event if it fails. I can say I have never had an issue as long as I'm listening for the error event on the client, I believe it emits all the socket errors to the client. I could be wrong though.

If you want an example you can check out this wrapper I created a while ago for a company, the one thing to keep in mind is this wrapper generates a new Client every time thus meaning multiple calls to the functions creates multiple connections to the LDAP server instead of using the built-in queue in ldapjs. There was reliability issues with ldapjs queue and AD and the wrapper was built for approximately 300 users, so this wasn't an issue and solved the reliability issues.
https://github.com/tastypackets/node-ad-tools/blob/master/lib/ActiveDirectory.js

Another alternative is to consider ldapts, which someone on here created after getting frustrated with this lib not being maintained and no PRs being merged. I have not used it myself, but from scanning over the repo it looks like it's using promises by default and actively being updated.
https://github.com/ldapts/ldapts#readme

Currently I'm experimenting with building a Go auth service for AD, so I no longer need to connect NodeJS or any other applications directly to AD via LDAP. That is why I haven't tested out ldapts myself.

All 5 comments

createClient returns an event emitter, which emits error events. This event emitter is where you noramlly will see the TLS error you mentioned above, which commonly is caused by a self-signed certificate while using LDAPS.

Have you tried listening for the errors and handling them?

For example using your code:

const ldapClient = ldap.createClient({ url: ldapAddress });

// Console log errors
ldapClient.on('error', error => {
    console.error(error)
});

Also if you are not closing the client / connection you will likely see some network errors from timeouts. Hope that helps.

@tastypackets A follow-up. I'd like to wrap createClient into a Promise, in this case, how do I know it is successfully set up?

Edit

Is connect event a good way to go?

new Promise((resolve, reject) => {
  this.client = ldap.createClient({ url });
  this.client.on("error", (error) => {
    reject(error);
  });
  this.client.on("connect", () => {
    resolve();
  });
});

It should emit a connect event, so you could. It should also emit a connectError event if it fails. I can say I have never had an issue as long as I'm listening for the error event on the client, I believe it emits all the socket errors to the client. I could be wrong though.

If you want an example you can check out this wrapper I created a while ago for a company, the one thing to keep in mind is this wrapper generates a new Client every time thus meaning multiple calls to the functions creates multiple connections to the LDAP server instead of using the built-in queue in ldapjs. There was reliability issues with ldapjs queue and AD and the wrapper was built for approximately 300 users, so this wasn't an issue and solved the reliability issues.
https://github.com/tastypackets/node-ad-tools/blob/master/lib/ActiveDirectory.js

Another alternative is to consider ldapts, which someone on here created after getting frustrated with this lib not being maintained and no PRs being merged. I have not used it myself, but from scanning over the repo it looks like it's using promises by default and actively being updated.
https://github.com/ldapts/ldapts#readme

Currently I'm experimenting with building a Go auth service for AD, so I no longer need to connect NodeJS or any other applications directly to AD via LDAP. That is why I haven't tested out ldapts myself.

@tastypackets Thank you very much for the detailed information. I'm also considering to change to another well maintained library in the future. Thanks for the recommendation!

Closing as I think the question has been sufficiently answered. I would welcome any PRs that improve this developer experience.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

lmusat picture lmusat  路  5Comments

atiertant picture atiertant  路  6Comments

djholly123 picture djholly123  路  3Comments

ifroz picture ifroz  路  8Comments

UziTech picture UziTech  路  4Comments