Skip to content
This repository has been archived by the owner on Feb 11, 2020. It is now read-only.

Don't emit clientDisconnected event when force close client. #79

Merged
merged 1 commit into from
Dec 15, 2013

Conversation

mcollina
Copy link
Collaborator

Hi, I got a problem. When I force close a client use CTRL+C , Mosca not emit clientDisconnected event. Here is the code:

  • Server
var server = new mosca.Server(options);
server.on('clientDisconnected', function () {
    console.log('client gone');
})
  • Client
var client = mqtt.createClient();
// Works fine, mosca got the clientDisconnected event.
client.end();
var client = mqtt.createClient();
// Use CTRL+C kill the process and mosca not got the clientDisconnected event.

@mcollina
Copy link
Collaborator

The 'clientDisconnected' event is received one a client sends the DISCONNECT message.
It's a clean disconnect, as the connection is closed by the server. However, there is no event fired when the socket dies.

I think we need another event, 'clientErrored', or an error parameter in the 'clientDisconnected' event.

Anyone else has faced the same problem?

@haio
Copy link
Author

haio commented Dec 13, 2013

Yes, a event when client abnormal disconnected would be very useful, client connection state is very important in our use case.

@haio
Copy link
Author

haio commented Dec 13, 2013

An error parameter in the clientDisconnected event should be more graceful.

@mcollina
Copy link
Collaborator

I checked better, and it's indeed a bug introduced by the latest fix for persistance messages by @ldstein.
I will try to propose a fix asap.

This problem occurs because in case of error the cleanUp() function in https://github.com/mcollina/mosca/blob/master/lib/client.js#L515-L529 it is not called.

@haio
Copy link
Author

haio commented Dec 13, 2013

Great! Thanks a lot.

@mcollina
Copy link
Collaborator

This should be fixed by the latest commit on this branch. Can you please confirm everything is working as expected?

@haio
Copy link
Author

haio commented Dec 15, 2013

Yes, I just checked that commit, everything work as expected.

mcollina added a commit that referenced this pull request Dec 15, 2013
Don't emit clientDisconnected event when force close client.
@mcollina mcollina merged commit dfd58ba into master Dec 15, 2013
@mcollina mcollina deleted the fix-79 branch December 15, 2013 19:01
@mcollina
Copy link
Collaborator

Released as 0.14.4.

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

Successfully merging this pull request may close these issues.

2 participants