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

WebsocketServerTransport bind does not resolve when given a WebsocketServer which leverages an listening http.Server instance #237

Open
viglucci opened this issue Aug 7, 2022 · 0 comments
Labels
1.0 Pullrequests & issues related to the Typescript rewrite and 1.0 release enhancement Suggests, requests, or implements a feature or enhancement

Comments

@viglucci
Copy link
Member

viglucci commented Aug 7, 2022

As described below, when calling bind on a WebsocketServerTransport created with a WebsocketServer that uses a listening http.Server, the bind call never resolves.

------- Original comment body:

Hi @viglucci, I'm finally getting around to integrating RSocket 1.0 in a greenfield project, so I'll be adding to this thread as and when I find anything worth sharing.

So far loving the simple integration with RxJS, particularly on the client side this has reduced our RSocket setup code by at least 10x. Great work on that.

I had one small hiccup that I've had to hack around on the server side. We're running an Express server with some "normal" HTTP endpoints, and we want RSocket to only listen on a /subscriptions URL. The way we're doing this is:

We have an existing httpServer: http.Server, and my initial instinct was to instantiate the transport like this:

  const websocketServer = new WebSocket.Server({
    server: httpServer,
    path: '/subscriptions',
  })
  const transport = new WebsocketServerTransport({
    wsCreator: () => websocketServer,
  })
  const rsocketServer = new RSocketServer({
    transport,
    // ...
  })

We were calling this as follows in our server setup code:

  await listen() // () => new Promise<void>((resolve) => httpServer.listen(env.port, resolve))
  await rsocketServer.bind()

This didn't work though, as rsocketServer.bind() never resolves.

I noticed that WebsocketServerTransport#connectServer uses the listening event to resolve the server:

websocketServer.addListener("listening", () => resolve(websocketServer));

Manually adding websocketServer.addListener('listening', () => console.log('I am listening')), and also adding 'connection' listeners showed me that the WebSocket server was listening correctly.

Instantiating the WebSocket.Server inside the wsCreator callback did not help either. WebSocket.Server just binds to the HTTP server's listening event (code), so this never emits if the server is already listening.

I can fix this for now by instantiating the RSocket server before listening on the HTTP server, eg.:

  const rsocketStarted = rsocketServer.bind()
  await listen()
  await rsocketStarted

...but it's a bit confusing that the order makes a difference.

I can see why we'd want to await the listening event inside connectServer, but is there potentially some way of checking whether the WebSocket.Server is already listening and immediately resolving if so?

Anyway not a blocker at all, just a papercut :)

Will report on more as I come across it & once again thanks a lot for working on this!

Originally posted by @lachenmayer in #158 (comment)

@viglucci viglucci added 1.0 Pullrequests & issues related to the Typescript rewrite and 1.0 release enhancement Suggests, requests, or implements a feature or enhancement labels Aug 7, 2022
@viglucci viglucci changed the title WebsocketServerTransport bind does not resolve when given a WebsocketServer which leverages an listing http.Server instance WebsocketServerTransport bind does not resolve when given a WebsocketServer which leverages an listening http.Server instance Aug 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 Pullrequests & issues related to the Typescript rewrite and 1.0 release enhancement Suggests, requests, or implements a feature or enhancement
Projects
None yet
Development

No branches or pull requests

1 participant