Allow requests to proceed on cache server connection errors

admin/server/server.go

@@ -344,12 +344,6 @@ func (s *Server) checkRateLimit(ctx context.Context) (context.Context, error) {
 		return ctx, fmt.Errorf("server context does not have a method")
 	}
 
-	// If the connection to the cache server is lost, skip rate limiting.
-	if err := s.limiter.Ping(ctx); err != nil {
-		s.logger.Warn("skipping rate limiting due to cache connection error", zap.Error(err))
-		return ctx, nil
-	}
-
 	// Don't rate limit superusers. This is useful for scripting.
 	if auth.GetClaims(ctx).Superuser(ctx) {
 		return ctx, nil

runtime/pkg/ratelimit/ratelimiter.go

@@ -2,8 +2,12 @@ package ratelimit
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"io"
 	"math"
+	"net"
+	"strings"
 
 	"github.com/go-redis/redis_rate/v10"
 	"github.com/redis/go-redis/v9"
@@ -42,6 +46,13 @@ func (l *Redis) Limit(ctx context.Context, limitKey string, limit redis_rate.Lim
 
 	rateResult, err := l.Allow(ctx, limitKey, limit)
 	if err != nil {
+		// If the error is a server connection error, we should not return an error.
+		// This is because the server may be temporarily unavailable, and we should not block the request.
+		// The client should retry the request.
+		if isServerConnError(err) {
+			return nil
+		}
+
 		return err
 	}
 
@@ -72,15 +83,14 @@ func (n Noop) Ping(ctx context.Context) error {
 	return nil
 }
 
-var Default = redis_rate.PerMinute(180)
-
-var Sensitive = redis_rate.PerMinute(30)
-
-var Public = redis_rate.PerMinute(750)
-
-var Unlimited = redis_rate.PerSecond(math.MaxInt)
-
-var Zero = redis_rate.Limit{}
+// Common rate limit configurations
+var (
+	Default   = redis_rate.PerMinute(180)
+	Sensitive = redis_rate.PerMinute(30)
+	Public    = redis_rate.PerMinute(750)
+	Unlimited = redis_rate.PerSecond(math.MaxInt)
+	Zero      = redis_rate.Limit{}
+)
 
 type QuotaExceededError struct {
 	message string
@@ -101,3 +111,45 @@ func AuthLimitKey(methodName, authID string) string {
 func AnonLimitKey(methodName, peer string) string {
 	return fmt.Sprintf("anon:%s:%s", methodName, peer)
 }
+
+// Common Redis error messages
+const (
+	errMaxClients  = "ERR max number of clients reached"
+	errLoading     = "LOADING "
+	errReadOnly    = "READONLY "
+	errMasterDown  = "MASTERDOWN "
+	errClusterDown = "CLUSTERDOWN "
+	errTryAgain    = "TRYAGAIN "
+)
+
+func isServerConnError(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	// Check specific I/O errors
+	if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
+		return true
+	}
+
+	// Check for network-specific errors
+	var netErr net.Error
+	if errors.As(err, &netErr) {
+		return true
+	}
+
+	// Check specific Redis error strings
+	s := err.Error()
+	if s == errMaxClients {
+		return true
+	}
+	if strings.HasPrefix(s, errLoading) ||
+		strings.HasPrefix(s, errReadOnly) ||
+		strings.HasPrefix(s, errMasterDown) ||
+		strings.HasPrefix(s, errClusterDown) ||
+		strings.HasPrefix(s, errTryAgain) {
+		return true
+	}
+
+	return false
+}

runtime/server/server.go

@@ -343,12 +343,6 @@ func mapGRPCError(err error) error {
 }
 
 func (s *Server) checkRateLimit(ctx context.Context) (context.Context, error) {
-	// If the connection to the cache server is lost, skip rate limiting.
-	if err := s.limiter.Ping(ctx); err != nil {
-		s.logger.Warn("skipping rate limiting due to cache connection error", zap.Error(err))
-		return ctx, nil
-	}
-
 	// Any request type might be limited separately as it is part of Metadata
 	// Any request type might be excluded from this limit check and limited later,
 	// e.g. in the corresponding request handler by calling s.limiter.Limit(ctx, "limitKey", redis_rate.PerMinute(100))