Merge pull request rails#55000 from Shopify/rescue_dalli_timeout_in_read_multi_entries

Rescue connection related errors in MemCacheStore#read_multi_entries

Author: byroot

activesupport/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Fix `ActiveSupport::Cache::MemCacheStore#read_multi` to handle network errors.
+
+    This method specifically wasn't handling network errors like other codepaths.
+
+    *Alessandro Dal Grande*
+
 *   Fix Active Support Cache `fetch_multi` when local store is active.
 
     `fetch_multi` now properly yield to the provided block for missing entries

activesupport/lib/active_support/cache/mem_cache_store.rb

@@ -212,26 +212,24 @@ def write_serialized_entry(key, payload, **options)
         def read_multi_entries(names, **options)
           keys_to_names = names.index_by { |name| normalize_key(name, options) }
 
-          raw_values = begin
-            @data.with { |c| c.get_multi(keys_to_names.keys) }
-          rescue Dalli::UnmarshalError
-            {}
-          end
+          rescue_error_with({}) do
+            raw_values = @data.with { |c| c.get_multi(keys_to_names.keys) }
 
-          values = {}
+            values = {}
 
-          raw_values.each do |key, value|
-            entry = deserialize_entry(value, raw: options[:raw])
+            raw_values.each do |key, value|
+              entry = deserialize_entry(value, raw: options[:raw])
 
-            unless entry.nil? || entry.expired? || entry.mismatched?(normalize_version(keys_to_names[key], options))
-              begin
-                values[keys_to_names[key]] = entry.value
-              rescue DeserializationError
+              unless entry.nil? || entry.expired? || entry.mismatched?(normalize_version(keys_to_names[key], options))
+                begin
+                  values[keys_to_names[key]] = entry.value
+                rescue DeserializationError
+                end
               end
             end
-          end
 
-          values
+            values
+          end
         end
 
         # Delete an entry from the cache.

activesupport/test/cache/stores/mem_cache_store_test.rb

@@ -371,6 +371,30 @@ def test_can_read_multi_entries_raw_values_from_dalli_store
     assert_equal({}, @cache.send(:read_multi_entries, [key]))
   end
 
+  def test_falls_back_to_default_value_when_client_raises_dalli_error
+    cache = lookup_store
+    client = cache.instance_variable_get(:@data)
+    client.stub(:get_multi, lambda { |*_args| raise Dalli::DalliError.new("test error") }) do
+      assert_equal({}, cache.read_multi("key1", "key2"))
+    end
+  end
+
+  def test_falls_back_to_default_value_when_client_raises_connection_pool_timeout_error
+    cache = lookup_store
+    client = cache.instance_variable_get(:@data)
+    client.stub(:get_multi, lambda { |*_args| raise ConnectionPool::TimeoutError.new("test error") }) do
+      assert_equal({}, cache.read_multi("key1", "key2"))
+    end
+  end
+
+  def test_falls_back_to_default_value_when_client_raises_connection_pool_error
+    cache = lookup_store
+    client = cache.instance_variable_get(:@data)
+    client.stub(:get_multi, lambda { |*_args| raise ConnectionPool::Error.new("test error") }) do
+      assert_equal({}, cache.read_multi("key1", "key2"))
+    end
+  end
+
   def test_pool_options_work
     cache = ActiveSupport::Cache.lookup_store(:mem_cache_store, pool: { size: 2, timeout: 1 })
     pool = cache.instance_variable_get(:@data) # loads 'connection_pool' gem