timakin / bodyclose

Analyzer: checks whether HTTP response body is closed and a re-use of TCP connection is not blocked.
MIT License
309 stars 33 forks source link

Minor false positive when response body is passed to deferred function as parameter #60

Open pellared opened 3 months ago

pellared commented 3 months ago

I got a minor false positive for a deferred response close like below:

    defer func(Body io.ReadCloser) {
        err := Body.Close()
        if err != nil {
            log.Printf("failed to close http response body, %v\n", err)
        }
    }(res.Body)

The workaround was to refactor to:

    defer func() {
        err := res.Body.Close()
        if err != nil {
            log.Printf("failed to close http response body, %v\n", err)
        }
    }()

Tested with https://github.com/golangci/golangci-lint/releases/tag/v1.59.1 and https://github.com/open-telemetry/opentelemetry-go-contrib. PR for reference: https://github.com/open-telemetry/opentelemetry-go-contrib/pull/5962

pellared commented 3 months ago

Might be related to https://github.com/timakin/bodyclose/issues/51

tisonkun commented 1 week ago

@timakin This is quite a big downside for other project adopt it. As per #50 I add a helper function:

// sneakyBodyClose closes the body and ignores the error.
// This is useful to close the HTTP response body when we don't care about the error.
func sneakyBodyClose(body io.ReadCloser) {
    if body != nil {
        _ = body.Close()
    }
}

And now bodyclose cannot find defer sneakyBodyClose(resp.Body) actually properly close the body.