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

Created clientError event (fixes #609) #615

Merged
merged 7 commits into from May 2, 2017
Merged

Created clientError event (fixes #609) #615

merged 7 commits into from May 2, 2017

Conversation

hicaro
Copy link
Contributor

@hicaro hicaro commented Mar 4, 2017

This change creates a new event called clientError which can be catch in the broker code by:

// fired when client encounters an error
server.on('clientError', function(error, client) {
    // handle the output message   
});

This way the user will not have unexpected messages coming from that.logger.warn(err);.

Thank you for the awesome library!

@mcollina
Copy link
Collaborator

mcollina commented Mar 4, 2017

Would you mind adding a unit test for this feature?

@hicaro
Copy link
Contributor Author

hicaro commented Mar 5, 2017

@mcollina I need a little help.

I am having a hard time to reproduce the error in the tests. I will tell you why.

Basically, every time I had this error happening, it was as a result of the socket on the client side hanging up. It would cause a Error: read ECONNRESET on the client instance in the server.

However, I've tried it in many different ways to simulate this error. client.stream.end() and client.stream.destroy() were among them.

I even tried:

 var stream = net.createConnection(instance.opts.port);
 stream.once('connect', function(){
     stream.destroy();
  });

All of them seem to close the connection smoothly which does not cause any error.

On my machine, not using the testing script, if I create two different processes, one for the server and another one for the client, and then use kill -SIGINT <pid> I can create the error on the server.

What do you think I should do?

@mcollina
Copy link
Collaborator

mcollina commented Mar 5, 2017

you can either spawn a separate process using 'child_process' or just emit('error') on the client.

@hicaro
Copy link
Contributor Author

hicaro commented Mar 11, 2017

@mcollina Could you rerun the test for Node.js:4 build? It failed not due to my code.

It timed out on mosca.persistence.LevelUp offline packets should delete the offline packets once streamed.

@mcollina mcollina merged commit 471ed1b into moscajs:master May 2, 2017
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.

None yet

2 participants