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

Allow custom responses in onRequest #274

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cactysman
Copy link

Custom responses in onRequest

Note:

This PR is based on commits in #273. Technically this could be an individual change, but it would be reasonable to merge #273 first to avoid all sorts of problems before taking on this PR.

Current state of the art

The onRequest handler introduced by @jchip offers a way to access the request and response objects before proxying is being handled¹. This allows modification of the request that's being sent to the eventual target. The target can also be changed entirely.

The problem

Sending a custom response instead of just modifying the request, at the moment, is not quite possible without redbird still attempting to proxy the request somewhere, which results in it either overwriting your response with a 404 Not Found response or it throwing an exception because it attempted to send a header after the response has already been (partially) sent.

What this PR intends to do

With this PR draft I propose changes that make sending custom responses and proper handling for them in the onRequest possible.

Upon returning the (mutated) response object in the onRequest handler, redbird will know that the request should skip proxying and instead will be replied to with what the handler fed to the response object.

The logic for that, a test for it and a brief mention in the README have already been implemented in commit 5395e51.

What's still missing / has to be done

The current logic still requires resolvers to supply a URL to proxy the request to [see ¹], because that's how the internals determine a target to process and attempt to send the request to.

The test (and eventual use of this feature) would, for now, just submit '0.0.0.0' as the proxy target, which allows the logic to reach the code that handles custom responses in the first place.

Example code:

const redbirdOptions = {
  // ...
  resolvers: [
    (req, res) => {
      if(shouldDoRedirect(req)) return {
        url: '0.0.0.0', // still required but kinda uncool
        opts: {
          onRequest: (req, res, target) => {
            res.setHeader('Location', 'https://github.com')
            res.statusCode = 301
            return res
          }
        }
      }
    }
  ]
}

I am aware that that's not ideal and we should find a better and more user friendly way to handle that. That would require bigger a few more changes though.

New possibilities with these changes:

  • Generic "bouncer" responses for whatever kind of reason instead of stuffing logic into the notFound handler
  • Host/path/header/whatever-based redirects (adding a header and setting the status to 3XX)

Tasks

  • Handle onRequest handlers that send a custom response
  • Find a clean way to handle custom responses without submitting trash like 0.0.0.0 as proxy target

I hope handling redirects in a reverse proxy module does not seem out-of-scope to you folks. Personally I think it makes sense for stuff like old domains or deprecated routes that, using this and some sort of user-specific config, can just be redirected instead of needing to proxy it to some kind of separate bouncer server instance or wrapping more logic around redbird.

Since we already have onRequest logic, making these changes seems like a reasonable addition to me.

If something is missing or feels off, don't hesitate to discuss it!

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

Successfully merging this pull request may close these issues.

1 participant