Refresh with retry + bug fixes (#88)

Author: dim

CHANGELOG.md

@@ -1,6 +1,12 @@
-## v0.10.0
-
- - Feature: New ObtainMulti method to acquire multiple locks atomically [#70](https://github.com/bsm/redislock/pull/70)
+## Unreleased
+
+- Feature: New ObtainMulti method to acquire multiple locks atomically [#70](https://github.com/bsm/redislock/pull/70)
+- Feature: `Lock.Refresh` now accepts an `Options.RetryStrategy` and retries transient redis errors.
+- Fix: a lost lock (refresh against a key whose value no longer matches) now returns `ErrNotObtained` immediately instead of being silently retried by the retry loop.
+- Fix: `withRetry` surfaces the last transient error when retries are exhausted, instead of masking it as `ErrNotObtained`.
+- Fix: when retries are interrupted by a cancelled context, return `errors.Join(ErrNotObtained, ctx.Err())` (or the last transient error joined with `ctx.Err()`) so callers can match on both with `errors.Is`.
+- Docs: clarify `ExponentialBackoff` formula and the behaviour of its `min`/`max` arguments (first wait is 4ms; pass `min >= 16ms` for a sensible floor, non-zero `max` to cap growth).
+- Chore: bump `go-redis/v9` to v9.19.0 and raise minimum Go to 1.25 [#86](https://github.com/bsm/redislock/pull/86).
 
 ## v0.9.4
 

redislock.go

@@ -80,27 +80,65 @@ func (c *Client) ObtainMulti(ctx context.Context, keys []string, ttl time.Durati
 
 	value := token + opt.getMetadata()
 	ttlVal := strconv.FormatInt(int64(ttl/time.Millisecond), 10)
-	retry := opt.getRetryStrategy()
 
-	// make sure we don't retry forever
+	if err := withRetry(ctx, ttl, opt.getRetryStrategy(), func(ctx context.Context) (bool, error) {
+		ok, err := c.obtain(ctx, keys, value, len(token), ttlVal)
+		if err != nil {
+			// any non-nil error from obtain is terminal (transient redis
+			// errors are unlikely to clear within a lock TTL and retrying a
+			// broken server is futile).
+			return true, err
+		}
+		if ok {
+			return true, nil
+		}
+		// lock is held by someone else; retryable.
+		return false, nil
+	}); err != nil {
+		return nil, err
+	}
+	return &Lock{Client: c, keys: keys, value: value, tokenLen: len(token)}, nil
+}
+
+// withRetry runs attempt repeatedly until it signals it is done, the retry
+// strategy is exhausted, or ctx is done. The attempt function returns
+// (done, err):
+//   - done=true: stop and return err (success when err is nil, otherwise a
+//     terminal failure).
+//   - done=false, err=nil: retryable, no error to surface; on exhaustion
+//     withRetry returns ErrNotObtained.
+//   - done=false, err!=nil: retryable transient error; on exhaustion
+//     withRetry returns this last error, and on ctx cancellation it returns
+//     the last error joined with ctx.Err(). If no transient error was seen,
+//     ctx cancellation returns ErrNotObtained joined with ctx.Err().
+//
+// If ctx has no deadline one is derived from ttl as a final safety cap, so a
+// caller passing an unbounded retry strategy and a background context cannot
+// loop forever.
+func withRetry(ctx context.Context, ttl time.Duration, retry RetryStrategy, attempt func(context.Context) (bool, error)) error {
 	if _, ok := ctx.Deadline(); !ok {
 		var cancel context.CancelFunc
 		ctx, cancel = context.WithDeadline(ctx, time.Now().Add(ttl))
 		defer cancel()
 	}
 
 	var ticker *time.Ticker
+	var lastErr error
 	for {
-		ok, err := c.obtain(ctx, keys, value, len(token), ttlVal)
+		done, err := attempt(ctx)
+		if done {
+			return err
+		}
 		if err != nil {
-			return nil, err
-		} else if ok {
-			return &Lock{Client: c, keys: keys, value: value, tokenLen: len(token)}, nil
+			lastErr = err
 		}
 
 		backoff := retry.NextBackoff()
 		if backoff < 1 {
-			return nil, ErrNotObtained
+			if lastErr != nil {
+				return lastErr
+			}
+			return ErrNotObtained
 		}
 
 		if ticker == nil {
@@ -112,7 +150,11 @@ func (c *Client) ObtainMulti(ctx context.Context, keys []string, ttl time.Durati
 
 		select {
 		case <-ctx.Done():
-			return nil, ctx.Err()
+			err := lastErr
+			if err == nil {
+				err = ErrNotObtained
+			}
+			return errors.Join(err, ctx.Err())
 		case <-ticker.C:
 		}
 	}
@@ -210,14 +252,19 @@ func (l *Lock) Refresh(ctx context.Context, ttl time.Duration, opt *Options) err
 		return ErrNotObtained
 	}
 	ttlVal := strconv.FormatInt(int64(ttl/time.Millisecond), 10)
-	_, err := luaRefresh.Run(ctx, l.client, l.keys, l.value, ttlVal).Result()
-	if err != nil {
+
+	return withRetry(ctx, ttl, opt.getRetryStrategy(), func(ctx context.Context) (bool, error) {
+		_, err := luaRefresh.Run(ctx, l.client, l.keys, l.value, ttlVal).Result()
+		if err == nil {
+			return true, nil
+		}
 		if errors.Is(err, redis.Nil) {
-			return ErrNotObtained
+			// the lock is no longer held by us; retrying cannot recover it.
+			return true, ErrNotObtained
 		}
-		return err
-	}
-	return nil
+		// transient error; allow the retry strategy to try again.
+		return false, err
+	})
 }
 
 // Release manually releases the lock.