fix: close connection when dial setup fails to avoid a file descriptor leak

[knQzx]

Summary

dial opens the TCP connection first, then runs the compression / JWT / handshake / addendum setup. if any of those steps fails it returns the error but never closes the socket, so the fd leaks on every failed connection attempt - a reconnect storm against a wrong password, a TLS failure or an unreachable server eventually exhausts the process file descriptors and no new sockets can be opened

reproduce: point a client at an endpoint that accepts the TCP connection but fails the handshake (or just use a wrong password), open connections in a loop and watch the open fd count climb without bound. the added TestDialClosesConnectionOnSetupFailure injects a connection whose handshake fails and asserts dial closed it - it fails on main and passes with this change

the fix closes the freshly dialed connection on any error after the dial succeeds, with a deferred conditional close on the named error return

Checklist

• Unit and integration tests covering the common scenarios were added

[github-actions]

💡 Nit — dial-error return still leaks if a custom DialContext returns both a conn and an error

This return sits above the new defer, so a user-supplied opt.DialContext that returns a non-nil net.Conn alongside a non-nil error still leaks the fd — the exact failure mode this PR closes everywhere else in the function.

Stdlib dialers never do this, so it's defensive only, but it's two lines and it makes the guarantee unconditional.

Suggested change

	return nil, err
	if conn != nil {
	_ = conn.Close()
	}
	return nil, err

[github-actions]

🤖 Claude review

Closes the freshly dialed socket when a post-dial setup step (compression validation, JWT fetch, handshake, addendum) fails in dial, using a deferred conditional close on a named error return. The fix is correct and minimal, and it covers both the native Open and the database/sql OpenDB surfaces since both route through dial (clickhouse.go:271, clickhouse_std.go:54).

I traced every return between the new defer and the successful return — all of them are error returns, and the named err is assigned by each explicit return nil, X, so the inner err := shadowing in the JWT block does not defeat the check. TestDialClosesConnectionOnSetupFailure does exercise the invariant and would fail on main.

One follow-up, not a blocker:

• The HTTP path has the identical leak. dialHttp builds a fresh http.Client/http.Transport per connection and returns on queryHello failure without calling CloseIdleConnections() — see the general finding below.

Blind spots:

• The skill's allowed tooling does not permit running go test/go vet, so the new test was validated by tracing rather than execution.
• The HTTP leak was reasoned from executeRequest/discardAndClose rather than measured against a live server.

Verdict: ✅ Approve

General findings

• ⚠️ Should fix — HTTP path leaks the same fd on setup failure — dialHttp never calls CloseIdleConnections()
dialHttp (conn_http.go:153) creates a per-connection http.Client with its own http.Transport, then returns on queryHello failure (conn_http.go:231) without releasing it. That drops the transport while it still holds an established keep-alive TCP connection, so the fd survives for IdleConnTimeout — which is set to opt.ConnMaxLifetime, defaulting to 1 hour (clickhouse_options.go:419).

  This reproduces under the PR description's own scenario. On a wrong password the server answers 401, executeRequest runs discardAndClose(resp.Body) (conn_http.go:760), and the socket goes back to the transport's idle pool rather than being closed. A reconnect storm over HTTP therefore still climbs without bound.

  The asymmetry is visible in the existing code: httpConnect.close() (conn_http.go:794) does call h.client.CloseIdleConnections(), but nothing on the dialHttp failure path does.

   	handshake, err := conn.queryHello(ctx, func(nativeTransport, error) {})
   	if err != nil {
  +		conn.client.CloseIdleConnections()
   		return nil, fmt.Errorf("failed to query server hello: %w", err)
   	}

  Happy to see this as a follow-up PR rather than growing this one — but it should be tracked, since the current title promises the fd leak is fixed and half the protocol surface is still affected.

_{Inline comments are attached to the relevant lines. This summary updates in place on re-review.}