fastify/fastify-http-proxy

Allow for dynamic websocket upstreams with `replyOptions.getUpstream`

jcbain opened this issue · 4 comments

jcbain commented

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

It would be nice to allow for dynamic upstreams when forwarding on websocket upgraded requests much in the same way that you can specify a getUpstream function to your replyOptions to pass some logic to determine an http upstream. However, leaving the wsUpstream property as an empty string results in the following error:

Error: upstream must be specified

It would be nice if this could be avoided when a getUpstream is supplied for an empty wsUpstream option.

Motivation

No response

Example

For example, it would be nice to register a websocket proxying mechanism like so

fastify.register(require('@fastify/http-proxy'), {
   websocket: true,
   wsUpstream: '',
   replyOptions: {
      getUpstream: function (req) {
          // some logic to determine the wsUpstream
      }
   }
})
mcollina commented

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

  • 👍1
jcbain commented

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

Yeah, I can do that!

  • 🚀1
jcbain commented

Sorry for the silence. After further review, it appears that this functionality does work with dynamic websocket proxying endpoints but only for the upstream option and not the wsUpstream. I'm happy to create a PR for allowing dynamic endpoints using the similar pattern of an empty upstream for an empty wsUpstream for perhaps more clarity of usage.

I'm also, personally, okay with this being closed since there is a workaround.

  • 👍1
mcollina commented

a PR would be great.

  • 👍1