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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for dynamic websocket upstreams with replyOptions.getUpstream #333

Open
2 tasks done
jcbain opened this issue Feb 6, 2024 · 4 comments
Open
2 tasks done

Allow for dynamic websocket upstreams with replyOptions.getUpstream #333

jcbain opened this issue Feb 6, 2024 · 4 comments

Comments

@jcbain
Copy link

jcbain commented Feb 6, 2024

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
Copy link
Member

mcollina commented Feb 7, 2024

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

@jcbain
Copy link
Author

jcbain commented Feb 7, 2024

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
Copy link
Author

jcbain commented Mar 22, 2024

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
Copy link
Member

a PR would be great.

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

2 participants