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

WebSocket support..? #2

Closed
simshanith opened this issue Sep 10, 2014 · 9 comments
Closed

WebSocket support..? #2

simshanith opened this issue Sep 10, 2014 · 9 comments

Comments

@simshanith
Copy link
Contributor

The TODO checkbox is checked, but I don't see the expected code in index.js from looking at the nodejitsu/node-http-proxy docs...

//
// Listen to the `upgrade` event and proxy the
// WebSocket requests as well.
//
proxyServer.on('upgrade', function (req, socket, head) {
  proxy.ws(req, socket, head);
});
@shakyShane
Copy link
Owner

That TODO item indicates that the Browser Sync socket is correctly proxied, It's not clear my bad.

@buunguyen
Copy link

@shakyShane @simshanith I look at this and BrowserSync/browser-sync#71 (which was closed) but am still not clear on whether I can use BrowserSync with an app that uses socket.io. And there's nothing in the CHANGELOG of this or BrowserSync mentions about it.

Basically I use node-http-proxy to proxy API and SocketIO requests from BrowserSync to the backend app but even though I precisely followed the doc of node-http-proxy, I couldn't make SocketIO requests work (API requests work though). I'm still trying to make it work, assuming I do some configuration wrong, but it would be nice if you could tell me whether it's possible with BrowserSync at all. Thanks!

Updated
Here's what I'm trying:

var bsync = require('browser-sync')
   , proxyFactory = require('http-proxy')

var httpProxy = proxyFactory.createProxyServer({
  target: {host: 'localhost', port: 3001}, 
  ws: true // also fails if `false` 
})
... 
bsync({
    startPath: '/',
    browser: 'default',
    port: 3000,
    ui: { port: 9091},
    server: {
      baseDir: 'public',
      middleware: [
        function proxyApiRequests(req, res, next) {
          if (/^\/api/i.test(req.url)) httpProxy.web(req, res)
          else next()
        }
      ]
    }
  }, function (err, bs) {
    if (err) throw err
    bs.server.on('upgrade', function (req, socket, head) {
      if (/^\/api/i.test(req.url)) {
        httpProxy.ws(req, socket, head)
      }
    })
  })

@simshanith
Copy link
Contributor Author

@buunguyen By default, SocketIO requests go to "/socket.io/", so wouldn't get proxied by your custom middleware.

Does your "backend server" also serve your public directory? If so, you should probably use BrowserSync's proxy mode.

Current BrowserSync proxy supports Socket.IO over XHR long-polling. With the snippet pasted in my original post opening this issue (#2), it could potentially proxy WebSockets as well.

@buunguyen
Copy link

@simshanith thank you for your response.

I've configured socket.io to use a custom path /api/. In my snippet, you can see I perform the WS upgrade on the server instance started by BrowserSync:

 bs.server.on('upgrade', function (req, socket, head) {
   if (/^\/api/i.test(req.url)) {
     httpProxy.ws(req, socket, head)
   }
})

The setup is essentially this:

  • Express-based API + socket.io (backend) running on port 3001
  • Static assets from public (frontend) are served by BrowserSync at port 3000
  • Backend and frontend are completely separate, one doesn't "contain" another
  • JS files (in public) make same-domain API and WS requests to port 3000
    • Hence, I want to proxy these requests from port 3000 to 3001 using BrowserSync middleware option and node-http-proxy

With the provided snippet, API requests are proxied just fine. So are socket.io long-polling XHR requests, but not WS connections.

Do you think it's possible to make WS work given the above setup?

@simshanith
Copy link
Contributor Author

@buunguyen apologies for the significant delay in response.

After returning to the issue, no, the above setup will not work.

The above setup listens for the upgrade on the proxy server, not the foxy app server. The foxy server is where you're attaching the event, but it actually happens on the foxy-internal proxy server to which the foxy app forwards requests.

I'll see if I can get it working and draft a PR here.

@simshanith
Copy link
Contributor Author

Ok, so a revision of the original synopsis. Looks like the foxy server is where the event should be attached, but you still weren't using the internal proxy instance.

simshanith added a commit to simshanith/foxy that referenced this issue Apr 25, 2015
Expose `upgrade` proxy handler on foxy `app` instance as `handleUpgrade`
Attach `upgrade` listener in CLI server creation.

Issue shakyShane#2
simshanith added a commit to simshanith/foxy that referenced this issue Apr 25, 2015
Expose `upgrade` proxy handler on foxy `app` instance as `handleUpgrade`
Attach `upgrade` listener in CLI server creation.

Issue shakyShane#2
@shakyShane
Copy link
Owner

Fixed in latest :)

@buunguyen
Copy link

@simshanith 👍 thanks a lot for working on this.

@shakyShane is this in browser-sync yet? I've just pulled 2.7.0, but still have the same error ("Connection closed before receiving a handshake response"). My usage is:

gulp.task('sync', function (cb) {
  bsync.init({
    ...
    server: {
      middleware: [
        function (req, res, next) {
          if (/^\/api/.test(req.url)) proxy.web(req, res, {target: 'http://localhost:3000'});
          else next();
        }
      ]
    }
  });
})

Do I need to perform WS upgrade manually? I suppose not by looking at the changes, but I'm not very sure.

@alp82
Copy link

alp82 commented May 16, 2015

As i am not sure if this is a foxy or BrowserSync problem, i opened an issue in the other project as well.:
BrowserSync/browser-sync#625

I apologize if that complicates things.

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

No branches or pull requests

4 participants