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

'socket' object has no attribute 'pending' #857

Closed
j-linds opened this issue Sep 7, 2022 · 19 comments
Closed

'socket' object has no attribute 'pending' #857

j-linds opened this issue Sep 7, 2022 · 19 comments

Comments

@j-linds
Copy link

j-linds commented Sep 7, 2022

Hi,

Since version 1.4.0 we have been experiencing an issue using WebSocketApp and obtaining this error:

'socket' object has no attribute 'pending'

Downgrading to an older version, such as pip install websocket-client==1.3.3, resolves the problem.

Example code:

#!/usr/bin/env python3
import websocket
import ssl
import json

message_to_send = {"key": "value"}

def on_message(ws, message):
    print("response: " + str(message))
    ws.close()

def on_error(ws, error):
    print("error: " + str(error))

def on_close(ws, *args, **kwargs):
    print("Closed")

def on_open(ws):
    msg = json.dumps(message_to_send)
    print(msg)
    ws.send(msg)

ws = websocket.WebSocketApp(
    "ws://localhost:8089/",
    on_message=on_message,
    on_error=on_error,
    on_close=on_close,
)
ws.on_open = on_open
ws.run_forever(sslopt={"cert_reqs": ssl.CERT_NONE})

Some example debugging information:

--- request header ---
GET / HTTP/1.1
Upgrade: websocket
Host: localhost:8089
Origin: http://localhost:8089
Sec-WebSocket-Key:
Sec-WebSocket-Version: 13
Connection: Upgrade

-----------------------
--- response header ---
HTTP/1.1 101 Switching Protocols
Connection: Upgrade
Sec-WebSocket-Accept:
Server: server
Upgrade: websocket
-----------------------

websocket connected
'socket' object has no attribute 'pending' - goodbye
error: 'socket' object has no attribute 'pending'

Have searched online/FAQ etc and cannot see anything related to the issue.

Many thanks for the help.

@engn33r
Copy link
Collaborator

engn33r commented Sep 7, 2022

The same issue was reported in this comment and follow-up comments: #854 (comment)

I don't have more information other than what is mentioned there at the moment but will update with details when I do.

@bubbleboy14
Copy link
Collaborator

Hello @j-linds , shucks sorry to hear. I ran your test against a wss server I run out in the wild, and I didn't notice any problems, or see the error you encountered.

Could you please provide your server code as well?

@j-linds
Copy link
Author

j-linds commented Sep 8, 2022

@bubbleboy14 This specific server code I cannot share, but I'll try to create a reproducible example and report back.

For reference I am running Python 3.10 on Ubuntu 22.04 installed via Pyenv.

@cyr123
Copy link

cyr123 commented Sep 10, 2022

I ran into the same issue with https://github.com/jellyfin/jellyfin-mpv-shim (v2.2.0, talking to jellyfin server v10.8.3)

Also running Python 3.10 on Ubuntu 22.04, downgrading to websocket-client 1.3.3 fixed the issue for me as well.

@engn33r
Copy link
Collaborator

engn33r commented Sep 14, 2022

For anyone encountering this issue, can you share whether specifying rel as dispatcher in run_forever like found in the long-lived connection example improves things?

@martydingo
Copy link

Ah I finally found the right issue that's tracking this problem, this also occurs in https://github.com/screepers/python-screeps.

I've tracked it down (and bypassed for testing purposes) to lines 80-95, within _app.py, removing the following lines fixes the issue for me, however my use case only requires non-SSL connections:

class SSLDispatcher(DispatcherBase):
    """
    SSLDispatcher
    """
    def read(self, sock, read_callback, check_callback):
        while self.app.keep_running:
            r = self.select()
            if r:
                if not read_callback():
                    break
            check_callback()

    def select(self):
        sock = self.app.sock.sock
-       if sock.pending():
-           return [sock,]

@martydingo
Copy link

martydingo commented Sep 20, 2022

For anyone encountering this issue, can you share whether specifying rel as dispatcher in run_forever like found in the long-lived connection example improves things?

adding the parameter dispatcher=rel does seem to work at first, however specifying this parameter causes no output but no loop, I see a clean exit of the application, where I expect to see websocket messages printed to stdout.

Commenting the highlighted code above however does fix the issue entirely.

@bubbleboy14
Copy link
Collaborator

Hey @martydingo.

adding the parameter dispatcher=rel does seem to work at first, however specifying this parameter causes no output but no loop, I see a clean exit of the application, where I expect to see websocket messages printed to stdout.

Ah yes, if you pass dispatcher=rel, you also have to (probably on the last line of your start script) call rel.dispatch(). Also, it's noteworthy that passing an alternate dispatcher is only necessary if you want to run multiple WebSocketApps (without using threads) in the same process.

Commenting the highlighted code above however does fix the issue entirely.

@martydingo, would you be willing to provide a short example of server code that reproduces the issue? Since you mentioned your application doesn't require SSL, I can modify it so we can test both (which seems necessary before removing code from the SSLDispatcher).

Is anyone still experiencing this problem on 1.4.1? Thanks all!

@QuinnDamerell
Copy link
Contributor

Is there a fix or work around for this? A lot of my users are hitting this issue for my project. :(

@rm-you
Copy link

rm-you commented Oct 18, 2022

So, I had this problem and I tracked it down in my own app to a specific new behavior around detection of whether the socket should be SSL or not.
I had: self._socket.run_forever(sslopt={"cert_reqs": ssl.CERT_NONE}) but I was not actually connecting to an SSL socket necessarily, it is based on config the user passes. When I stop passing that, it works (on non-SSL connections).

This client used to detect SSL/not based on whether the socket itself detected SSL in use, but now it bases it solely on whether any sslopt exists. See: v1.3.3...v1.4.1#diff-9798fd5aec9260ec26bdb6ac875c3f9948ea29ddc48bc4bfc268c1ad3c135411L397

Now, the socket isn't initialized yet at the point where it needs to check, which I guess is why it's just using the truthiness of sslopt.
I wish this could be done differently, but it is what it is. For now, I'll work around it by doing my own parsing of the configured websocket URL to see if it's supposed to be SSL ("ws://" vs "wss://") but I wish I didn't have to. 😢

@rm-you
Copy link

rm-you commented Oct 18, 2022

Ah I finally found the right issue that's tracking this problem, this also occurs in https://github.com/screepers/python-screeps.

I've tracked it down (and bypassed for testing purposes) to lines 80-95, within _app.py, removing the following lines fixes the issue for me, however my use case only requires non-SSL connections:

@martydingo Looks like you have the same issue I did, always passing sslopt even when the connection may not be SSL:
https://github.com/screepers/python-screeps/blob/b4dd23c4d7d987ea73bfc53faca445abd4f5b58d/screepsapi/screepsapi.py#L544-L546

@garethsb
Copy link

garethsb commented Nov 1, 2022

I had same issue as @rm-you, code that always included a Truthy sslopt whether doing Secure WebSocket or not. I already had a urlparse nearby so I've used the same workaround of checking url.scheme == "wss" in the calling code.

@bubbleboy14
Copy link
Collaborator

@j-linds @cyr123 @martydingo @QuinnDamerell @rm-you @garethsb try this PR:

#875

Does it fix things for you guys? Thoughts @engn33r ?

@rm-you
Copy link

rm-you commented Nov 2, 2022

I imagine it will fix things for me, but I wish I was more certain that the URL using ws/wss is guaranteed to be accurate. Is this 100% required by the protocol or is it just convention? Also, may want to do the check along with a lower() or something for the random case where someone passes a mixed or upper case URL?

@bubbleboy14
Copy link
Collaborator

Good call @rm-you, I added a lower() to the PR.

When I pass a url without a (ws or wss) protocol, I get "hostname is invalid - goodbye". Fairly sure this is necessary.

Hope #875 fixes this issue.

@engn33r
Copy link
Collaborator

engn33r commented Nov 2, 2022

@rm-you The ws/wss at the start of the URL is just like http/https in front of most web URLs. Without ws/wss/http/https, it is unclear what protocol should be used. If you use a https URL for a website that only supports http, your browser will return an error because the default port for https is port 443 while the default port for http is port 80. The ws/wss should be included in the URL to specify whether the websocket is encrypted (wss) or unencrypted (ws). Like http/https, ws is often on port 80 and wss on port 443. Example:

if port == 80 or port == 443:

Code like this example shows that websocket-client expects the user to specify the proper ws/wss protocol in the URL (just like web browser expect the user to specify http/https correctly), so relying on ws/wss in the URL instead of a sslopt argument like #875 does seems to me the right approach. I will merge 875 in about 48 hrs if there are no objections.

And to answer the question about handling uppercase/mixed URLs, urlparse is used which handles such cases

parsed = urlparse(url, scheme="http")

@rm-you
Copy link

rm-you commented Nov 2, 2022

@rm-you The ws/wss at the start of the URL is just like http/https in front of most web URLs. Without ws/wss/http/https, it is unclear what protocol should be used. If you use a https URL for a website that only supports http, your browser will return an error because the default port for https is port 443 while the default port for http is port 80. The ws/wss should be included in the URL to specify whether the websocket is encrypted (wss) or unencrypted (ws).

Sure, I get that, but we're at a bit lower level here. The browser needs that because it supports a ton of protocols and needs to know which is being requested given only a single input (the URL), whereas we're literally in code creating a socket connection object, it shouldn't need the protocol explicitly defined, just a host and port, since we've already passed the protocol selection stage. I doubt anyone would instantiate a websocket class expecting it to need to decide between HTTP and WS. 😅
That said, WS and WSS is useful and makes sense, just wanted to make sure that was really spec and not just a convention (haven't used websockets more than a few times and never really bothered to read the RFC, oof).

That PR looks good to me and will resolve my issue (I can revert my own workaround if I get to it). Thanks!

@j-linds
Copy link
Author

j-linds commented Nov 3, 2022

@j-linds @cyr123 @martydingo @QuinnDamerell @rm-you @garethsb try this PR:

#875

Does it fix things for you guys? Thoughts @engn33r ?

@bubbleboy14 That works on my end.

@engn33r
Copy link
Collaborator

engn33r commented Nov 4, 2022

Thanks for the help on troubleshooting this one. Should be fixed in #875 and version 1.4.2 👍

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

No branches or pull requests

8 participants