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

Double emission of error in failed bind

Open jsumners opened this issue 5 years ago • 2 comments

https://github.com/ldapjs/node-ldapjs/blob/8384a4736ff03bc6a09da5ee0e12d69eae02d277/lib/client/client.js#L157-L159

When the client fails to bind, e.g. due to invalid credentials, it generates the same error event twice. We should figure out a way to emit this only once.

jsumners avatar Jan 12 '20 15:01 jsumners

That's what I was talking about in https://github.com/ldapjs/node-ldapjs/pull/574#issuecomment-541458835 . I don't know of any other packages that uses event emitters and callbacks. I'm thinking the easiest way to solve it would be to eliminate callbacks and just use the event emitter.

UziTech avatar Jan 12 '20 15:01 UziTech

Yeah, I thought I remembered a discussion. I agree at least for the connection events. But in this specific case, I think the ultimate solution is to remove the automatic bind on client creation. But I'd rather have that in a v3 release because it is a very major change.

jsumners avatar Jan 12 '20 17:01 jsumners

👋

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