Repository: caddyserver/caddy
File: modules/caddyhttp/reverseproxy/healthchecks.go (Go)

Code before:

	timeout := time.Duration(a.Timeout)
	if timeout == 0 {
		timeout = 5 * time.Second
	}

	if a.Path != "" {
		a.logger.Warn("the 'path' option is deprecated, please use 'uri' instead!")
	}

	// parse the URI string (supports path and query)
	if a.URI != "" {
		parsedURI, err := url.Parse(a.URI)
		if err != nil {
			return err
		}
		a.uri = parsedURI
	}

	a.httpClient = &http.Client{
		Timeout:   timeout,
		Transport: h.Transport,
		CheckRedirect: func(req *http.Request, via []*http.Request) error {
			if !a.HealthFollowRedirects {
				return errors.New(
					"Redirects are disabled in health check, set health_follow_redirects flag in config to avoid this error")
			} else {
				return nil
			}
		},
	}

	for _, upstream := range h.Upstreams {
		// if there's an alternative port for health-check provided in the config,
		// then use it, otherwise use the port of upstream.
		if a.Port != 0 {
			upstream.activeHealthCheckPort = a.Port
		}
	}

	if a.Interval == 0 {
		a.Interval = caddy.Duration(30 * time.Second)
	}

	if a.ExpectBody != "" {
		var err error
		a.bodyRegexp, err = regexp.Compile(a.ExpectBody)
		if err != nil {
			return fmt.Errorf("expect_body: compiling regular expression: %v", err)
		}
	}

Review comment (suggestion):
We typically prefer `fmt.Errorf` for consistency, avoids an extra import line and makes it easier to add some parameters for formatting if needed.

The newline is not necessary.

Adjusted the message as well.

```suggestion
				return fmt.Errorf("active health check encountered a redirect; enable 'health_follow_redirects' if redirects are intentional")
```

But actually, doing a bit of research, I think it would be better to use `http.ErrUseLastResponse` because that's a special error that the Go stdlib uses as a signal to mean "just return the response as-is without an error if a redirect is encountered". It does mean that users won't see an explanation to use this flag in their logs, but I think that's okay, since we document it elsewhere. They'll instead see an `status code out of tolerances` message in the logs. (/cc @mholt it looks like we actually reject `< 200 || >= 400` which means `3xx` are allowed right now, shouldn't we reduce it to `< 200 || >= 300` ?)

```suggestion
				return http.ErrUseLastResponse
```

Code after:
	timeout := time.Duration(a.Timeout)
	if timeout == 0 {
		timeout = 5 * time.Second
	}

	if a.Path != "" {
		a.logger.Warn("the 'path' option is deprecated, please use 'uri' instead!")
	}

	// parse the URI string (supports path and query)
	if a.URI != "" {
		parsedURI, err := url.Parse(a.URI)
		if err != nil {
			return err
		}
		a.uri = parsedURI
	}

	a.httpClient = &http.Client{
		Timeout:   timeout,
		Transport: h.Transport,
		CheckRedirect: func(req *http.Request, via []*http.Request) error {
			if !a.FollowRedirects {
				return http.ErrUseLastResponse
			}
			return nil
		},
	}

	for _, upstream := range h.Upstreams {
		// if there's an alternative port for health-check provided in the config,
		// then use it, otherwise use the port of upstream.
		if a.Port != 0 {
			upstream.activeHealthCheckPort = a.Port
		}
	}

	if a.Interval == 0 {
		a.Interval = caddy.Duration(30 * time.Second)
	}

	if a.ExpectBody != "" {
		var err error
		a.bodyRegexp, err = regexp.Compile(a.ExpectBody)
		if err != nil {
			return fmt.Errorf("expect_body: compiling regular expression: %v", err)
		}
	}

	if a.Passes < 1 {
		a.Passes = 1