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

4.4.10 release makes ws connection even when raising ConnectionRefusedError #391

Closed
YuqiaoS opened this issue Dec 3, 2019 · 9 comments
Closed
Assignees
Labels

Comments

@YuqiaoS
Copy link

YuqiaoS commented Dec 3, 2019

Since this is actually an separate issue, even though on the same topic, I'm creating a new issue for it.

Open browser inspector tool, go to network->ws, and you will see a websocket connection even when you raise the ConnectionRefusedError. It stays half-connected as you can see ping events, but aren't able to send any messages. On trying to connect using the js client socket again after I login through http (flask-session server side setup), the client socket cannot be used to connect as it is still connected.

python

@socketio.on('connect')
def connect():
    # if not current_user.is_authenticated:
    raise ConnectionRefusedError()

js

import io from 'socket.io-client';
socket = io()

// this used to be hit when on python-socketio 4.3.1
socket.on('connect_error', function(error){
  console.log(error)
}
// this is now hit on 4.4.1
socket.on('error', function(error){
  console.log(error)
}

// on connect, disconnect ...
@miguelgrinberg miguelgrinberg self-assigned this Dec 3, 2019
@zamazan4ik
Copy link

I've met the same issue. @miguelgrinberg is there any plans for providing a fix for this? Or can you suggest any workaround, how to fix/avoid it manually without patching python-socketio library?

Thank you!

@miguelgrinberg
Copy link
Owner

Haven't had time to prepare a fix for this issue yet, but it is coming.

@acnebs
Copy link

acnebs commented Apr 20, 2020

I'm coming up against this as well. In the meantime, should I just downgrade to 4.3.x?

Actually, I don't even seem to be able to listen for the error events or connect_error events. If I don't throw an error in my server-side connect handler, my client works just fine. If I do throw an error, it doesn't seem to get any event and it does the "sort of pending connection" thing explained above.

@miguelgrinberg
Copy link
Owner

Unfortunately this is proving to be a very tricky issue to address due to different behaviors by different Socket.IO clients. Going back to the older way will just bring a different (yet related) issue back, so I'll keep working on this and try to get a proper fix in place that works for all clients. If the older release works better for you, then by all means downgrade.

@acnebs
Copy link

acnebs commented Apr 20, 2020

Gotcha, thanks for working on it! Am I correct in the understanding that currently, if we want to stop the client from connecting in a connect handler (for example for the purpose of auth), we should not throw any sort of exception, and instead just return False in the connect handler?

And if we want the client to receive some sort of error message about what is going on when we deny their connection, there isn't much we can do as raising an exception doesn't seem to work consistently?

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Apr 20, 2020

@acnebs both returning False and raising the exception should stop the connection. The latter allows you to provide some extra info to the client, but other than that it is the same as the former.

In the current release the server does not force the connection to end, so it is up to the client to close it. Many clients do not close it on their own, so the connection remains, even though the server doesn't use it for anything.

In the previous releases the server forced the connection to close, but this prevented that extra error information you can pass in the raised exception from ever reaching the client.

I need to come up with a fix that returns error information to the client, and only once the client received it it closes the connection to force the client out.

@acnebs
Copy link

acnebs commented Apr 20, 2020

Right, that makes sense, thanks for the explanation.

The issue I'm having is that I don't know where I'm supposed to handle the raised exception on the client-side (when using the JS client). Obviously the JS library isn't your purview, but one generally hopes that the various SocketIO libs should seamlessly interoperate with each other. But when I read through the python-socketio client docs, I couldn't seem to find much guidance there either for this situation.

In the python-socketio client, is it the connect_error() handler that handles both the return False method and the raise ConnectionRefusedError("info") method for short-circuiting a connection? And am I right in thinking that the args in connect_error() are the args passed into the ConnectionRefusedError?

Because when using the JS client lib, my error handler is triggered when I return False server-side in both 4.3.x and 4.5.x. Meanwhile, neither my client-side error nor my connect_error handlers are being triggered when I call raise ConnectionRefusedError() on the server side (in both mentioned versions). The JS client has "support" for connect_errors, it just seems like raising an exception in the python-socketio server isn't doing what the JS client expects.

@miguelgrinberg
Copy link
Owner

This is actually the core of the problem I'm trying to solve. I haven't looked specifically at the JS client yet, but yes, the idea is that the connection error should be invoked when the server declines a connection.

@skamensky
Copy link

@acnebs did you ever resolve the JS side of this issue? I'm still seeing the behavior of connections being closed when returning a simple False but connections staying alive when raising a ConnectionRefusedError within flask. Also as it seems other people experienced, t only works on socket.on('error') not on socket.on('connection_error'). Did you find a way of communicating to the client why the connection failed? Or did you have to create custom client and server events to handle that?

Because when using the JS client lib, my error handler is triggered when I return False server-side in both 4.3.x and 4.5.x. Meanwhile, neither my client-side error nor my connect_error handlers are being triggered when I call raise ConnectionRefusedError() on the server side (in both mentioned versions). The JS client has "support" for connect_errors, it just seems like raising an exception in the python-socketio server isn't doing what the JS client expects.

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

5 participants