rlidwka / sinopia-ldap

LDAP auth plugin for sinopia
18 stars 13 forks source link

Create a new LDAP client for each request and also close the connection. #9

Closed olesku closed 9 years ago

olesku commented 9 years ago

After running for 20 minutes or so the plugin stopped working leaving us with this error:

err":{"message":"write after end","name":"Error","stack":"Error: write after end\n at writeAfterEnd (_stream_writable.js:167:12)\n at Socket.Writable.write (_stream_writable.js:214:5)\n at Socket.write (net.js:625:40)\n at Client._send (/usr/lib/node_modules/sinopia-ldap/node_modules/ldapjs/lib/client/client.js:914:17)\n at Client.search (/usr/lib/node_modules/sinopia-ldap/node_modules/ldapjs/lib/client/client.js:678:15)\n at /usr/lib/node_modules/sinopia-ldap/node_modules/ldapauth-fork/lib/ldapauth.js:197:23\n at LdapAuth._adminBind (/usr/lib/node_modules/sinopia-ldap/node_modules/ldapauth-fork/lib/ldapauth.js:165:12)\n at LdapAuth._search (/usr/lib/node_modules/sinopia-ldap/node_modules/ldapauth-fork/lib/ldapauth.js:193:8)\n at LdapAuth._findUser (/usr/lib/node_modules/sinopia-ldap/node_modules/ldapauth-fork/lib/ldapauth.js:242:8)\n at LdapAuth.authenticate (/usr/lib/node_modules/sinopia-ldap/node_modules/ldapauth-fork/lib/ldapauth.js:298:8)"},"msg":"LDAP error @{err}","time":"2015-04-29T09:01:05.022Z","v":0}

This problem goes away if you create a new ldap client instance for each query. I've created a patch that solves this issue for us, pull if you like :)

rlidwka commented 9 years ago

I'm happy to merge it, but just for completeness sake I'd like to repeat my question in a similar PR (https://github.com/rlidwka/sinopia-ldap/pull/6#issuecomment-67781470):

Is it possible to catch this particular error and re-connect to LDAP server when it happens instead of creating a separate request every time? Would it even make sense to maintain a permanent connection to LDAP server?

rlidwka commented 9 years ago

Merged and published as sinopia-ldap@0.5.0. Thanks!