caddyfile: keep error chain info in Dispenser.Errf

[ggicci]

Use fmt.Errorf instead of fmt.Sprintf in Dispenser.Errf method to reserve the error chain information. Make the underlying error unwrapable by using errors.Is or errors.As helpers.

[CLAassistant]

All committers have signed the CLA.

[mholt]

Thanks for this. What is the use case for this? (There's no context / issue associated.) And why do we have to define our own error type instead of using %w to do it for us?

[ggicci]

What is the use case for this? (There's no context / issue associated.)

This API Dispenser.Errf is useful when extending the Caddyfile parser. When I was writing the Caddyfile parser for ggicci/caddy-jwt, I found that it's not "easy" to check the error in a test. I can't use errors.Is nor errors.As to reach some more specific error information in the error chain. The root error may be wrapped multiple times during the parsing stuff. For example:

In the codebase at this time, I returned an error object as follows:

func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {
    // ...
    return nil, h.Errf("invalid sign_key")
}

And in the corresponding test, I wrote:

func TestParsingCaddyfileError(t *testing.T) {
    // ...
    _, err := parseCaddyfile(helper)
    assert.Contains(t, err.Error(), "sign_key")  // fragile, broken when error content changed
}

Assertion statements checking some error information by string comparison can be fragile.

If Dispenser.Errf can wrap errors, It can help writing more confident code as follows:

var ErrInvalidSignKey = errors.New("invalid sign_key")
func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {
    // ...
    return nil, h.Errf("%w", ErrInvalidSignKey)
}

func TestParsingCaddyfileError(t *testing.T) {
    // ...
    _, err := parseCaddyfile(helper)
    assert.ErrorIs(t, err, ErrInvalidSignKey)  // more stable
}

And why do we have to define our own error type instead of using %w to do it for us?

1. It does no harm to performance nor resource usage of the program.
2. The code can be more readable.

[mholt]

Gotcha, thanks for the explanation.

If using %w would achieve the same result, I think I'd rather use that just for simplicity's sake; I'm not sure what about that is harder to read than having a whole new type defintion to manually wrap the error. But if this is the only way to get the result you want, then that's OK I guess.

[mholt]

If Err could use Errf, or vice-versa, that might be nice to reduce a little bit of code duplication / repetition.

[ggicci]

I refactored it and I think it's more concise now.

[mholt]

LGTM now -- thanks!