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

Release 1.0.0 caused an AttributeError: 'NoneType' object has no attribute 'settimeout' on websocketapp.close() #694

Closed
PFaurel opened this issue May 18, 2021 · 16 comments

Comments

@PFaurel
Copy link

PFaurel commented May 18, 2021

Hi,

With the 1.0.0 release, a major update in WebSocket.close() function in _core.py was done and it causes an AttributeError when we try to close WebSocketApp.

This issue is a side effect of an update in websocket-client/_core.py file

Initially, the code of WebSocket.close() function was:

    def close(self, status=STATUS_NORMAL, reason=six.b(""), timeout=3):
            ....
            try:
                ....
                self.sock.settimeout(sock_timeout)
                self.sock.shutdown(socket.SHUT_RDWR)
            except:
                pass

Now, exceptions are no more catched ... except for Mac OS (!?!) :

    def close(self, status=STATUS_NORMAL, reason=bytes('', encoding='utf-8'), timeout=3):
            ....
            try:
                ....
                self.sock.settimeout(sock_timeout)
                self.sock.shutdown(socket.SHUT_RDWR)
           except OSError:  # This happens often on Mac
                pass
            except:
                raise

I found one reason that could cause self.sock set to None :
when frame is decoded (line 479: frame = self.recv_frame()),
if there is any error during decoding, the web socket is closed and set to None:

    def _recv(self, bufsize):
        try:
            return recv(self.sock, bufsize)
        except WebSocketConnectionClosedException:
            if self.sock:
                self.sock.close()
            self.sock = None
            self.connected = False
            raise

I suppose our issue isn't new but was hidden by the try / except pass until 1.0.0 release.

Could you test self.sock before trying to set timeout and call shutdown() ?

@eirikur-grid
Copy link

Today we are seeing sporadic failures on our CI system. They seem to stem from the same regression.

Traceback (most recent call last):
  File "/app/.ci/socketio_smoketest.py", line 41, in <module>
    if test():
  File "/app/.ci/socketio_smoketest.py", line 34, in test
    sio.disconnect()
  File "/usr/local/lib/python3.9/site-packages/socketio/client.py", line 437, in disconnect
    self.eio.disconnect(abort=True)
  File "/usr/local/lib/python3.9/site-packages/engineio/client.py", line 219, in disconnect
    self.ws.close()
  File "/usr/local/lib/python3.9/site-packages/websocket/_core.py", line 491, in close
    self.sock.settimeout(sock_timeout)
AttributeError: 'NoneType' object has no attribute 'settimeout'

@engn33r
Copy link
Collaborator

engn33r commented May 18, 2021

Can you provide a minimum reproducible example as outlined in the contributing guidelines?

I suspected that issues might arise due to this change from commit 13e83b4, but because exceptions were only passed before, I thought this change would help clarify where the exceptions were coming from so that the code could be made more robust in the long run. The unit tests for this project are currently all passing, so I need to better understand what leads to these exceptions. I do agree that short-term issues might be a problem and I will work to address them and build them into unit tests, or possibly revert to the old solution of passing all exceptions.

@steinnes
Copy link

The failure @eirikur-grid mentioned originates in a fairly primitive smoketest script, I'll share the code here:

import sys
import time

import socketio


def test():
    sio = socketio.Client(logger=True, engineio_logger=True)
    connected = False
    namespace = "/change-notifications"

    def connect():
        nonlocal connected
        connected = True
        print("Connected")

    def disconnect():
        print("Disconnecting")

    sio.on("connect", connect, namespace=namespace)
    sio.on("disconnect", disconnect, namespace=namespace)
    sio.connect(
        "ws://localhost:5001", namespaces=[namespace], socketio_path="socket.io5", transports=["websocket"]
    )

    for tries in range(10):
        if not connected:
            print(f"Not connected yet, sleeping for 1s")
            time.sleep(1)
        else:
            break
    print("About to disconnect")
    sio.disconnect()
    sio.wait()
    print("Done disconnecting")
    return connected


if __name__ == "__main__":
    if test():
        sys.exit(0)
    else:
        sys.exit(1)

The output from this script does suggest that it may not be using the socketio client correctly, at least the disconnection log output from the loggers is not confidence-inspiring:


Attempting WebSocket connection to ws://localhost:5001/socket.io5/?transport=websocket&EIO=4
WebSocket connection accepted with {'sid': 'KhMXUTGFEx33GQkLAAAA', 'upgrades': [], 'pingTimeout': 5000, 'pingInterval': 25000}
Engine.IO connection established
Sending packet MESSAGE data 0/change-notifications
Received packet MESSAGE data 0/change-notifications,{"sid":"WHrbRQBenMC3BbbsAAAB"}
Namespace /change-notifications is connected
Connected
About to disconnect
Sending packet MESSAGE data 1/change-notifications
Sending packet CLOSE data None
Engine.IO connection dropped
Exiting write loop task
Disconnecting
Received packet NOOP data None
Traceback (most recent call last):
Waiting for write loop task to end
  File "/Users/ses/w/app/.ci/socketio_smoketest.py", line 41, in <module>
Exiting read loop task
    if test():
  File "/Users/ses/w/app/.ci/socketio_smoketest.py", line 34, in test
    sio.disconnect()
  File "/Users/ses/w/app/.venv/lib/python3.9/site-packages/socketio/client.py", line 437, in disconnect
    self.eio.disconnect(abort=True)
  File "/Users/ses/w/app/.venv/lib/python3.9/site-packages/engineio/client.py", line 219, in disconnect
    self.ws.close()
  File "/Users/ses/w/app/.venv/lib/python3.9/site-packages/websocket/_core.py", line 491, in close
    self.sock.settimeout(sock_timeout)
AttributeError: 'NoneType' object has no attribute 'settimeout'

@steinnes
Copy link

I'm sorry that I do not have a reproducible test which only uses websocket-client, hopefully this is of some help.

@PFaurel
Copy link
Author

PFaurel commented May 18, 2021

Can you provide a minimum reproducible example as outlined in the contributing guidelines?

I suspected that issues might arise due to this change from commit 13e83b4, but because exceptions were only passed before, I thought this change would help clarify where the exceptions were coming from so that the code could be made more robust in the long run. The unit tests for this project are currently all passing, so I need to better understand what leads to these exceptions. I do agree that short-term issues might be a problem and I will work to address them and build them into unit tests, or possibly revert to the old solution of passing all exceptions.

I don't have example to reproduce but I confirm that, at least, the commit 13e83b4 lets AttributeError exception raised to WekSocketApp clients whereas it wasn't the ase before (see my initial comment).

@engn33r
Copy link
Collaborator

engn33r commented May 18, 2021

Unfortunately I can't get the sample code from @steinnes to work. I get the error AttributeError: module 'socketio' has no attribute 'Client' from the line sio = socketio.Client(logger=True, engineio_logger=True). I haven't used socketio or engineio before, even though they are built on websocket-client, so I am probably missing something obvious. I do have some questions:

  1. Is anyone encountering this issue without using engineio or socketio? If so, I would very much like to see a code sample that can cause this issue.
  2. What version of engineio, socketio, and asyncio do you have installed when the CI issue is triggered, so that we can try to replicate this? For reference, I see that socketio and engineio use asyncio, which I am not very familiar with, but the threading documentation notes that issues using asyncio with websocket-client have occurred for some users before.
  3. What version of websocket-client did you have installed before the issue appeared? Were you using v0.59.0 before, or an older release?
  4. Which Python version is used in the CI causing the error?

I'm a bit confused now because the error being triggered in websocket-client is from self.sock.settimeout(sock_timeout), which is before the close() exception clause that was changed in commit 13e83b4, so I'm not sure what changed in websocket-client that might be causing this.

Thanks for the quick responses. We should be able to resolve this before long.

@engn33r
Copy link
Collaborator

engn33r commented May 18, 2021

Ok I had an old version of socketio installed causing the module 'socketio' has no attribute 'Client' error for me. Now with socketio v5.3.0, engineio v4.2.0, asyncio v3.4.3, websocket-client 1.0.0, and Python 3.8.6 running on Ubuntu, I receive the following traceback running the code you provided, so I cannot recreate the same settimeout issue that you reported:

Traceback (most recent call last):
  File "error.py", line 4, in <module>
    import socketio
  File "/home/user/.local/lib/python3.8/site-packages/socketio/__init__.py", line 3, in <module>
    from .client import Client
  File "/home/user/.local/lib/python3.8/site-packages/socketio/client.py", line 7, in <module>
    import engineio
  File "/home/user/.local/lib/python3.8/site-packages/engineio/__init__.py", line 8, in <module>
    from .asyncio_client import AsyncClient
  File "/home/user/.local/lib/python3.8/site-packages/engineio/asyncio_client.py", line 7, in <module>
    import aiohttp
  File "/home/user/.local/lib/python3.8/site-packages/aiohttp/__init__.py", line 11, in <module>
    from .connector import *  # noqa
  File "/home/user/.local/lib/python3.8/site-packages/aiohttp/connector.py", line 16, in <module>
    from . import hdrs, helpers
  File "/home/user/.local/lib/python3.8/site-packages/aiohttp/helpers.py", line 30
    ensure_future = asyncio.async
                            ^
SyntaxError: invalid syntax

@PFaurel if the issue you are encountering is different from what steinnes and eirikur-grid have shared, can you provide a traceback to help with troubleshooting? It sounds like you are encountering a AttributeError exception and I would like to see where this exception comes from.

@eirikur-grid
Copy link

We are using the circleci/python:3.9.1 docker image in our CI workflow.
Try creating a new python virtual env, activate it and then run pip install python-socketio[client]==5.1.0
Then attempt the reproduction within that virtual env.

@eirikur-grid
Copy link

Here are the exact versions that got pulled in by pip when we encountered the issue

Requirement already satisfied: python-socketio[client]==5.1.0 in /usr/local/lib/python3.9/site-packages (5.1.0)
Requirement already satisfied: bidict>=0.21.0 in /usr/local/lib/python3.9/site-packages (from python-socketio[client]==5.1.0) (0.21.2)
Requirement already satisfied: python-engineio>=4 in /usr/local/lib/python3.9/site-packages (from python-socketio[client]==5.1.0) (4.0.1)
Requirement already satisfied: requests>=2.21.0 in /usr/local/lib/python3.9/site-packages (from python-socketio[client]==5.1.0) (2.25.1)
Requirement already satisfied: urllib3<1.27,>=1.21.1 in /usr/local/lib/python3.9/site-packages (from requests>=2.21.0->python-socketio[client]==5.1.0) (1.25.6)
Requirement already satisfied: chardet<5,>=3.0.2 in /usr/local/lib/python3.9/site-packages (from requests>=2.21.0->python-socketio[client]==5.1.0) (3.0.4)
Requirement already satisfied: idna<3,>=2.5 in /usr/local/lib/python3.9/site-packages (from requests>=2.21.0->python-socketio[client]==5.1.0) (2.8)
Requirement already satisfied: certifi>=2017.4.17 in /usr/local/lib/python3.9/site-packages (from requests>=2.21.0->python-socketio[client]==5.1.0) (2019.9.11)
Collecting websocket-client>=0.54.0
  Downloading websocket_client-1.0.0-py2.py3-none-any.whl (68 kB)
Installing collected packages: websocket-client
Successfully installed websocket-client-1.0.0

@engn33r
Copy link
Collaborator

engn33r commented May 18, 2021

@eirikur-grid thanks, I can recreate the issue now using a virtual env, even with the latest python-socketio v5.3.0. Unfortunately it's unclear to me at first glance whether the issue is in websocket-client or elsewhere. I might not have time to dig into this much deeper in the next 24 hours or so, but I will provide updates if I find something. Maybe other websocket-client users can provide more insight if the issue is widespread.

@miguelgrinberg since you're the author of python-engineio and python-socketio, do you have insight to whether the issue in this thread might be due to websocket-client or the engineio/socketio code? The v1.0.0 release of websocket-client (released about 12 hours ago) may have impacted socket-io/engine-io functionality, so if your unit tests reveal a problem in websocket-client, I can look into it.

@PFaurel
Copy link
Author

PFaurel commented May 18, 2021

Ok I had an old version of socketio installed causing the module 'socketio' has no attribute 'Client' error for me. Now with socketio v5.3.0, engineio v4.2.0, asyncio v3.4.3, websocket-client 1.0.0, and Python 3.8.6 running on Ubuntu, I receive the following traceback running the code you provided, so I cannot recreate the same settimeout issue that you reported:

Traceback (most recent call last):
  File "error.py", line 4, in <module>
    import socketio
  File "/home/user/.local/lib/python3.8/site-packages/socketio/__init__.py", line 3, in <module>
    from .client import Client
  File "/home/user/.local/lib/python3.8/site-packages/socketio/client.py", line 7, in <module>
    import engineio
  File "/home/user/.local/lib/python3.8/site-packages/engineio/__init__.py", line 8, in <module>
    from .asyncio_client import AsyncClient
  File "/home/user/.local/lib/python3.8/site-packages/engineio/asyncio_client.py", line 7, in <module>
    import aiohttp
  File "/home/user/.local/lib/python3.8/site-packages/aiohttp/__init__.py", line 11, in <module>
    from .connector import *  # noqa
  File "/home/user/.local/lib/python3.8/site-packages/aiohttp/connector.py", line 16, in <module>
    from . import hdrs, helpers
  File "/home/user/.local/lib/python3.8/site-packages/aiohttp/helpers.py", line 30
    ensure_future = asyncio.async
                            ^
SyntaxError: invalid syntax

@PFaurel if the issue you are encountering is different from what steinnes and eirikur-grid have shared, can you provide a traceback to help with troubleshooting? It sounds like you are encountering a AttributeError exception and I would like to see where this exception comes from.

The stack we have:

python/release_candidates/dataplatform_project/refinitiv/data/_data/core/session/_session.py", line 1105, in _stop_streaming
    await stream_connection.close_async()
  File "/efs/jenkslave/slave5/workspace/elektron-api-library-python/release_candidates/dataplatform_project/refinitiv/data/_data/delivery/stream/stream_connection.py", line 625, in close_async
    await self._close_async()
  File "/efs/jenkslave/slave5/workspace/elektron-api-library-python/release_candidates/dataplatform_project/refinitiv/data/_data/delivery/stream/stream_connection.py", line 746, in _close_async
    self._websocket.close()
  File "/efs/jenkslave/slave5/workspace/elektron-api-library-python/release_candidates/dataplatform_project/.tox/tests-nix/lib/python3.9/site-packages/websocket/_app.py", line 197, in close
    self.sock.close(**kwargs)
  File "/efs/jenkslave/slave5/workspace/elektron-api-library-python/release_candidates/dataplatform_project/.tox/tests-nix/lib/python3.9/site-packages/websocket/_core.py", line 491, in close
    self.sock.settimeout(sock_timeout)
    AttributeError: 'NoneType' object has no attribute 'settimeout'

I confirm that self.sock is set to None in _recv() function (_core.py file) :

    def _recv(self, bufsize):
        try:
            return recv(self.sock, bufsize)
        except WebSocketConnectionClosedException:
            if self.sock:
                self.sock.close()
            self.sock = None
            self.connected = False
            raise

WebSocketConnectionClosedException is always raised when we call websocketapp.close().

    def close(self, status=STATUS_NORMAL, reason=bytes('', encoding='utf-8'), timeout=3):
        ...
            try:
                self.connected = False
                self.send(struct.pack('!H', status) + reason, ABNF.OPCODE_CLOSE)
                sock_timeout = self.sock.gettimeout()
                self.sock.settimeout(timeout)
                start_time = time.time()
                while timeout is None or time.time() - start_time < timeout:
                    try:
1>                    frame = self.recv_frame()       => raise WebSocketConnectionClosedException  !
                        if frame.opcode != ABNF.OPCODE_CLOSE:
                            continue
                        if isEnabledForError():
                            recv_status = struct.unpack("!H", frame.data[0:2])[0]
                            if recv_status >= 3000 and recv_status <= 4999:
                                debug("close status: " + repr(recv_status))
                            elif recv_status != STATUS_NORMAL:
                                error("close status: " + repr(recv_status))
                        break
                    except:
                        break
2>            self.sock.settimeout(sock_timeout)    => AttributeError because self.sock was set to None
                self.sock.shutdown(socket.SHUT_RDWR)
            except OSError:  # This happens often on Mac
                pass
            except:
                raise

@engn33r
Copy link
Collaborator

engn33r commented May 18, 2021

I just realized this issue might be due to PR #690, which could cause sock to be None. When I patch websocket-client to revert this PR, it looks like it might resolve this issue. Can @PFaurel, @eirikur-grid, or @steinnes provide feedback if reverting PR #690 fixes this issue for you? If we confirm this is the issue, I can release a version 1.0.1 with this fix.

@miguelgrinberg
Copy link

@engn33r the unit tests in python-engineio and python-socketio do not depend on websocket-client, so they would not reveal any problems if your package has a breaking change. Looking at the error I don't really see how this can be triggered from the socketio side, but let me know if there is anything I can do to help figure this out.

@engn33r
Copy link
Collaborator

engn33r commented May 19, 2021

@miguelgrinberg thank you for your insights, I agree that the issue stems from one of a recent change in websocket-client. If any issues appear in your projects like this, you can direct them here. I hope to have this resolved soon.

@engn33r
Copy link
Collaborator

engn33r commented May 21, 2021

A short update: I was mistaken about PR #690 being a quick fix. @PFaurel was spot on in the first report of this issue - the quickest fix would be to change the raise back to pass on this line of _core.py. However, I think reverting back to pass for all exceptions might not be the best long-term fix, because there could be another issue that has been hidden for a while by the pass that was previously used.

My initial finding is that the recv() function in _socket.py is where the exception is triggered from, which causes self.sock.settimeout(sock_timeout) to run right after frame = self.recv_frame(). In case anyone else wants to examine this deeper, the recv_frame() function referenced in the latest code block that @PFaurel shared calls recv_header(), which calls recv_strict(), which calls the recv() function in _socket.py. There are many exceptions that the recv() function can trigger, but the one I observed was that the bytes_ variable was empty in the _socket.py recv() function, which raised a WebSocketConnectionClosedException. I will need more time to think about where an improvement is needed in this process, but I wanted to share my initial analysis.

@engn33r
Copy link
Collaborator

engn33r commented May 22, 2021

I reverted to the simple except: pass code that was originally used in commit 0e4298a. Maybe future refactoring will find a more elegant approach here.

I will create release 1.0.1 with this fix. If this problem is not fixed in release 1.0.1, let me know and I can reopen this issue.

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

5 participants