提交 e63fb240 编写于 作者: L Lin Jen-Shin

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
上级 c8c1460f
......@@ -28,13 +28,13 @@ def call(env)
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
......
......@@ -47,6 +47,17 @@ def test_local_cache_cleared_on_exception
assert_raises(RuntimeError) { middleware.call({}) }
assert_nil LocalCacheRegistry.cache_for(key)
end
def test_local_cache_cleared_on_throw
key = "super awesome key"
assert_nil LocalCacheRegistry.cache_for key
middleware = Middleware.new("<3", key).new(->(env) {
assert LocalCacheRegistry.cache_for(key), "should have a cache"
throw :warden
})
assert_throws(:warden) { middleware.call({}) }
assert_nil LocalCacheRegistry.cache_for(key)
end
end
end
end
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册