Revert the raise_errors param and Removed rescue block from ActiveSupport::Cache::RedisCacheStore#handle_exception

Author: Nicholas Stuart

activesupport/CHANGELOG.md

@@ -1,15 +1,8 @@
-*   Add `raise_errors` to `redis_cache_store`, defaults to false
+*   Removed rescue block from `ActiveSupport::Cache::RedisCacheStore#handle_exception`
 
-    Previously if an `::Redis::BaseError` was throw while performing a `Rails.cache` operation,
-    it would result in cache-miss like behavior. This allows cache implementors who want to handle
-    these connection issues on their own have an option to do so. 
-
-    ```ruby
-    cache_servers = %w(redis://cache-01:6379/0 redis://cache-02:6379/0)
-    config.cache_store = :redis_cache_store, { url: cache_servers,
-      raise_errors: true # Defaults to false. Raise Redis::BaseError instead of dropping them.
-    }
-    ```
+    Previously, if you provided a `error_handler` to `redis_cache_store`, any errors thrown by
+    the error handler would be rescued and logged only. Removed the `rescue` clause from `handle_exception`
+    to allow these to be thrown. 
 
 *   Allow entirely opting out of deprecation warnings.
 

activesupport/lib/active_support/cache/redis_cache_store.rb

@@ -140,12 +140,11 @@ def build_redis_client(url:, **redis_options)
       # Race condition TTL is not set by default. This can be used to avoid
       # "thundering herd" cache writes when hot cache entries are expired.
       # See <tt>ActiveSupport::Cache::Store#fetch</tt> for more.
-      def initialize(namespace: nil, compress: true, compress_threshold: 1.kilobyte, coder: default_coder, expires_in: nil, race_condition_ttl: nil, error_handler: DEFAULT_ERROR_HANDLER, raise_errors: false, **redis_options)
+      def initialize(namespace: nil, compress: true, compress_threshold: 1.kilobyte, coder: default_coder, expires_in: nil, race_condition_ttl: nil, error_handler: DEFAULT_ERROR_HANDLER, **redis_options)
         @redis_options = redis_options
 
         @max_key_bytesize = MAX_KEY_BYTESIZE
         @error_handler = error_handler
-        @raise_errors = raise_errors
 
         super namespace: namespace,
           compress: compress, compress_threshold: compress_threshold,
@@ -461,20 +460,13 @@ def failsafe(method, returning: nil)
           yield
         rescue ::Redis::BaseError => e
           handle_exception exception: e, method: method, returning: returning
-
-          if @raise_errors
-            raise e
-          else
-            returning
-          end
+          returning
         end
 
         def handle_exception(exception:, method:, returning:)
           if @error_handler
             @error_handler.(method: method, exception: exception, returning: returning)
           end
-        rescue => failsafe
-          warn "RedisCacheStore ignored exception in handle_exception: #{failsafe.class}: #{failsafe.message}\n  #{failsafe.backtrace.join("\n  ")}"
         end
     end
   end

activesupport/test/cache/stores/redis_cache_store_test.rb

@@ -287,34 +287,34 @@ class FailureRaisingFromUnavailableClientTest < StoreTest
     include FailureRaisingBehavior
 
     private
-    def emulating_unavailability
-      old_client = Redis.send(:remove_const, :Client)
-      Redis.const_set(:Client, UnavailableRedisClient)
+      def emulating_unavailability
+        old_client = Redis.send(:remove_const, :Client)
+        Redis.const_set(:Client, UnavailableRedisClient)
 
-      assert_raise Redis::BaseConnectionError do
-        yield ActiveSupport::Cache::RedisCacheStore.new(namespace: @namespace, raise_errors: true)
+        assert_raise Redis::BaseConnectionError do
+          yield ActiveSupport::Cache::RedisCacheStore.new(namespace: @namespace, error_handler: -> (method:, returning:, exception:) { raise exception })
+        end
+      ensure
+        Redis.send(:remove_const, :Client)
+        Redis.const_set(:Client, old_client)
       end
-    ensure
-      Redis.send(:remove_const, :Client)
-      Redis.const_set(:Client, old_client)
-    end
   end
 
   class FailureRaisingFromMaxClientsReachedErrorTest < StoreTest
     include FailureRaisingBehavior
 
     private
-    def emulating_unavailability
-      old_client = Redis.send(:remove_const, :Client)
-      Redis.const_set(:Client, MaxClientsReachedRedisClient)
+      def emulating_unavailability
+        old_client = Redis.send(:remove_const, :Client)
+        Redis.const_set(:Client, MaxClientsReachedRedisClient)
 
-      assert_raise Redis::CommandError do
-        yield ActiveSupport::Cache::RedisCacheStore.new(namespace: @namespace, raise_errors: true)
+        assert_raise Redis::CommandError do
+          yield ActiveSupport::Cache::RedisCacheStore.new(namespace: @namespace, error_handler: -> (method:, returning:, exception:) { raise exception })
+        end
+      ensure
+        Redis.send(:remove_const, :Client)
+        Redis.const_set(:Client, old_client)
       end
-    ensure
-      Redis.send(:remove_const, :Client)
-      Redis.const_set(:Client, old_client)
-    end
   end
 
   class FailureSafetyFromUnavailableClientTest < StoreTest

guides/source/caching_with_rails.md

@@ -528,7 +528,6 @@ config.cache_store = :redis_cache_store, { url: cache_servers,
   read_timeout:       0.2, # Defaults to 1 second
   write_timeout:      0.2, # Defaults to 1 second
   reconnect_attempts: 1,   # Defaults to 0
-  raise_errors: true, # Defaults to false. Raise Redis::BaseError instead of dropping them.
   error_handler: -> (method:, returning:, exception:) {
     # Report errors to Sentry as warnings
     Raven.capture_exception exception, level: 'warning',