Add some test and fix a rare network condition that caused a crash #19
Conversation
|
@sarfata this looks good to me. You have a commented-out test that I also couldn't get to work with the fake timer, but worked fine with a setTimeout. |
This fixes a bug because I would manually start a recovery but the socket would also fire a socket-error event which would cause another attempt to recover. Very quickly, each attempt would multiply and it got really really messy...
* A socket send error should not throw an error right away - we will wait for the socket to send the error itself. * De-commented the auto-reconnect test. Use setTimeout() instead of sinon.tick()
|
Thanks @emgee3 for the feedback! @tmeasday I would like to publish this as 0.3.3 - is this ok with you? Can you give me npm publish-right (sarfata/[email protected])? Or do you want to do it? |
|
Done! Sorry about the silence at my end. @emgee3 - would you like to be added as an npm package owner too? PS if either of you are feeling adventurous, I'm fairly sure that nodejs/node-v0.x-archive#5557 (comment) is affecting the library via a problem with ws. I know it's affecting Meteorite. |
|
@tmeasday I guess that would be a good idea. [email protected] on npm |
|
@emgee3 -- it can't find you. Do you have a username? On Tuesday, 28 May 2013 at 1:30 PM, emgee3 wrote:
|
|
On NPM it's just emgee |
|
Cool, sorted |
Done this to follow ws error-handling best practices (see https://github.com/einaros/ws/blob/master/README.md).
Add some test and fix a rare network condition that caused a crash
I am opening this branch to fix #18. In the process, I have added some test to reproduce my bug and added some very simple tests on basic features.
This will need to be expanded but it sets the base for a test infrastructure.
This is not quite ready to be merged yet, I need to double-check that my fix works and does not have any side-effect but I am opening this pull-request to discuss it with you guys ( ping @tmeasday / @emgee3).
FYI - I am using rewire and sinon (new dependencies) to mock WebSocket.