Skip to content

Raise NotFoundError with additional info#13

Merged
Nottezz merged 14 commits intomainfrom
feature/not-found-error
Jul 29, 2025
Merged

Raise NotFoundError with additional info#13
Nottezz merged 14 commits intomainfrom
feature/not-found-error

Conversation

@Nottezz
Copy link
Collaborator

@Nottezz Nottezz commented Jul 8, 2025

Resolve #11

self._pop_item(key)
logger.debug("Element %s removed from cache - TTL expired", key)
return None
raise NotFoundError(key, message="Key removed from cache - TTL expired")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cюда может быть другой эксепшен сделать, связанный с TTL?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно да, от этого ексепшена сделать еще один, который будет использоваться когда значение есть но протухло

@Nottezz Nottezz requested a review from chud0 July 8, 2025 07:42
pass


class NotFoundError(StorageError):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше что то типа NotFoundStorageError или что то такое


try:
result = await storage.get(cache_key)
except NotFoundError as e:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

стоит ли здесь ловить? может пусть летит до мидлвари? как будто код проще тогда будет, без проверок на None

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Пока делал думал об этом.
А точно ли будет всё это долетать до мидлваря и информировать что с данными что-то не так.

self._pop_item(key)
logger.debug("Element %s removed from cache - TTL expired", key)
return None
raise NotFoundError(key, message="Key removed from cache - TTL expired")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно да, от этого ексепшена сделать еще один, который будет использоваться когда значение есть но протухло

@chud0
Copy link
Owner

chud0 commented Jul 24, 2025

bugbot run

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bugbot free trial expires on July 29, 2025
Learn more in the Cursor dashboard.

)
return None

return self._serializer.loads(raw_data)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Redis Storage Fails on Deserialization Errors

The RedisStorage.get method no longer handles deserialization errors. Previously, exceptions from self._serializer.loads(raw_data) (e.g., ValueError, JSONDecodeError) were caught, logged as warnings, and None was returned. Now, these exceptions propagate uncaught, potentially crashing the application when corrupted data is encountered in the cache, as the calling code expects NotFoundError for missing keys but not other serialization failures.

Locations (1)

Fix in CursorFix in Web

assert result is None # Элемент должен быть удален
with pytest.raises(NotFoundError):
result = await storage.get("test_key")
assert result is None # Элемент должен быть удален
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Unreachable Assertion in Exception Handling

The assert result is None statement on line 102 is unreachable code. It is located within a pytest.raises(NotFoundError) context, meaning that if await storage.get("test_key") raises the expected NotFoundError, execution exits the block, preventing the assertion from being reached. If no exception is raised, pytest.raises will fail the test before the assertion is executed.

Locations (1)

Fix in CursorFix in Web

full_key = self._full_key(key)

if not await self._storage.exists(full_key):
raise TTLExpiredStorageError(full_key)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сюда добавил проверку на существование ключа, аналогично тому, что сделано в in_memory решении. Если это избыточно и хватит простой проверки на наличие данных, которая ниже ( 80 строчка ), то уберу.

@Nottezz Nottezz requested a review from chud0 July 27, 2025 18:45
@Nottezz Nottezz merged commit b081daa into main Jul 29, 2025
3 checks passed
@Nottezz Nottezz deleted the feature/not-found-error branch July 29, 2025 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Raise NotFoundError with additional info in storage instead of returning None

2 participants

Comments