reverseproxy: Add handle_response blocks to reverse_proxy (#3710)#4021
Conversation
|
Sorry for what's maybe an elementary question, but can this be used to add some nuance to what Caddy does when the server being reverse proxied to is completely down? Caddy currently serves a blank page with status code 502 (Bad Gateway) in that scenario. I'd like to be able to serve a page with a readable message, rather than just a blank page. |
|
@frou I'm pretty sure you can use https://caddyserver.com/docs/caddyfile/directives/handle_errors for that. The difference is who emits the error. If it's Caddy itself that emits it, then you use |
8c2dde1 to
873c241
Compare
|
👋 I've been sorely trying to get a shadow of Gitlab Pages/nginx's |
|
Thanks for sparking the question @amyspark 😂 I realized that there was a piece lacking to make that work. The |
|
Close enough! but what I need is something like this: (i.e. rewrite the petition, send it again upstream, but keep the returned status code from the original response's) |
|
In that case you'll just need to make sure your upstream writes the correct status. I don't think it's a good idea to modify |
873c241 to
e75f92d
Compare
Docs for the new directive in caddyserver/caddy#4034. Also adding a bit in `handle_errors` mentioning that `reverse_proxy` doesn't trigger errors when a response has an "error" HTTP status; not sure how to word this clearly, cause `reverse_proxy` still can trigger errors if there's no upstream or whatever. We should probably add examples for that later, and augment this paragraph to mention `handle_response` once that's merged (caddyserver/caddy#4021)
b452815 to
8130097
Compare
There was a problem hiding this comment.
Just little things, I think. Thanks for this contribution. Sorry it took me a while to devote the focus to review it.
I wonder if we could keep the UnmarshalCaddyfile() method if we keep the Caddyfile token parsing in there, but then leave any extra setup of the handler (e.g. the hanlde_response routes) to a caller, such as parseCaddyfile that takes the Helper. (I could elaborate in Slack...)
2b295bb to
9acede3
Compare
274f4cc to
58a178e
Compare
mholt
left a comment
There was a problem hiding this comment.
Excellent work! It turned out just about as I had hoped. Yes it's a bummer the complete Caddyfile unmarshaling can't take place inside UnmarshalCaddyfile(), but I think maintaining that interface is important, and the workaround is pretty reasonable.
It might be worth adding to the godoc comment for UnmarshalCaddyfile() that an additional call to Finalize... is needed when a Helper is available.
reverseproxy: Add support for changing status code
We already have d.Err("transport already specified") in the reverse_proxy parsing code which covers this case
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
0c96f05 to
14eb708
Compare
|
Great work. Thanks for your patience and persistence with this. |
|
And thanks for @maxatome for doing the groundwork for this! 🎉 |
Supersedes #3712, closes #2920, #3707, #3828
Adds a new
handle_responsesubdirective toreverse_proxy, and also@<name>named response matchers to pair with it.Example which might serve files that are otherwise not accessible based on some response header:
The matcher is optional; having no matcher will match all responses. Any
handle_responsewithout a matcher will be sorted to the end just in case the user put them in a bad order.This also supports an optional status code on the same line, to change the status code of the response. If using this syntax, then a block may not be defined, and the matcher is required. (Why would you ever want to
We should also remember to document that a status code matcher like
4matches4xx, as per thecaddyhttp.StatusCodeMatches()function. It's useful.