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

Callbacks from multiple clients fail when using RabbitMQ #1152

Closed
bartvanb opened this issue Mar 13, 2023 · 3 comments
Closed

Callbacks from multiple clients fail when using RabbitMQ #1152

bartvanb opened this issue Mar 13, 2023 · 3 comments
Labels

Comments

@bartvanb
Copy link

Describe the bug
I have a python-socketIO server with multiple python-socketIO clients. The server pings all clients in a particular room, and they respond with an identifier. The server then registers this client as being online via a callback.

When the server is coupled to a RabbitMQ queue, only the first callback is registered. All other callbacks lead to the following message:

socketio.server - WARNING  - Unknown callback received, ignoring.

I found out that this is because RabbitMQ passes the name of the room as sid instead of the actual sid, which causes the callback to be deleted after the first time - see Additional context.

To Reproduce

  • Start RabbitMQ - we used docker run -p 5672:5672 rabbitmq:latest to get a RabbitMQ queue on port 5672 on the host machine
  • Run SocketIO server with message_queue=amqp://...
  • Server sends event to a room with multiple clients.
  • Each client should respond to the callback
  • First callback succeeds, next callbacks fail

I kept this brief - let me know if a full MWE is helpful.

Expected behavior
Callbacks are all processed correctly, instead of only the first one.

Logs

2023-03-13 09:56:46 - socketio.server - INFO     - emitting event "ping" to all_nodes [/tasks]
2023-03-13 09:56:46 - socketio.server - INFO     - pubsub message: emit
2023-03-13 09:56:46 - socketio.server - INFO     - received ack from OK71xHKTNlLJXoGKAAAB [/tasks]
2023-03-13 09:56:46 - socketio.server - INFO     - received ack from eVeuiH73Q1UxB8yTAAAD [/tasks]
2023-03-13 09:56:46 - socketio.server - INFO     - pubsub message: callback
2023-03-13 09:56:46 - socketio.server - INFO     - pubsub message: callback
2023-03-13 09:56:46 - socketio.server - WARNING  - Unknown callback received, ignoring.

Additional context
I think I found what the issue is, but have not been able to solve it.

In trigger_callback (here), an sid is passed to the callback event. When not using RabbitMQ, the sid is a random string (e.g. OK71xHKTNlLJXoGKAAAB), as expected. However, when using RabbitMQ, the sid is the room name (in my case, all_nodes).

After a callback is executed, trigger_callback() deletes the callback from a callback dictionary based on its sid. Since RabbitMQ passes the room name as sid, it is not unique and is therefore unavailable when the second callback is processed.

I tried to trace back further to find why the room name is passed as sid. Via KombuManager._listen() I finally ended up in a _receive() function in the kombu package (here). There, I can print the RabbitMQ message as follows:

>> print(pickle.loads(message.payload))
{'method': 'callback', 'host_id': 'b4bfb134fcef42028ea289ecc98ca0b2', 
'sid': 'all_nodes', 'namespace': '/tasks', 'id': 6, 'args': (3,)}

So I guess it may very well be an issue in either Kombu or RabbitMQ, but as it is rather entangled with python-socketio, I decided to post it here first.

More context:

  • I asked this Stackoverflow question about this recently.
  • We are planning work around this issue by sending socket events from the clients to the server. That way we avoid callbacks altogether.
  • version info:
    Flask-SocketIO==5.3.2 (server)
    python-socketio==5.7.2 (clients)
@miguelgrinberg
Copy link
Owner

Callbacks are not supported when sending to more than one clients. This has been the case since the early days of this project and is not related to RabbitMQ, it is a general limitation.

From the docs:

SCR-20230313-gq8

@bartvanb
Copy link
Author

bartvanb commented Mar 13, 2023

Right, clear. I did not read that part of the docs closely.
This part of the docs suggested to me that it is possible but not recommended. I assumed this was because of potential performance issues which I didn't think relevant in my case. Maybe you could reword that?

image

@miguelgrinberg
Copy link
Owner

@bartvanb sorry about that, I did not even realize that was in the documentation. I have now made a correction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants