fastify / fastify-http-proxy

Proxy your http requests to another server, with hooks.
MIT License
338 stars 91 forks source link

Allow for dynamic websocket upstreams with `replyOptions.getUpstream` #333

Closed jcbain closed 3 months ago

jcbain commented 8 months ago

Prerequisites

🚀 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 8 months ago

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

jcbain commented 8 months ago

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!

jcbain commented 6 months ago

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.

mcollina commented 6 months ago

a PR would be great.