Skip to content
This repository has been archived by the owner on Mar 29, 2022. It is now read-only.

Added callback on close #32

Merged
merged 3 commits into from
Dec 6, 2015
Merged

Added callback on close #32

merged 3 commits into from
Dec 6, 2015

Conversation

jamiemchale
Copy link
Contributor

Bump ws version to 0.8.1
Added callback to close
Altered README to reflect changes

Closes #31

Bump ws version to 0.8.1
Added callback to close
Altered README to reflect changes
@urish
Copy link
Owner

urish commented Dec 4, 2015

Thanks!

What's the use case?

@urish
Copy link
Owner

urish commented Dec 4, 2015

N/m, just saw the other issue that you opened with the explanation. Can you please add a test case as well?

@jamiemchale
Copy link
Contributor Author

I have a bunch of Mocha tests that run in sequence. I was closing the server on the after() in each spec, and loading it in the before() in others. Sometimes the close() on the server didn't complete by the time the before() of the next script ran, so a port in use error was raised.

@jamiemchale
Copy link
Contributor Author

Sure, I can have a go. This is one of my first open-source contributions!

@urish
Copy link
Owner

urish commented Dec 4, 2015

That's wonderful, keep them coming :)

If you need any assistance, feel free to ask

@jamiemchale
Copy link
Contributor Author

The close test was failing on Node v0.10 - it seems that the server wasn't loading in time for the close callback. I'm not sure if there is a tidier or better way of testing this?

@urish
Copy link
Owner

urish commented Dec 6, 2015

Thanks! Did you try to use setImmediate instead of setTimeout?

urish added a commit that referenced this pull request Dec 6, 2015
@urish urish merged commit a3a2e04 into urish:master Dec 6, 2015
@jamiemchale jamiemchale deleted the close-callback branch December 6, 2015 13:46
@urish
Copy link
Owner

urish commented Dec 13, 2015

Released as 0.5.1

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.

Add ws close callback
2 participants