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

Code before:

// Cleanup cleans up the resources made by h.
func (h *Handler) Cleanup() error {
	// close hijacked connections (both to client and backend)
	closeConnections := func() error {
		var err error
		h.connectionsMu.Lock()
		defer h.connectionsMu.Unlock()

		for _, oc := range h.connections {
			if oc.gracefulClose != nil {
				// this is potentially blocking while we have the lock on the connections
				// map, but that should be OK since the server has in theory shut down
				// and we are no longer using the connections map
				gracefulErr := oc.gracefulClose()
				if gracefulErr != nil && err == nil {
					err = gracefulErr
				}
			}
			closeErr := oc.conn.Close()
			if closeErr != nil && err == nil {
				err = closeErr
			}
		}
		return err
	}

	var err error
	if h.StreamCloseDelay > 0 {
		h.connectionsMu.Lock()
		// the handler is shut down, no new connection can appear,
		// so we can skip setting up the timer when there are no connections
		if len(h.connections) > 0 {
			delay := time.Duration(h.StreamCloseDelay)
			*h.connectionsCloseTimer = time.AfterFunc(delay, func() {
				h.logger.Debug("closing streaming connections after delay",
					zap.Duration("delay", delay))
				err := closeConnections()
				if err != nil {
					h.logger.Error("failed to closed connections after delay",
						zap.Error(err),
						zap.Duration("delay", delay))
				}
			})
		}
		h.connectionsMu.Unlock()
	} else {
		err = closeConnections()
	}

	// remove hosts from our config from the pool

Review comment (suggestion):
This could probably be a separate function (unexported, but doesn't need to be nested inside Cleanup as far as I can tell.)

Code after:
func (h *Handler) Cleanup() error {
	err := h.cleanupConnections()

	// remove hosts from our config from the pool
	for _, upstream := range h.Upstreams {
		_, _ = hosts.Delete(upstream.String())
	}

	return err
}

func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
	repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)

	// prepare the request for proxying; this is needed only once
	clonedReq, err := h.prepareRequest(r, repl)
	if err != nil {
		return caddyhttp.Error(http.StatusInternalServerError,
			fmt.Errorf("preparing request for upstream round-trip: %v", err))
	}

	// we will need the original headers and Host value if
	// header operations are configured; this is so that each
	// retry can apply the modifications, because placeholders
	// may be used which depend on the selected upstream for
	// their values
	reqHost := clonedReq.Host
	reqHeader := clonedReq.Header

	start := time.Now()
	defer func() {
		// total proxying duration, including time spent on LB and retries
		repl.Set("http.reverse_proxy.duration", time.Since(start))
		repl.Set("http.reverse_proxy.duration_ms", time.Since(start).Seconds()*1e3) // multiply seconds to preserve decimal (see #4666)
	}()

	// in the proxy loop, each iteration is an attempt to proxy the request,
	// and because we may retry some number of times, carry over the error
	// from previous tries because of the nuances of load balancing & retries
	var proxyErr error
	var retries int
	for {
		var done bool
		done, proxyErr = h.proxyLoopIteration(clonedReq, r, w, proxyErr, start, retries, repl, reqHeader, reqHost, next)
		if done {
			break
		}
		retries++
	}

	if proxyErr != nil {