serverless-offline icon indicating copy to clipboard operation
serverless-offline copied to clipboard

Allow binding `websocketPort` to `port`

Open cnuss opened this issue 1 year ago • 1 comments

Description

In the following serverless.yml configuration:

custom:
  serverless-offline:
    port: 3000
    websocketPort: 3000

The following error occurs at startup:

✖ Unexpected error while starting serverless-offline server on port 3000: { Error: listen EADDRINUSE: address already in use ::1:3000
      at Server.setupListenHandle [as _listen2] (node:net:1898:16)
      at listenInCluster (node:net:1946:12)
      at GetAddrInfoReqWrap.doListen [as callback] (node:net:2116:7)
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:111:8)
    code: 'EADDRINUSE',
    errno: -98,
    syscall: 'listen',
    address: '::1',
    port: 3000 }

Motivation and Context

In a lot of HTTP + Websocket services, the Websocket and HTTP server can share the same port since they are on different protocols.

This PR:

  • Introduces a AbstractHttpServer superclass which sets up commonalities between the various HttpServer classes most likely due to copy-paste of them over time
  • Has helper functions so the HttpServer classes are more uniform
  • Uses the Lambda class to maintain a map of which HttpServer is on which port
  • Uses the Map() to have a single HttpServer created once and is the sole source of truth for all HttpServer instances
  • Updated HttpServer in the websocket class override superclass functions to bind on the correct server.

Finally:

  • In the event the websocketPort is set to the same value as port, the @connections API is hosted on the HttpServer created by the Lambda class. Less is more, right?!

How Has This Been Tested?

  • All existing tests work after the refactor
  • The websocket-oneway and websocket-twoway integration tests were copied with the new shared port configuration and also work.

cnuss avatar Jun 18 '24 13:06 cnuss

hi @dherault and @DorianMazur could you take a look at this PR? Thanks!

cnuss avatar Jun 18 '24 13:06 cnuss

Thanks for the PR. I like the idea of the PR, but I feel it introduces too many changes at once. If you'd be willing to reduce its size somehow, I'd be more enclined to merge it.

dherault avatar Aug 17 '24 19:08 dherault

Closing. I'm developing a framework outside of serverless and no longer need this

cnuss avatar Aug 28 '24 11:08 cnuss