-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Remove path registry #885
Remove path registry #885
Conversation
Thanks for the great workaround write up! |
Hi @lpinca , is it possible to keep using different WS servers for different paths with |
@antoniobusrod see the above example. Can't you do that? |
hi @lpinca , I'm preparing a PR, so let's discuss there |
Thanks for the solution (Y) |
Can this be written somewhere in the docs? Lost a good chunk of time trying to figure out why one server worked but adding another blew up with an |
I am using the pattern. Can anybody explain to me what's meaning of websocket.emit("connection")? can it work together with websocket.on("connection", ws)? It seems that If I replace [ws.handleUpgrade] emit connection event.
events.js:167
throw er; // Unhandled 'error' event
^
Error [ERR_STREAM_WRITE_AFTER_END]: write after end
at writeAfterEnd (_stream_writable.js:243:12)
at Receiver.Writable.write (_stream_writable.js:292:5) |
Here is my code: function connection (socket, req) {
/*https://github.com/websockets/ws/pull/1099*/
if ('upgradeReq' in socket) {
req = socket.upgradeReq
}
const loc = url.parse(req.url, true).pathname
console.log(`[server:connection] on connection from ${loc}`)
if (!originIsAllowed(loc)) {
/*
* @desc handle unwanted request
*/
console.warn(`Visit from ${loc} is not allowed.`)
}
req.__handled = false
let res = new http.ServerResponse(req)
req._websocket = ws
// @see express-ws-router tricks
res.writeHead = function(status) {
if (status > 200) {
debug("rejected %s, status %s", loc, status)
socket.close()
req.__handled = true
}
}
console.log(`[server:connection] begin to handle using express router system ...`)
app.handle(req, res, function() {
if (!req.__handled){
debug("websocket connection %s not handled", loc)
socket.close()
}
})
/*
socket.on('message', function incoming(message){
console.log('received: %s', message)
})
socket.send("connection built successfully!")
*/
/*
* @desc register websocket routers
*/
}
ws.on("connection", connection)
/*
* @desc This techniques allow multiple websockets used, @see
*/
function onUpgrade(req, HTTPSocket, head) {
const loc = url.parse(req.url).pathname
switch(loc) {
case "/sim_world":
// WebSocketServer handle it
// console.log(`Receive websocket connection request ${stringify(req, null, 2)} from ${loc}`)
console.log(`[server:onUpgrade] receive websocket connection request ${req} from ${loc}`)
ws.handleUpgrade(req, HTTPSocket, head, (socket) => {
// debug("ws.handleUpgrade %s, %s", socket, req)
console.log("[ws.handleUpgrade] emit connection event.")
ws.emit('connection', socket)
})
break
default:
console.log(`Not recognized websocket request ${loc}`)
HTTPSocket.destroy()
}
} Here is my output [server:onUpgrade] receive websocket connection request [object Object] from /sim_world
[ws.handleUpgrade] emit connection event.
events.js:167
throw er; // Unhandled 'error' event
^
Error [ERR_STREAM_WRITE_AFTER_END]: write after end
at writeAfterEnd (_stream_writable.js:243:12)
at Receiver.Writable.write (_stream_writable.js:292:5)
at Socket.socketOnData (/Users/lei.wang1/Github/RandomMapGenerator/backend/node_modules/ws/lib/websocket.js:788:35)
at Socket.emit (events.js:187:15)
at Socket.EventEmitter.emit (domain.js:442:20)
at addChunk (_stream_readable.js:280:12)
at readableAddChunk (_stream_readable.js:265:11)
at Socket.Readable.push (_stream_readable.js:220:10)
at TCP.onread (net.js:638:20)
Emitted 'error' event at:
at Receiver.receiverOnError (/Users/lei.wang1/Github/RandomMapGenerator/backend/node_modules/ws/lib/websocket.js:698:13)
at Receiver.emit (events.js:182:13)
at Receiver.EventEmitter.emit (domain.js:442:20)
at writeAfterEnd (_stream_writable.js:245:10)
at Receiver.Writable.write (_stream_writable.js:292:5)
[... lines matching original stack trace ...]
at TCP.onread (net.js:638:20)
|
Check my working code. var server = http.createServer().listen(300);
}); |
Yeah, thanks! That's working for me likewise. else {
socket.destroy();
} |
Summary: ## Context Closes #674. There is a security vulnerability with the current version of ws, that requires it to be upgraded to 5.2.3. I upgraded it to 7.5.1 while I was it. ## In this diff A few of the API changes that needed to be fixed: - `upgradeReq` was removed from the `WebSocket` object, the fix being to take the URL from the `request` param instead. - `onError` now correctly passes an `ErrorEvent` instead of an `Error` object - [can't attach more than one websocket per http server](websockets/ws#885 (comment)): this is the big one. On metro we use multiple websocket servers to do different things: HMR, JS debugger and generally talking back and forth with the dev server. The trick is to basically have an on `upgrade` handler that does the manual routing between the various servers. This is a breaking change for anybody who was using the Metro api to then attach a web socket server. **The new recommended way to attach a websocket server if need be** is to pass a `websocketEndpoints` argument to the Metro runtime options in `Metro.runServer` that looks like `{ [path: string] : <an instance of WebSocketServer created with the "noServer: true" option> }`. Reviewed By: motiz88 Differential Revision: D30012392 fbshipit-source-id: e69503f1a4da2ee417e7dcc9d42680e4dc0415d8
The
_webSocketPaths
object is added to the underlying HTTP sever of aWebSocketServer
when thepath
option is specified. It is used to store the path of eachWebSocketServer
using the same underlying HTTP server in order to avoid path conflicts.This allows to use multiple WebSocket severs (one per path) and only one shared HTTP server.
The problem here is that each
WebSocketServer
adds a new listener for theupgrade
event on the HTTP server and when that event is emitted,handleUpgrade
is called on all servers.Now, using the above example, a connection to
ws://example.com/foo
would first be established onwss1
and then closed bywss2
as they both write on the same socket.Originally this worked because path validation did not close the connection on mismatch.
Passing the same HTTP server in the constructor when instantiating multiple WebSocket servers is not a good idea imo and should be avoided. I can't think of a use case where it makes sense.
In order to have multiple WebSocket servers and only one shared HTTP server, a developer could use something like this: