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

Room parameter of send() was silently renamed between v4 and v5. #1771

Closed
stackp opened this issue Jan 17, 2022 · 3 comments
Closed

Room parameter of send() was silently renamed between v4 and v5. #1771

stackp opened this issue Jan 17, 2022 · 3 comments
Assignees
Labels

Comments

@stackp
Copy link

stackp commented Jan 17, 2022

Greetings and thanks for your work !

After upgrading from 4 to 5, the following code started ignoring the room parameter and sending a message to everyone. Looking at the changelog, it appears that the room parameter has been renamed to.

I believe it would help avoid surprises if an error was thrown when room is passed. Or to treat room as an alias for to (like python-socketio) and issuing a deprecation warning.

from flask_socketio import SocketIO

socketio = SocketIO(path="/ws")

# [...]

socketio.send(
    values,
    json=True,
    room="room_name",
    skip_sid=skip_sid,
)

Here's the requirements.txt diff :

-Flask-SocketIO==4.2.1
+Flask-SocketIO==5.1.1
-python-engineio==3.12.1
+python-engineio==4.3.0
-python-socketio==4.5.1
+python-socketio==5.5.0

Thanks again,
Pierre

@miguelgrinberg
Copy link
Owner

You are correct that the room parameter was renamed to to a while ago, but I did not remove room, and did not intend for it to be ignored.

The to parameter is obtained as follows:

to = kwargs.pop('to', kwargs.pop('room', None))

Which seems correct. There also unit tests that use room instead of to, such as https://github.com/miguelgrinberg/Flask-SocketIO/blob/main/test_socketio.py#L501.

So I don't understand why this isn't working for you.

@stackp
Copy link
Author

stackp commented Jan 17, 2022

Thank you for your reply @miguelgrinberg.

Looking at the code, it looks like this is indeed handled correctly in socketio.send() but there is a subtle bug in socketio.SocketIO().send()

  1. self.send(..., room="name") inserts to=None in the kwargs ofself.emit() => kwargs={"to":None, "room": "name"})
  2. in self.emit() kwargs.pop('to', kwargs.pop('room', None)) returns None even though room == "name" because to does exists in kwargs

So, the bug in a nutshell:

# self.emit(..., to=None, **{"room": "name"})
>>> kwargs = {"to": None, "room": "name"}
>>> to = kwargs.pop('to', kwargs.pop('room', None))
>>> print(to)
None

Cheers! :)

@miguelgrinberg
Copy link
Owner

Okay, yeah, you did mention it was on send() and not on emit(). Sorry, I missed that. I'll correct this.

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