Bug #389/propagate redlock exceptions (#394)

__aexit__ was returning whether asyncio Event was removed or not. In some cases this was avoiding the context manager to propagate exceptions happening inside. Now its not returning anything and will raise always any exception raised from inside

.gitignore

@@ -94,3 +94,4 @@ _release_notes
 
 tags
 .mypy_cache/
+.pytest_cache/

Pipfile

@@ -14,3 +14,4 @@ name = "pypi"
 
 "-e .[dev]" = "*"
 "e1839a8" = {path = ".", editable = true}
+ipdb = "*"

aiocache/lock.py

@@ -94,13 +94,12 @@ async def _wait_for_release(self):
             pass
 
     async def __aexit__(self, exc_type, exc_value, traceback):
-        return await self._release()
+        await self._release()
 
     async def _release(self):
         removed = await self.client._redlock_release(self.key, self._value)
         if removed:
             RedLock._EVENTS.pop(self.key).set()
-        return removed
 
 
 class OptimisticLock:

tests/acceptance/test_decorators.py

@@ -81,14 +81,24 @@ async def test_cached_stampede(self, mocker, cache):
     async def test_locking_dogpile_lease_expiration(self, mocker, cache):
         mocker.spy(cache, 'get')
         mocker.spy(cache, 'set')
-        decorator = cached_stampede(ttl=10, lease=1)
+        decorator = cached_stampede(ttl=10, lease=3)
 
         await asyncio.gather(
+            decorator(stub)(1, seconds=1),
             decorator(stub)(1, seconds=2),
-            decorator(stub)(1, seconds=2))
+            decorator(stub)(1, seconds=3))
 
-        assert cache.get.call_count == 4
-        assert cache.set.call_count == 2
+        assert cache.get.call_count == 6
+        assert cache.set.call_count == 3
+
+    @pytest.mark.asyncio
+    async def test_locking_dogpile_task_cancellation(self, mocker, cache):
+        @cached_stampede()
+        async def cancel_task():
+            raise asyncio.CancelledError()
+
+        with pytest.raises(asyncio.CancelledError):
+            await cancel_task()
 
 
 class TestMultiCachedDecorator:

tests/acceptance/test_lock.py

@@ -20,7 +20,7 @@ async def test_acquire(self, cache, lock):
 
     @pytest.mark.asyncio
     async def test_release_does_nothing_when_no_lock(self, lock):
-        assert await lock.__aexit__("exc_type", "exc_value", "traceback") == 0
+        assert await lock.__aexit__("exc_type", "exc_value", "traceback") is None
 
     @pytest.mark.asyncio
     async def test_acquire_release(self, cache, lock):
@@ -72,6 +72,15 @@ async def dummy():
         assert cache.get.call_count == 8
         assert cache.set.call_count == 4
 
+    @pytest.mark.asyncio
+    async def test_locking_dogpile_propagates_exceptions(self, cache):
+        async def dummy():
+            async with RedLock(cache, pytest.KEY, lease=1):
+                raise ValueError()
+
+        with pytest.raises(ValueError):
+            await dummy()
+
 
 class TestMemoryRedLock:
 
@@ -83,20 +92,20 @@ def lock(self, memory_cache):
     async def test_release_wrong_token_fails(self, lock):
         await lock.__aenter__()
         lock._value = "random"
-        assert await lock.__aexit__("exc_type", "exc_value", "traceback") == 0
+        assert await lock.__aexit__("exc_type", "exc_value", "traceback") is None
 
     @pytest.mark.asyncio
     async def test_release_wrong_client_fails(self, memory_cache, lock):
         wrong_lock = RedLock(memory_cache, pytest.KEY, 20)
         await lock.__aenter__()
-        assert await wrong_lock.__aexit__("exc_type", "exc_value", "traceback") == 0
+        assert await wrong_lock.__aexit__("exc_type", "exc_value", "traceback") is None
 
     @pytest.mark.asyncio
     async def test_float_lease(self, memory_cache):
         lock = RedLock(memory_cache, pytest.KEY, 0.1)
         await lock.__aenter__()
         await asyncio.sleep(0.2)
-        assert await lock.__aexit__("exc_type", "exc_value", "traceback") == 0
+        assert await lock.__aexit__("exc_type", "exc_value", "traceback") is None
 
 
 class TestRedisRedLock:
@@ -109,20 +118,20 @@ def lock(self, redis_cache):
     async def test_release_wrong_token_fails(self, lock):
         await lock.__aenter__()
         lock._value = "random"
-        assert await lock.__aexit__("exc_type", "exc_value", "traceback") == 0
+        assert await lock.__aexit__("exc_type", "exc_value", "traceback") is None
 
     @pytest.mark.asyncio
     async def test_release_wrong_client_fails(self, redis_cache, lock):
         wrong_lock = RedLock(redis_cache, pytest.KEY, 20)
         await lock.__aenter__()
-        assert await wrong_lock.__aexit__("exc_type", "exc_value", "traceback") == 0
+        assert await wrong_lock.__aexit__("exc_type", "exc_value", "traceback") is None
 
     @pytest.mark.asyncio
     async def test_float_lease(self, redis_cache):
         lock = RedLock(redis_cache, pytest.KEY, 0.1)
         await lock.__aenter__()
         await asyncio.sleep(0.2)
-        assert await lock.__aexit__("exc_type", "exc_value", "traceback") == 0
+        assert await lock.__aexit__("exc_type", "exc_value", "traceback") is None
 
 
 class TestMemcachedRedLock:
@@ -135,13 +144,13 @@ def lock(self, memcached_cache):
     async def test_release_wrong_token_succeeds_meh(self, lock):
         await lock.__aenter__()
         lock._value = "random"
-        assert await lock.__aexit__("exc_type", "exc_value", "traceback") == 1
+        assert await lock.__aexit__("exc_type", "exc_value", "traceback") is None
 
     @pytest.mark.asyncio
     async def test_release_wrong_client_succeeds_meh(self, memcached_cache, lock):
         wrong_lock = RedLock(memcached_cache, pytest.KEY, 20)
         await lock.__aenter__()
-        assert await wrong_lock.__aexit__("exc_type", "exc_value", "traceback") == 1
+        assert await wrong_lock.__aexit__("exc_type", "exc_value", "traceback") is None
 
     @pytest.mark.asyncio
     async def test_float_lease(self, memcached_cache):

tests/ut/test_lock.py

@@ -21,6 +21,7 @@ async def test_acquire(self, mock_cache, lock):
 
     @pytest.mark.asyncio
     async def test_release(self, mock_cache, lock):
+        mock_cache._redlock_release.return_value = True
         await lock._acquire()
         await lock._release()
         mock_cache._redlock_release.assert_called_with(
@@ -29,13 +30,11 @@ async def test_release(self, mock_cache, lock):
         assert pytest.KEY + '-lock' not in lock._EVENTS
 
     @pytest.mark.asyncio
-    async def test_second_release(self, mock_cache, lock):
-        await lock._acquire()
-        mock_cache._redlock_release.side_effect = [True, False]
-        assert pytest.KEY + '-lock' in lock._EVENTS
-        assert await lock._release() is True
+    async def test_release_no_acquire(self, mock_cache, lock):
+        mock_cache._redlock_release.return_value = False
+        assert pytest.KEY + '-lock' not in lock._EVENTS
+        await lock._release()
         assert pytest.KEY + '-lock' not in lock._EVENTS
-        assert await lock._release() is False
 
     @pytest.mark.asyncio
     async def test_context_manager(self, mock_cache, lock):
@@ -47,6 +46,13 @@ async def test_context_manager(self, mock_cache, lock):
             pytest.KEY + '-lock',
             lock._value)
 
+    @pytest.mark.asyncio
+    async def test_raises_exceptions(self, mock_cache, lock):
+        mock_cache._redlock_release.return_value = True
+        with pytest.raises(ValueError):
+            async with lock:
+                raise ValueError
+
     @pytest.mark.asyncio
     async def test_acquire_block_timeouts(self, mock_cache, lock):
         await lock._acquire()