False positive when comparing `any`
I came across the following code in connectrpc.com/connect:
r := recover()
// net/http checks for ErrAbortHandler with ==, so we should too.
if r == http.ErrAbortHandler { //nolint:errorlint,goerr113
panic(r) //nolint:forbidigo
}
retErr = i.handle(ctx, req.Spec(), req.Header(), r)
We can see a similar comparison in the stdlib:
if err := recover(); err != nil && err != ErrAbortHandler {
const size = 64 << 10
buf := make([]byte, size)
buf = buf[:runtime.Stack(buf, false)]
c.server.logf("http: panic serving %v: %v\n%s", c.remoteAddr, err, buf)
}
This code is fairly common across HTTP middleware that seeks to replace panics with an internal error response and add some additional observability (logs, etc.) while we're there.
The use of != is likely because recover returns any rather than an error implementation. As such, it would require an additional cast to be able to use errors.Is. Regardless, it seems the de facto correct way to test for http.ErrAbortHandler is with == or != and the lint suppression represents a false positive.
I see two ways out of this, although perhaps there's a third option I haven't spotted:
- Skip checks if either operand is
any, or perhaps does not implementerror, solving the general case. - Add
recoverwithhttp.ErrAbortHandlerto the existing allowlist, solving the specific case.
I think the preferred behavior would be to ignore instances of any comparisons. Such expressions can not be substituted by errors.Is since it only accepts error anyway