Caddyfile handle_errors optional status code argument

[francislavoie]

The handle_errors directive is useful for implementing custom error pages and such.

Currently, its ergonomics are not great, because matching errors by status code needs to be done via a handle with an expression matcher (see examples in the docs).

I suggest that we add an optional status code argument (or variadic multiple status codes) to handle_errors as a shortcut for wrapping the routes in a status code matcher. For example:

handle_errors 404 410 {
	<directives...>
}

This would effectively be the same as:

handle_errors {
	@codes expression `{http.error.status_code} in [404, 410]`
	handle @codes {
		<directives...>		
	}
}

Using handle_errors this way multiple times should simply append more routes. A handle_errors with no arguments (no matcher) should always be inserted last to ensure it acts as a fallback.

[mholt]

I like this idea.

[pkkht]

Hi,
I would like to work on this task. Can I pick it up?

[francislavoie]

Sure @pkkht, PRs welcome.

[pkkht]

Ah,, unable to contribute due to change in circumstances. Sorry about that!

[armadi1809]

Hello, I would like to contribute. Can I pick this issue up?

[mholt]

Sure 👍

[armadi1809]

Thank you! I am on it then!

[armadi1809]

In the description of the issue, does multiple status codes just mean two status codes? From the looks of it we are looking at ranges here right?

Also, how do you suggest going about enforcing that a handle_error directive without arguments should come last?

[mohammed90]

does multiple status codes just mean two status codes? From the looks of it we are looking at ranges here right?

Not only 2. It can be any number of arguments or something like 4xx, see example in the Response Matcher of reverse_proxy.

how do you suggest going about enforcing that a handle_error directive without arguments should come last?

You can either keep it aside in the parsing process then append it at the end after parsing all other handlers, or manage the sorting logic in your implementation of the sort.Interface.

[armadi1809]

Sorry for the multiple follow ups and maybe the "trivial" questions, but I guess I am a little bit confused on how I should be approaching this. Is the idea here to build a new handle_errors parser from scratch where we programmatically add expressions based on the status codes arguments (instead of the user having to type them)?

Also, we need to make sure that if someone has the arguments, they are not allowed to have the current expression / handle syntax or are we getting rid of that syntax completely?

[francislavoie]

See handle_path for example:

caddy/modules/caddyhttp/rewrite/caddyfile.go

Line 155 in 18f3429

func parseCaddyfileHandlePath(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) {

Basically just need to update func parseHandleErrors to call h.RemainingArgs() and if that slice is non-empty then you create a caddyhttp.MatchExpression (with field Expr set) which is a string like "{http.error.status_code} in [" + strings.Join(", ", args) + "]" effectively.

Bonus points (not required, but nice to have) for handling 4xx style strings (which means any 400-499 status, a convention we support in most places with status code matching), which should separately expand to something like "{http.error.status_code} >= 200 && {http.error.status_code} <= 299" given 2xx, and join that with an || if there's any args with no xx.

So handle_errors 2xx 404 { should produce a matcher like:

({http.error.status_code} >= 200 && {http.error.status_code} <= 299) || {http.error.status_code} in [404]

In addition, the code in httptype.go doing if errorSubrouteVals, ok := sblock.pile["error_route"]; ok { needs to be updated to sort the error_route pile such that any of the ones with matchers go first, and ones with no matcher go last.

Run caddy adapt --pretty while developing to see what the JSON output looks like, make sure it looks correct. You can write adapt tests as well in files in caddytest/integration/caddyfile_adapt which assert that for a given Caddyfile, the correct JSON is produced. You can see an example of a sort like that from earlier in that same file with sort.SliceStable(p.serverBlocks, or even more relevant, look at func sortRoutes( in directives.go (maybe that function can be reused here? Not sure but worth a try).

[armadi1809]

Thank you for the detailed response!

[armadi1809]

@francislavoie, I think I am almost there, I got individual handle_errors directives to work with both of the status codes formats. One problem I am facing now is that only the first hanlde_errors directive in the CaddyFile is triggered and all the others are omitted. Can I get some guidance on where in the code we are enforcing this, because up until the code you showed above from httptype.go I can see all the handle_errors routes inside the errorSubrouteVals.

Thanks in advance for your help!

[francislavoie]

That's handled in the pile thing. It'll need to loop through those and insert them as routes in the final config (instead of only grabbing one).

[armadi1809]

One more thing to add here, looks like parseSegmentAsConfig does not allow more than one argument which causes parseSegmentAsSubroute to return nil whenever we have more than one argument to handle_errors. Should I be recreating a similar function to parseSegmentAsConfig that does not perform the h.NextArg() check?

Again, sorry if my questions are very trivial, I am still learning the way we want to do things here.

[mholt]

Hmm, are you referring to the one argument being the caddyhttp.Handler value? You should only need 1 handler, as it has all the tokens for the segment in it. Does that make sense?

Again, sorry if my questions are very trivial, I am still learning the way we want to do things here.

Not at all; you're contributing in a place that very few other people have even read the code for.

[armadi1809]

@mholt I was referring to the arguments on the first line right after the handle_errors directive (So 404 and 410 if the example was handle_errors 404 410 {...). But looks like that parseSegmentAsSubroute expects the cursor to be at the token just before the block opening and there are helper functions to manipulate the cursor so I think I got this figured out. Thank you!

[mholt]

Oh ok, cool -- thanks for working on this! Sounds like you're figuring it out (and it's a bit complicated, so kudos)