vesse / node-ldapauth-fork

Simple node.js module to authenticate against an LDAP server
Other
127 stars 79 forks source link

The 'error' event returned by ldapjs not handled correctly #25

Closed faacy closed 9 years ago

faacy commented 9 years ago

This is causing an unhandled 'error' event when ever the connection to the LDAP server is down.

nazrhyn commented 9 years ago

In version 1.0.0 of mcavage/node-ldapjs, it appears that the errors that were only being emitted but not passed to the callbacks are now also being passed to the callbacks. We've retracted our PR (vesse/node-ldapauth-fork#26) as this library does not appear to require any changes any longer. My PR vesse/passport-ldapauth#38 still remains as it is still needed.

In your library, you just need to update your dependencies to apply this fix.

vesse commented 9 years ago

Thanks, I was briefly looking over ldapjs changes just the other day and hopefully will have time on weekend to update the dependency.

nazrhyn commented 9 years ago

:+1: Awesome. I look forward to it.

chriseckhardt commented 9 years ago

Ran into this myself trying to get a sinopia installation running with an ldap plugin this week. Appreciate your work!

vesse commented 9 years ago

@nazrhyn At least DNS errors come still as uncaught, this test fails with

  1) LDAP authentication strategy with invalid LDAP url should handle the error:
     Uncaught Error: getaddrinfo ENOTFOUND
      at errnoException (dns.js:37:11)
      at Object.onanswer [as oncomplete] (dns.js:124:16)