feat: Route RedisProxy errors through handle_store_error

Replaces the bespoke rescuing helper with handle_store_error and
declares ['Redis::BaseConnectionError'] as the default bypass list.

Author: mpenalozag

lib/rack/attack/store_proxy/redis_proxy.rb

@@ -6,32 +6,36 @@ module Rack
   class Attack
     module StoreProxy
       class RedisProxy < BaseProxy
-        def initialize(*args)
+        def initialize(store, **options)
           if Gem::Version.new(Redis::VERSION) < Gem::Version.new("3")
             warn 'RackAttack requires Redis gem >= 3.0.0.'
           end
 
-          super(*args)
+          super(store, **options)
         end
 
         def self.handle?(store)
           defined?(::Redis) && store.class == ::Redis
         end
 
+        def self.default_bypassable_store_errors
+          ['Redis::BaseConnectionError']
+        end
+
         def read(key)
-          rescuing { get(key) }
+          handle_store_error { get(key) }
         end
 
         def write(key, value, options = {})
           if (expires_in = options[:expires_in])
-            rescuing { setex(key, expires_in, value) }
+            handle_store_error { setex(key, expires_in, value) }
           else
-            rescuing { set(key, value) }
+            handle_store_error { set(key, value) }
           end
         end
 
         def increment(key, amount, options = {})
-          rescuing do
+          handle_store_error do
             pipelined do |redis|
               redis.incrby(key, amount)
               redis.expire(key, options[:expires_in]) if options[:expires_in]
@@ -40,14 +44,14 @@ def increment(key, amount, options = {})
         end
 
         def delete(key, _options = {})
-          rescuing { del(key) }
+          handle_store_error { del(key) }
         end
 
         def delete_matched(matcher, _options = nil)
           cursor = "0"
           source = matcher.source
 
-          rescuing do
+          handle_store_error do
             # Fetch keys in batches using SCAN to avoid blocking the Redis server.
             loop do
               cursor, keys = scan(cursor, match: source, count: 1000)
@@ -56,14 +60,6 @@ def delete_matched(matcher, _options = nil)
             end
           end
         end
-
-        private
-
-        def rescuing
-          yield
-        rescue Redis::BaseConnectionError
-          nil
-        end
       end
     end
   end

spec/rack_attack_redis_proxy_spec.rb

@@ -0,0 +1,135 @@
+# frozen_string_literal: true
+
+require_relative 'spec_helper'
+
+module RedisProxySpec
+  class FakeRedis
+    class Error < StandardError; end
+
+    attr_reader :calls
+
+    def initialize(raises: nil)
+      @raises = raises
+      @calls = []
+    end
+
+    def get(key)
+      record(:get, key)
+    end
+
+    def set(key, value)
+      record(:set, [key, value])
+    end
+
+    def setex(key, expires_in, value)
+      record(:setex, [key, expires_in, value])
+    end
+
+    def del(*keys)
+      record(:del, keys)
+    end
+
+    def pipelined
+      record(:pipelined)
+      yield(self) if block_given?
+      [1]
+    end
+
+    def incrby(key, amount)
+      record(:incrby, [key, amount])
+    end
+
+    def expire(key, ttl)
+      record(:expire, [key, ttl])
+    end
+
+    def scan(_cursor, **_opts)
+      ["0", []]
+    end
+
+    def record(method, args = nil)
+      @calls << [method, args]
+      raise @raises if @raises
+
+      :ok
+    end
+  end
+end
+
+describe Rack::Attack::StoreProxy::RedisProxy do
+  before do
+    skip 'redis gem not available' unless defined?(::Redis)
+  end
+
+  it "declares its default as ['Redis::BaseConnectionError']" do
+    _(Rack::Attack::StoreProxy::RedisProxy.default_bypassable_store_errors)
+      .must_equal ['Redis::BaseConnectionError']
+  end
+
+  it "bypasses Redis::BaseConnectionError by default on #read" do
+    redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down"))
+    proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis)
+    _(proxy.read("k")).must_be_nil
+  end
+
+  it "bypasses Redis::BaseConnectionError subclasses on #write" do
+    redis = RedisProxySpec::FakeRedis.new(raises: Redis::CannotConnectError.new("down"))
+    proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis)
+    _(proxy.write("k", "v")).must_be_nil
+  end
+
+  it "re-raises other errors by default" do
+    redis = RedisProxySpec::FakeRedis.new(raises: RuntimeError.new("boom"))
+    proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis)
+    _(-> { proxy.read("k") }).must_raise RuntimeError
+  end
+
+  it "bypasses custom errors when configured" do
+    redis = RedisProxySpec::FakeRedis.new(raises: RedisProxySpec::FakeRedis::Error.new("oom"))
+    proxy = Rack::Attack::StoreProxy::RedisProxy.new(
+      redis,
+      bypassable_store_errors: [RedisProxySpec::FakeRedis::Error]
+    )
+    _(proxy.read("k")).must_be_nil
+  end
+
+  it "bypasses all errors when configured with :all" do
+    redis = RedisProxySpec::FakeRedis.new(raises: RuntimeError.new("anything"))
+    proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis, bypassable_store_errors: :all)
+    _(proxy.read("k")).must_be_nil
+  end
+
+  it "re-raises default errors when configured with :none" do
+    redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down"))
+    proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis, bypassable_store_errors: :none)
+    _(-> { proxy.read("k") }).must_raise Redis::BaseConnectionError
+  end
+
+  it "user-provided Array overrides the default (does not merge)" do
+    redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down"))
+    proxy = Rack::Attack::StoreProxy::RedisProxy.new(
+      redis,
+      bypassable_store_errors: [RedisProxySpec::FakeRedis::Error]
+    )
+    _(-> { proxy.read("k") }).must_raise Redis::BaseConnectionError
+  end
+
+  it "returns nil from #increment on a matching error" do
+    redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down"))
+    proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis)
+    _(proxy.increment("k", 1, expires_in: 60)).must_be_nil
+  end
+
+  it "returns nil from #delete on a matching error" do
+    redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down"))
+    proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis)
+    _(proxy.delete("k")).must_be_nil
+  end
+
+  it "raises MisconfiguredStoreError for invalid config" do
+    redis = RedisProxySpec::FakeRedis.new
+    _(-> {
+      Rack::Attack::StoreProxy::RedisProxy.new(redis, bypassable_store_errors: :sometimes)
+    }).must_raise Rack::Attack::MisconfiguredStoreError
+  end
+end