fix: run MONITOR on a dedicated connection and propagate read errors

[ankit-songara]

Fixes #3079

Problem

Two layered bugs, the second discovered while CI's race detector ran the regression test for the first:

1. Read errors were silently swallowed (the reported issue). readMonitor discards the error from rd.Peek(1), so once the connection died the goroutine spun in a hot loop forever re-peeking it, cmd.Err() stayed nil, and a listener blocked on the monitor channel hung indefinitely with no way to detect the failure.

2. MONITOR ran on a pooled connection. MonitorCmd.readReply spawns the reader goroutine and returns immediately, so withConn put the connection back into the pool while the goroutine was still reading from it. The pool's putConn health-check peek then read from the same bufio.Reader concurrently — a data race (CI flagged it on the first version of this PR). It also meant the client's ReadTimeout stayed armed on the connection, which is why a monitor on a quiet server broke after a few seconds in the first place.

Changes

• _process routes MonitorCmd to a new processMonitor, which runs the command on a dedicated, non-pooled connection (ConnPool.NewConn) with no read deadline — matching both the doc comment's existing promise ("It needs a dedicated connection") and how MONITOR behaves in redis-cli. An idle server no longer breaks the monitor.
• On a read error the channel is closed and the error is available via cmd.Err(), so a blocked listener wakes up (msg, ok := <-ch with ok == false) — option 2 from the issue.
• Stop() closes the dedicated connection to unblock a reader waiting in Peek for traffic. A clean stop reports no error and leaves the channel open (unchanged from before).
• readMonitor no longer holds the mutex across the blocking Peek (that would deadlock Stop()); the mutex now only guards the status field. The reader goroutine is the sole reader of the connection, so no lock is needed around reads.

Testing

Two regression tests using a fake in-process TCP server speaking just enough RESP for MONITOR — they need no real Redis and are not gated behind RUN_MONITOR_TEST:

• TestMonitorConnErrorClosesChannel: server drops the connection mid-monitor → channel closes, Err() is set. Hangs forever on master.
• TestMonitorStopWithoutTraffic: Stop() takes effect with zero server traffic and reports no error. Deadlocks or times out on master since the reader only observed the stop flag when data arrived.

---

Note

Medium Risk
Changes core command dispatch and long-lived connection handling for MONITOR; behavior is more correct but affects anyone relying on the old pooled-connection semantics.

Overview
Fixes MONITOR lifecycle bugs (#3079): pooled connections, silent read failures, and Stop blocking when the server is idle.

MONITOR now runs on a dedicated non-pooled connection via processMonitor (NewConn, no read deadline), so idle servers do not hit ReadTimeout and the background reader no longer races with pool health checks on the same bufio.Reader.

On connection failure, readMonitor surfaces Peek/ReadString errors, sets cmd.Err(), and closes the monitor channel so blocked consumers can exit. A clean Stop() closes that connection to unblock Peek, leaves the channel open, and does not set an error; readMonitor avoids holding the mutex across blocking reads and treats stop-induced close as success.

Docs note the new behavior; CloseReasonMonitor labels dedicated monitor teardown. Regression tests use a fake TCP server for connection drop and stop-without-traffic.

^{Reviewed by Cursor Bugbot for commit 4176bec. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

^{Reviewed by Cursor Bugbot for commit 53941c2. Configure here.}

[ndyakov]

Thank you for this contribution @ankit-songara. I did close the previous fix which was not ideal anyway and I do prefer this one. Will review this one as soon as possible.