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

Hooks for websocket/peer connections. #314

Closed
AdamMagaluk opened this issue May 31, 2016 · 1 comment
Closed

Hooks for websocket/peer connections. #314

AdamMagaluk opened this issue May 31, 2016 · 1 comment

Comments

@AdamMagaluk
Copy link
Collaborator

After digging into zetta and trying find a good/decent way to plug into the websocket requests I realized there really isn't one.

My end goal is to validate a peer request based on a query parameter added to the peering request.

With normal http requests to zetta we can plug into argo's request flow but does not work for upgrade requests. We handle the upgrade requests here: https://github.com/zettajs/zetta/blob/master/lib/http_server.js#L83-L87

The only way I was able to get it to work with zetta as is is by copying all event listeners on that upgrade event and wrap it in my own method to pre check the request.

Something like: Which of course is not ideal.

.use(function(server) {
  var originalListeners = server.httpServer.server.listeners('upgrade');
  server.httpServer.server.removeAllListeners('upgrade');
  originalListeners.forEach(function(listener) {
    server.httpServer.server.on('upgrade', function(request, socket) {
      console.log('on upgrade', request.url)
      listener(request, socket);
    });
  });
})

My proposal would be to add two methods the httpServer or possibly the runtime. That would like:

.use(function() {
  // Peer connection
  server.httpServer.onPeerConnect(function(request, socket, next) {
    // Handle auth logic, 
    next(); // Continue the peering process
  })

  // Regular data websocket connections
  server.httpServer.onWebsocketConnect(function(request, socket, next) {
    // Handle auth logic, 
    next(); // Continue the event_socket process
  })
})

@kevinswiber @mdobson Any thoughts?

@mdobson
Copy link
Contributor

mdobson commented May 31, 2016

+1 to the proposal. Seems to be consistent with other hooks we have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants