Skip to content

fix(cluster): return ErrClosing instead of MOVED/ASK after Close - #1037

Open
ninadsutrave wants to merge 1 commit into
redis:mainfrom
ninadsutrave:fix-moved-on-close
Open

ninadsutrave wants to merge 1 commit into
redis:mainfrom
ninadsutrave:fix-moved-on-close

Conversation

@ninadsutrave

@ninadsutrave ninadsutrave commented Oct 12, 2026 •

Copy link
Copy Markdown

When Close() has been called, shouldRefreshRetry stops following redirections, so a call that gets a MOVED or ASK reply at that point returns the raw redirection error to the caller. The Close docs say pending calls should see ErrClosing, so following @rueian's suggestion in #937 this returns ErrClosing for Do, DoMulti, DoCache and DoMultiCache.

The stop check in shouldRefreshRetry itself is unchanged, since removing it would keep retrying against closed connections. Dedicated clients and Receive are left as they are because they don't follow redirections anyway.

Added TestClusterClientRedirectWhileClosing, which closes the client from inside the mock before replying with MOVED/ASK. It fails on main and passes with this change.

This only covers the Close() path, not the DNS timeout case reported later in the issue.

Related to #937


Note

Low Risk
Targeted error-mapping on shutdown with tests; redirect/retry logic is otherwise unchanged.

Overview
After Close(), in-flight cluster commands that receive MOVED or ASK now surface ErrClosing instead of the raw redirect error, matching documented behavior for pending work during shutdown.

A new isClosingRedirect helper detects redirect replies when the client’s stop flag is set. Do, DoCache, and the multi/cache result handlers apply it at the end of redirect/retry handling (without changing shouldRefreshRetry, so closed clients still do not follow redirects). TestClusterClientRedirectWhileClosing closes the client from a mock and asserts ErrClosing for Do, DoMulti, DoCache, and DoMultiCache with both MOVED and ASK.

Reviewed by Cursor Bugbot for commit 4578d0b. Bugbot is set up for automated code reviews on this repo. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant