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.

Sign up for GitHub

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

Jump to bottom

Provide http server port directly from _server #1294

Merged
merged 10 commits into from
Feb 5, 2018
Merged

Provide http server port directly from _server #1294

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8af5c88
provide http server port directly from _server
Feb 4, 2018
3e955f6
fix from review
Feb 5, 2018
3b4f124
write test to support node v4
Feb 5, 2018
b4ffa36
refactor code follow code review
Feb 5, 2018
66502a0
test start server mode noServer cann't call address()
Feb 5, 2018
a0b62e9
refactor code follow by code review
Feb 5, 2018
df4ff2e
Update ws.md
Feb 5, 2018
34b3fb6
use regex in test, and swap arg in assertion
Feb 5, 2018
0ef1ae1
Fix nits
lpinca Feb 5, 2018
cc9309e
Fix nits
lpinca Feb 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/websocket-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,14 @@ class WebSocketServer extends EventEmitter {

cb(ws);
}

/**
* Upgrade the connection to WebSocket.
* @public
*/
port () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of thoughts:

  1. It's probably better to call this address and return this._server.address(). This will handle the case where the HTTP server is closed.
  2. WebSocketServer could be initialized in noServer mode. In this case this._server is null. What should we return? Returning null doesn't make sense as it's possible that the external server is actually bound and listening for connections. Throwing an error is probably the best option.

Copy link
Contributor Author

@tnuanchuay tnuanchuay Feb 5, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

understood. Thank for the guide. :)

return this._server.address().port;
}
}

module.exports = WebSocketServer;
Expand Down