Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.disconnect() should provide a callback #9

Open
chadxz opened this issue Jan 8, 2015 · 1 comment
Open

.disconnect() should provide a callback #9

chadxz opened this issue Jan 8, 2015 · 1 comment

Comments

@chadxz
Copy link

chadxz commented Jan 8, 2015

when calling .disconnect(), you're performing a socket.end() but providing no callback for when the socket is completely closed. You could do this by listening to the 'close' event, i.e.

AsteriskAmi.prototype.disconnect = function(callback){
  this.reconnect = false;
  this.socket.on('close', function () {
    callback();
  });
  this.socket.end(this.generateSocketData({Action: 'Logoff'}));
}

You shouldn't need to remove the close listener on the socket because the socket should be destroyed when the connection is closed.

Sorry I'm lazy and didn't do a PR because this was just a tangential thought while reviewing someone's code.

@chadxz
Copy link
Author

chadxz commented Jan 8, 2015

I see now that you expose an "ami_socket_close" event when the underlying socket is closed, so I suppose a consumer of this library could use that, i.e.

function closeAmi(callback) {
  ami.on('ami_socket_close', function () {
    callback();
  });
  ami.disconnect();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant