Skip to content

Conversation

@phi-friday
Copy link
Contributor

This change adds two new parameters, dumps_default and ext_hook, to enable support for msgpack ext type.
With these arguments, MsgPackPacket can encode and decode values using msgpack ext type.

@phi-friday
Copy link
Contributor Author

The coverage action is failing for the following reason, but it doesn't seem to be caused by the changes in the PR.

[2025-11-02T07:35:13.570Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 429 - {"message":"Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 685s."}

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Nov 2, 2025

Looks good, thanks!

But how do you intend these new arguments will be passed when used in a complete application? Do you have any example code that shows how the integration works?

@phi-friday
Copy link
Contributor Author

Looks good, thanks!

But how do you intend these new arguments will be passed when used in a complete application? Do you have any example code that shows how the integration works?

I’m not sure I fully understand exactly what you need. Do you mean a sample code like the one below?

from __future__ import annotations

from typing import TYPE_CHECKING, Any, override
from uuid import UUID

import msgpack
import socketio
from socketio.async_manager import AsyncManager
from socketio.msgpack_packet import MsgPackPacket as SocketIOMsgPackPacket

if TYPE_CHECKING:
    from collections.abc import Buffer, Mapping


USERS: Mapping[UUID, Any] = {}


class MsgPackPacket(SocketIOMsgPackPacket):
    @override
    def encode(self) -> Any:
        return msgpack.dumps(self._to_dict(), default=default_encode)

    @override
    def decode(self, encoded_packet: Buffer) -> None:
        decoded = msgpack.loads(encoded_packet, ext_hook=ext_hook)
        self.packet_type = decoded["type"]
        self.data = decoded.get("data")
        self.id = decoded.get("id")
        self.namespace = decoded["nsp"]


def default_encode(value: Any) -> Any:
    match value:
        case UUID():
            return msgpack.ExtType(0, value.bytes)
        case _:
            return value


def ext_hook(code: int, data: bytes) -> Any:
    match code:
        case 0:
            return UUID(bytes=data)
        case _:
            error_msg = f"Unknown msgpack ext code: {code}"
            raise ValueError(error_msg)


sio = socketio.AsyncServer(
    cors_allowed_origins=[],
    async_mode="asgi",
    always_connect=True,
    client_manager=AsyncManager(),
    serializer=MsgPackPacket,
)


@sio.on("user-list")
async def user_list() -> None:
    output = {"user_ids": list(USERS)}
    await sio.emit("user-list", output)

I can’t share the code here, but we use socketio when the JS frontend and Python backend exchange messages, and we pass the uuid value using ext_type. The code above is a simplified version of that backend code.

@phi-friday
Copy link
Contributor Author

Ah — after writing the sample code I realized that because this class is being passed, it's not possible to provide arguments to its __init__. The places that use it will also need to be modified, or an alternative approach is required. Simply changing this class alone doesn't solve the problem.

@phi-friday
Copy link
Contributor Author

Ah — after writing the sample code I realized that because this class is being passed, it's not possible to provide arguments to its __init__. The places that use it will also need to be modified, or an alternative approach is required. Simply changing this class alone doesn't solve the problem.

I think if the user uses functools.partial, no modifications should be necessary.

@miguelgrinberg
Copy link
Owner

Partial works, but I don't see it as a good solution. If anything it would make every packet creation slower.

I have to think about this. I think passing these new arguments in the constructor of the packet is not the most usable solution. These have to be configured elsewhere and globally.

@phi-friday
Copy link
Contributor Author

I thought partial might work after looking at how the packet_class attribute is used. If you think this should be handled in this PR, I'll add a private function on BaseServer to create packets and update every place that creates packets. However, in that case we'd need to add parameters like serializer_args to BaseServer.__init__.

@phi-friday
Copy link
Contributor Author

I thought partial might work after looking at how the packet_class attribute is used. If you think this should be handled in this PR, I'll add a private function on BaseServer to create packets and update every place that creates packets. However, in that case we'd need to add parameters like serializer_args to BaseServer.__init__.

Shall I proceed with this task?

@phi-friday
Copy link
Contributor Author

It looks like partial will cause issues because of the code below. I'll add the private function I mentioned earlier.

        if json is not None:
            self.packet_class.json = json

@miguelgrinberg
Copy link
Owner

I've been thinking about this, and I'd like to have a simpler interface. For example:

serializer=MsgPackPacket  # use with default options
serializer=MsgPackPacket.configure(default=my_default, ext_hook=my_ext_hook)  # custom configuration

This makes the configuration explicit, and visible to IDEs and type checkers, versus the serializer_args approach which is going to be harder for people to figure out, and not possible to type check for those interested in that.

Unfortunately implementing my the above pattern is not trivial, as it requires generating new classes on the fly. I'll see if I can modify this PR to do it over the next few days.

@phi-friday
Copy link
Contributor Author

I've been thinking about this, and I'd like to have a simpler interface. For example:

serializer=MsgPackPacket  # use with default options
serializer=MsgPackPacket.configure(default=my_default, ext_hook=my_ext_hook)  # custom configuration

This makes the configuration explicit, and visible to IDEs and type checkers, versus the serializer_args approach which is going to be harder for people to figure out, and not possible to type check for those interested in that.

Unfortunately implementing my the above pattern is not trivial, as it requires generating new classes on the fly. I'll see if I can modify this PR to do it over the next few days.

Are you considering it as a class method of the Packet class? Or are you considering it as a class method of the MsgPackPacket class?

@miguelgrinberg
Copy link
Owner

I'm thinking the configure method can be added to any class that needs configuration. For Packet it could be used to specify the JSON encoder/decoder to use, for example. This is handled in a way that isn't great right now.

@phi-friday
Copy link
Contributor Author

I'm thinking the configure method can be added to any class that needs configuration. For Packet it could be used to specify the JSON encoder/decoder to use, for example. This is handled in a way that isn't great right now.

I implemented it roughly as a class method. Would this direction work?

@miguelgrinberg
Copy link
Owner

@phi-friday I appreciate you trying to implement what I'm saying, but I don't want you to waste time before I really figure out how I want this to work. I just wanted to provide you an update and to let you know that I needed to think some more about this.

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.

2 participants