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

Added functionality to close proxy. #679

Merged
merged 2 commits into from Aug 14, 2014
Merged

Added functionality to close proxy. #679

merged 2 commits into from Aug 14, 2014

Conversation

unilaterus
Copy link
Contributor

Added function to wrap closing the internal server. This function accepts an optional callback. The close will immediately cause proxy to stop listening on its current port but it will not close any pre-existing connections. Once all open connections are closed it will execute the given callback if one was supplied.

Closes issue: #675

Ensured server exists before closing.
Updated tests to use new close function.
Added documentation to README.
@jcrugzz
Copy link
Contributor

jcrugzz commented Aug 14, 2014

@unilaterus Thanks for the contribution! This looks good, the only thing I see potentially dangerous here is us nulling out the server object before the callback is executed (if one was passed in). I believe there should be a server.once('close') event that can be listened to to null the server after its fully closed or we could just wrap the callback and null the server before responding to the original callback that is passed in.

@unilaterus
Copy link
Contributor Author

@jcrugzz Nice catch there. I hadn't thought of that. I pushed up a revision to correct that issue. I chose to go with the wrap the callback option because I'm not entirely sure if listening for close event will work since I think the callback passed in replaces the standard close event.

jcrugzz added a commit that referenced this pull request Aug 14, 2014
@jcrugzz jcrugzz merged commit f92f7ae into http-party:master Aug 14, 2014
@jcrugzz
Copy link
Contributor

jcrugzz commented Aug 14, 2014

@unilaterus thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants