Skip to content

Conversation

@dbraley
Copy link

@dbraley dbraley commented Feb 20, 2020

Resolves Issue #350.

I have an interest in using hotel to manage not just UI servers, but also RESTful services as well. In order to do that effectively, we need to be able to accept multiple REST methods, not just GET, and be able to serve paths beyond just the root.

Unfortunately, most clients do not deal with redirects on non-GET methods particularly well (though the change #353 by @christianwerz made to switch the redirect to a 307 does help some clients). Because of that, I've also added the ability to specify a --forward-by-proxy option when doing an add which will tell the router to proxy the request directly, rather than returning a redirect.

@dbraley
Copy link
Author

dbraley commented Feb 20, 2020

Oh, I should mention this branch is based on the nice work done by christianwerz here: #353

@dbraley dbraley requested a review from typicode February 20, 2020 16:42

// Make sure to send only one response
const send = once(() => {
let target = item.target + (item.target.endsWith('/') ? '' : '/') + path

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, it seems like ideally we would just not even parse the path out from the query -- whatever it is we'd just append the whole thing. but maybe that's too much work to change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's shorter codewise to do that, but I suspect a bit less clear. When we get here, we know the original target (which unfortunately excludes the query params), and the new host, so it seems clearest just to append the three together I think. Otherwise we need to take the entire raw url, overwrite the host (for the redirect case) and parse out (potentially) the service name (for both cases). But, because the hotel port is configurable, and there might be some additional mapping outside of hotel that would modify the url in an unpredictable way, it seems clearer and safer just to grab the parts we're highly confident are correct and append them into a url that should be correct.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I'm saying we should just make it so that when we get here we know the original target, including query params.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants