From e63fb2407a1dd5de1bfe8d4d8b1976ce51022f0c Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 10 Mar 2017 17:34:54 +0000 Subject: Make sure local cache cleared even it's throwing: We (GitLab) hit into an issue that somewhere in the middleware chain was throwing `:warden`, which was caught in the wrapping middleware, but `LocalCache::Middleware` was not aware of it. It should look like: ``` ruby result = catch(:warden) do @app.call(env) end ``` Source: https://github.com/hassox/warden/blob/090ed153dbd2f5bf4a1ca672b3018877e21223a4/lib/warden/manager.rb#L35-L37 Using `ensure` could make sure that we would always do the cleanup, and better yet, avoid `rescue Exception` which we all should know that could cause some issues which could be very hard to debug. Please check the discussion thread for more context: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1402#note_25128108 --- .../lib/active_support/cache/strategy/local_cache_middleware.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'activesupport/lib/active_support/cache/strategy') diff --git a/activesupport/lib/active_support/cache/strategy/local_cache_middleware.rb b/activesupport/lib/active_support/cache/strategy/local_cache_middleware.rb index 174cb72b1e..4c3679e4bf 100644 --- a/activesupport/lib/active_support/cache/strategy/local_cache_middleware.rb +++ b/activesupport/lib/active_support/cache/strategy/local_cache_middleware.rb @@ -28,13 +28,13 @@ module ActiveSupport response[2] = ::Rack::BodyProxy.new(response[2]) do LocalCacheRegistry.set_cache_for(local_cache_key, nil) end + cleanup_on_body_close = true response rescue Rack::Utils::InvalidParameterError - LocalCacheRegistry.set_cache_for(local_cache_key, nil) [400, {}, []] - rescue Exception - LocalCacheRegistry.set_cache_for(local_cache_key, nil) - raise + ensure + LocalCacheRegistry.set_cache_for(local_cache_key, nil) unless + cleanup_on_body_close end end end -- cgit v1.2.3