Close does not return errors

[vizjerai]

This is probably more of a bug for ldapjs than ldapauth-fork but currently they are in a middle of a large refactor and may have fixed this issue. But I wanted to leave a note about it here in case some other people run into this problem.

ldapjs 0.7.1 returns before sockets are closed and remove events before sockets are closed.

If i set the connectTimeout to 1 and spam the server eventually it'll crash returning Error: This socket is closed. or something close to that.

My work around requires listening to the _userClient.socket events error or close and using when for a promise so it doesn't return before it should because the sockets aren't done closing when unbind's callback is called.

Sorry this is in coffeescript

    self = this
    return whenjs()
      .then ->
        if self._adminBound
          return whenjs.promise (resolve, reject) ->
            self._adminClient.socket.on 'error', (err) ->
              reject(err)
            self._adminClient.socket.on 'close', ->
              resolve()
            self._adminClient.unbind()
      .then ->
        return whenjs.promise (resolve, reject) ->
          self._userClient.socket.on 'close', ->
            resolve()
          self._userClient.socket.on 'error', (err) ->
            reject(err)
          self._userClient.unbind()
      .then ->
        callback()
      .otherwise (err) ->
        callback(err)

[vesse]

OK, thanks for the info. The listener workaround could be added to this library. If you're interested please go ahead and send a pull request (without promises though), otherwise I'll try to take a look myself someday.

[vizjerai]

Here is the code without the promises and it should work just as well as the previous version.

Go ahead and make any adjustments to the code.

class LdapAuth
  close: (callback) ->
    self = this
    this._adminClient.socket.on 'error', (err) ->
      callback(err)
    this._adminClient.socket.on 'close', ->
      self._userClient.unbind()

    this._userClient.socket.on 'error', (err) ->
      callback(err)
    this._userClient.socket.on 'close', ->
      callback()

    if this._adminBound
      this._adminClient.unbind()
    else
      this._userClient.unbind()

Untested translation to JS

LdapAuth.prototype.close = function(callback) {
  this._adminClient.socket.on('error', function(err) {
    callback(err);
  });
  this._adminClient.socket.on('close', function() {
    this._userClient.unbind();
  });

  this._userClient.socket.on('error', function(err) {
    callback(err);
  });
  this._userClient.socket.on('close', function() {
    callback();
  });

  if (this._adminBound) {
    this._adminClient.unbind();
  } else {
    this._userClient.unbind();
  }
}

[vesse]

Btw, did you try with ldaps? There are some workarounds in ldapjs for TLS related problems, wonder if that approach would work with them.