bmoscon / cryptofeed

Cryptocurrency Exchange Websocket Data Feed Handler
Other
2.22k stars 684 forks source link

ConnectionClosedError 1006 / IncompleteReadError #493

Closed tristan-murfitt-elw closed 2 years ago

tristan-murfitt-elw commented 3 years ago

Describe the bug Same bug as https://github.com/bmoscon/cryptofeed/issues/308 but I've managed to replicate this on a consistent basis.

[asyncio] ERROR: Future exception was never retrieved
future: <Future finished exception=ConnectionClosedError('code = 1006 (connection closed abnormally [internal]), no reason')>
Traceback (most recent call last):
  File "/lib/python3.7/site-packages/websockets/protocol.py", line 827, in transfer_data
    message = await self.read_message()
  File "/lib/python3.7/site-packages/websockets/protocol.py", line 895, in read_message
    frame = await self.read_data_frame(max_size=self.max_size)
  File "/lib/python3.7/site-packages/websockets/protocol.py", line 971, in read_data_frame
    frame = await self.read_frame(max_size)
  File "/lib/python3.7/site-packages/websockets/protocol.py", line 1051, in read_frame
    extensions=self.extensions,
  File "/lib/python3.7/site-packages/websockets/framing.py", line 105, in read
    data = await reader(2)
  File "/lib/python3.7/asyncio/streams.py", line 677, in readexactly
    raise IncompleteReadError(incomplete, n)
asyncio.streams.IncompleteReadError: 0 bytes read on a total of 2 expected bytes

The above exception was the direct cause of the following exception:

websockets.exceptions.ConnectionClosedError: code = 1006 (connection closed abnormally [internal]), no reason

Background: I have a requirement to disconnect particular feeds without restarting the feedhandler. To solve this I maintain a list of asyncio task objects in the FeedHandler whenever a feed is added. Then I can cancel any of these tasks from my main app. My fork is behind on your new connection changes, but it's just a simple modification here .

To Reproduce

I'm seeing this error from "natural" use too. Above seems to be a good way to replicate.

Expected behavior Gateio should gracefully handle abrupt WS closures as the other exchanges do. Is there any reason why it would behave differently?

Cryptofeed Version

bmoscon commented 3 years ago

I feel like canceling a task is a surefire way to get disconnection errors like this. Why not shut that task down gracefully? If you look at the stop method:

https://github.com/bmoscon/cryptofeed/blob/master/cryptofeed/feedhandler.py#L172

you can see what it does to a feed - it calls stop, and then creates a task to call feed.shutdown. Not all of what goes on it shutdown is necessary, but it does then close the connections. I think if you want to not get errors like this you'll need to do something like this and gracefully close it out, or implement an unsubscribe method for exchanges and call that and then close the connection

tristan-murfitt-elw commented 3 years ago

@bmoscon thanks for pointing these new methods out - this is a huge value add since the last release!

As for the ConnectionClosedError 1006 IncompleteReadError problem: this does happen often (and will continue to) just due to the nature of crypto infrastructure. Force cancellation of asnycio tasks is a great way to replicate the problem to see how we can make the library more resilient. I spent some time to try and understand why this was affecting Gateio feeds specifically, and not other exchanges, but to no avail. Maybe we can keep this pinned and refer back here if anybody else notices this becoming prevalent?

bmoscon commented 2 years ago

going to close this, but if anyone has any issues that look similar, please comment here or open a new ticket.