Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: raise granular exception types for lock errors #3222

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
* Raise granular exceptions when lock operations fail
* Update `ResponseT` type hint
* Allow to control the minimum SSL version
* Add an optional lock_name attribute to LockError.
Expand Down
19 changes: 12 additions & 7 deletions redis/asyncio/lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
from types import SimpleNamespace
from typing import TYPE_CHECKING, Awaitable, Optional, Union

from redis.exceptions import LockError, LockNotOwnedError
from redis.exceptions import (
IndefiniteLockError,
LockAquireError,
LockNotLockedError,
LockNotOwnedError,
)

if TYPE_CHECKING:
from redis.asyncio import Redis, RedisCluster
Expand Down Expand Up @@ -159,7 +164,7 @@ def register_scripts(self):
async def __aenter__(self):
if await self.acquire():
return self
raise LockError("Unable to acquire lock within the time specified")
raise LockAquireError("Unable to acquire lock within the time specified")

async def __aexit__(self, exc_type, exc_value, traceback):
await self.release()
Expand Down Expand Up @@ -249,7 +254,7 @@ def release(self) -> Awaitable[None]:
"""Releases the already acquired lock"""
expected_token = self.local.token
if expected_token is None:
raise LockError("Cannot release an unlocked lock")
raise LockNotLockedError("Cannot release an unlocked lock")
self.local.token = None
return self.do_release(expected_token)

Expand All @@ -275,9 +280,9 @@ def extend(
`additional_time`.
"""
if self.local.token is None:
raise LockError("Cannot extend an unlocked lock")
raise LockNotLockedError("Cannot extend an unlocked lock")
if self.timeout is None:
raise LockError("Cannot extend a lock with no timeout")
raise IndefiniteLockError("Cannot extend a lock with no timeout")
return self.do_extend(additional_time, replace_ttl)

async def do_extend(self, additional_time, replace_ttl) -> bool:
Expand All @@ -297,9 +302,9 @@ def reacquire(self) -> Awaitable[bool]:
Resets a TTL of an already acquired lock back to a timeout value.
"""
if self.local.token is None:
raise LockError("Cannot reacquire an unlocked lock")
raise LockNotLockedError("Cannot reacquire an unlocked lock")
if self.timeout is None:
raise LockError("Cannot reacquire a lock with no timeout")
raise IndefiniteLockError("Cannot reacquire a lock with no timeout")
return self.do_reacquire()

async def do_reacquire(self) -> bool:
Expand Down
15 changes: 15 additions & 0 deletions redis/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,21 @@ def __init__(self, message=None, lock_name=None):
self.lock_name = lock_name


class LockAquireError(LockError):
"Error acquring a lock in a given time"
...


class IndefiniteLockError(LockError):
"Error whilst trying to adjust lifetime of a lock that is indefinite"
...


class LockNotLockedError(LockError):
"Error whilst trying to perform an operation on an unlocked lock"
...


class LockNotOwnedError(LockError):
"Error trying to extend or release a lock that is (no longer) owned"
pass
Expand Down
27 changes: 20 additions & 7 deletions redis/lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
from types import SimpleNamespace, TracebackType
from typing import Optional, Type

from redis.exceptions import LockError, LockNotOwnedError
from redis.exceptions import (
IndefiniteLockError,
LockAquireError,
LockNotLockedError,
LockNotOwnedError,
)
from redis.typing import Number


Expand Down Expand Up @@ -157,7 +162,7 @@ def register_scripts(self) -> None:
def __enter__(self) -> "Lock":
if self.acquire():
return self
raise LockError(
raise LockAquireError(
"Unable to acquire lock within the time specified",
lock_name=self.name,
)
Expand Down Expand Up @@ -251,7 +256,9 @@ def release(self) -> None:
"""
expected_token = self.local.token
if expected_token is None:
raise LockError("Cannot release an unlocked lock", lock_name=self.name)
raise LockNotLockedError(
"Cannot release an unlocked lock", lock_name=self.name
)
self.local.token = None
self.do_release(expected_token)

Expand All @@ -276,9 +283,13 @@ def extend(self, additional_time: int, replace_ttl: bool = False) -> bool:
`additional_time`.
"""
if self.local.token is None:
raise LockError("Cannot extend an unlocked lock", lock_name=self.name)
raise LockNotLockedError(
"Cannot extend an unlocked lock", lock_name=self.name
)
if self.timeout is None:
raise LockError("Cannot extend a lock with no timeout", lock_name=self.name)
raise IndefiniteLockError(
"Cannot extend a lock with no timeout", lock_name=self.name
)
return self.do_extend(additional_time, replace_ttl)

def do_extend(self, additional_time: int, replace_ttl: bool) -> bool:
Expand All @@ -301,9 +312,11 @@ def reacquire(self) -> bool:
Resets a TTL of an already acquired lock back to a timeout value.
"""
if self.local.token is None:
raise LockError("Cannot reacquire an unlocked lock", lock_name=self.name)
raise LockNotLockedError(
"Cannot reacquire an unlocked lock", lock_name=self.name
)
if self.timeout is None:
raise LockError(
raise IndefiniteLockError(
"Cannot reacquire a lock with no timeout",
lock_name=self.name,
)
Expand Down
19 changes: 12 additions & 7 deletions tests/test_asyncio/test_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
import pytest
import pytest_asyncio
from redis.asyncio.lock import Lock
from redis.exceptions import LockError, LockNotOwnedError
from redis.exceptions import (
IndefiniteLockError,
LockAquireError,
LockNotLockedError,
LockNotOwnedError,
)


class TestLock:
Expand Down Expand Up @@ -125,7 +130,7 @@ async def test_context_manager(self, r):

async def test_context_manager_raises_when_locked_not_acquired(self, r):
await r.set("foo", "bar")
with pytest.raises(LockError):
with pytest.raises(LockAquireError):
async with self.get_lock(r, "foo", blocking_timeout=0.1):
pass

Expand All @@ -144,7 +149,7 @@ async def test_high_sleep_small_blocking_timeout(self, r):

async def test_releasing_unlocked_lock_raises_error(self, r):
lock = self.get_lock(r, "foo")
with pytest.raises(LockError):
with pytest.raises(LockNotLockedError):
await lock.release()

async def test_releasing_lock_no_longer_owned_raises_error(self, r):
Expand Down Expand Up @@ -183,13 +188,13 @@ async def test_extend_lock_float(self, r):

async def test_extending_unlocked_lock_raises_error(self, r):
lock = self.get_lock(r, "foo", timeout=10)
with pytest.raises(LockError):
with pytest.raises(LockNotLockedError):
await lock.extend(10)

async def test_extending_lock_with_no_timeout_raises_error(self, r):
lock = self.get_lock(r, "foo")
assert await lock.acquire(blocking=False)
with pytest.raises(LockError):
with pytest.raises(IndefiniteLockError):
await lock.extend(10)
await lock.release()

Expand All @@ -211,13 +216,13 @@ async def test_reacquire_lock(self, r):

async def test_reacquiring_unlocked_lock_raises_error(self, r):
lock = self.get_lock(r, "foo", timeout=10)
with pytest.raises(LockError):
with pytest.raises(LockNotLockedError):
await lock.reacquire()

async def test_reacquiring_lock_with_no_timeout_raises_error(self, r):
lock = self.get_lock(r, "foo")
assert await lock.acquire(blocking=False)
with pytest.raises(LockError):
with pytest.raises(IndefiniteLockError):
await lock.reacquire()
await lock.release()

Expand Down
22 changes: 14 additions & 8 deletions tests/test_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

import pytest
from redis.client import Redis
from redis.exceptions import LockError, LockNotOwnedError
from redis.exceptions import (
IndefiniteLockError,
LockAquireError,
LockError,
LockNotLockedError,
LockNotOwnedError,
)
from redis.lock import Lock

from .conftest import _get_client
Expand Down Expand Up @@ -129,7 +135,7 @@ def test_context_manager_blocking_timeout(self, r):

def test_context_manager_raises_when_locked_not_acquired(self, r):
r.set("foo", "bar")
with pytest.raises(LockError):
with pytest.raises(LockAquireError):
with self.get_lock(r, "foo", blocking_timeout=0.1):
pass

Expand All @@ -148,7 +154,7 @@ def test_high_sleep_small_blocking_timeout(self, r):

def test_releasing_unlocked_lock_raises_error(self, r):
lock = self.get_lock(r, "foo")
with pytest.raises(LockError):
with pytest.raises(LockNotLockedError):
lock.release()

def test_releasing_lock_no_longer_owned_raises_error(self, r):
Expand Down Expand Up @@ -187,13 +193,13 @@ def test_extend_lock_float(self, r):

def test_extending_unlocked_lock_raises_error(self, r):
lock = self.get_lock(r, "foo", timeout=10)
with pytest.raises(LockError):
with pytest.raises(LockNotLockedError):
lock.extend(10)

def test_extending_lock_with_no_timeout_raises_error(self, r):
lock = self.get_lock(r, "foo")
assert lock.acquire(blocking=False)
with pytest.raises(LockError):
with pytest.raises(IndefiniteLockError):
lock.extend(10)
lock.release()

Expand All @@ -215,13 +221,13 @@ def test_reacquire_lock(self, r):

def test_reacquiring_unlocked_lock_raises_error(self, r):
lock = self.get_lock(r, "foo", timeout=10)
with pytest.raises(LockError):
with pytest.raises(LockNotLockedError):
lock.reacquire()

def test_reacquiring_lock_with_no_timeout_raises_error(self, r):
lock = self.get_lock(r, "foo")
assert lock.acquire(blocking=False)
with pytest.raises(LockError):
with pytest.raises(IndefiniteLockError):
lock.reacquire()
lock.release()

Expand All @@ -234,7 +240,7 @@ def test_reacquiring_lock_no_longer_owned_raises_error(self, r):

def test_context_manager_reacquiring_lock_with_no_timeout_raises_error(self, r):
with self.get_lock(r, "foo", timeout=None, blocking=False) as lock:
with pytest.raises(LockError):
with pytest.raises(IndefiniteLockError):
lock.reacquire()

def test_context_manager_reacquiring_lock_no_longer_owned_raises_error(self, r):
Expand Down