Self-review: found a bug and a couple of observations.

Bug: ctx vs retryCtx mismatch

In listFromShard, we create retryCtx with a timeout but pass the original ctx (no timeout guarantee) to doList:

retryCtx, cancel := context.WithTimeout(ctx, c.options.requestTimeout)
backOff := time2.NewBackOff(retryCtx)

err := backoff.RetryNotify(func() error {
    return c.doList(ctx, request, hint, ch)  // ← uses ctx, not retryCtx
}, backOff, ...)

The read/write batch pattern uses the same timeout context for both the backoff and the RPC:

ctx, cancel := context.WithTimeout(context.Background(), b.requestTimeout)
backOff := time2.NewBackOff(ctx)
err = backoff.RetryNotify(func() error {
    response, err = b.doRequest(ctx, request, hint)  // ← same ctx
}, backOff, ...)

If the caller passes context.Background(), individual RPCs in doList have no deadline and could hang forever. We should pass retryCtx to doList instead of ctx.

Observation: error path when dataSent && IsRetriable

When data has already been sent and we hit a retriable error, we return backoff.Permanent(err). This is correct — the consumer sees partial data then an error result, and we avoid duplicates. Just want to call out this is intentional.

Will push a fix for the context bug.

Self-review: PR #937 (List retry)

Code correctness

Retry logic — Follows the same backoff.RetryNotify + FindLeaderHint pattern as readBatch.doRequestWithRetries and writeBatch.doRequestWithRetries. The retryCtx (with requestTimeout) is correctly used for both the backoff and the RPC calls.

Duplicate prevention — The dataSent flag ensures we only retry before any data has been written to the channel. Once data flows, a retriable error becomes permanent. This is the right trade-off: we handle the common case (not-leader on initial connect) and avoid the complex case (mid-stream retry with deduplication).

Channel close semantics — listFromShard never closes the channel, same as the original. The caller (List()) handles closing via close(ch) in the single-shard case and via WaitGroup + close(ch) in the multi-shard case. No behavior change here.

Error wrapping — backoff.Permanent(err) wraps the error, but backoff.RetryNotify unwraps PermanentError before returning. The caller sees the original error.

Observations

1. requestTimeout may be tight for large List operations returning millions of keys. However, this is consistent with read/write batch behavior and can be tuned via WithRequestTimeout.

2. Both PRs export IsRetriable independently — whichever merges second will have a no-op change on rpc_errors.go. Git merge will handle this cleanly.

3. Test setup is duplicated from existing TestLeaderHintWithoutClient/TestLeaderHintWithClient. Could extract a helper, but keeping it consistent with the existing test style.

Verdict

LGTM after the retryCtx fix (already pushed).