-
Notifications
You must be signed in to change notification settings - Fork 154
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
UnicodeDecodeError when using MsgPackSerializer #834
Comments
By the way, I also tried to use |
Possibly the same as #515? If not, then it'd probably be a good idea to create a test in a PR that reproduces the problem in CI. |
Yeah, #515 is probably related, but import asyncio
from aiocache import MemcachedCache
from aiocache.serializers import PickleSerializer
async def test():
cache = MemcachedCache(
serializer = PickleSerializer(),
)
await cache.set("test_case", { "key1": "test", "key2": 3 })
test_case = await cache.get("test_case")
print(test_case)
if __name__ == "__main__":
asyncio.run(test()) This code works fine.
I'll try and add that. |
Acceptance tests for Please let me know if you have any insight on the issue, I'd be glad to help. |
I guess the design currently is that a serialiser should use strings? Though I don't understand how this serialiser got added when it doesn't work with anything... (the original tests don't actually test it with any caches: https://github.com/aio-libs/aiocache/pull/370/files#diff-14d53e5086ad36720b5073972f662da656b46898f2f8bdcdd06bb2fd9c2df9f5R86). I guess the simple fix for now is to add Then maybe a better refactoring can be done to avoid the overhead in future, though it's probably best we get #512 done before tackling that (which also requires some other tasks to be complete before that can be done: https://github.com/aio-libs/aiocache/milestone/8). |
I tried adding the following to diff --git a/aiocache/serializers/serializers.py b/aiocache/serializers/serializers.py
index 39a67e6..a3fcce9 100644
--- a/aiocache/serializers/serializers.py
+++ b/aiocache/serializers/serializers.py
@@ -184,7 +184,7 @@ class MsgPackSerializer(BaseSerializer):
:param value: obj
:returns: bytes
"""
- return msgpack.dumps(value)
+ return msgpack.dumps(value).decode("utf-8")
def loads(self, value):
"""
@@ -196,4 +196,4 @@ class MsgPackSerializer(BaseSerializer):
raw = False if self.encoding == "utf-8" else True
if value is None:
return None
- return msgpack.loads(value, raw=raw, use_list=self.use_list)
+ return msgpack.loads(value.encode("utf-8"), raw=raw, use_list=self.use_list) |
Yeah, that's what I was thinking. Thinking about it more, the problem is probably that we are decoding the raw bytes first, and that is actually in some binary form, so the decode operation won't work. I guess we'd need some way to disable the decoding (or make the decoding the responsibility of the serialiser maybe?). I suspect such changes won't be backwards compatible and will need to go to v1. |
Hi!
When using MsgPack as the (de)serializer for aiocache (no matter the backend), the following error occurs when reading an item from the cache:
(I'm using Python 3.12.2, aiocache 0.12.2, aiomcache 0.8.1, redis 4.6.0 and msgpack 1.0.8.)
The error seems to stem from the fact that most backends (prematurely?) decode values from the cache before passing them to the (de)serializer (see backends/memcached.py:24). Replacing this line with
return value
makes it work (for MsgPack, at least).But I guess simply removing the value decoding logic isn't a solution. If anyone can give me a clue of what's going wrong here (besides the
value.decode()
), I'll gladly create a merge request with (my attempt at) a fix.Minimal test case (using
MemcachedCache
, useRedisCache
if you wish to):Full error:
Thanks a lot in advance :)
The text was updated successfully, but these errors were encountered: