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

potential client bug after 0.3.4 when sending slightly larger first message #230

Open
blbradley opened this issue Sep 12, 2017 · 6 comments

Comments

@blbradley
Copy link

Hello! Thanks for the library.

I have been using this library to interface with the GDAX websocket feed. While upgrading to their new websocket protocol, the size of the subscription message increased. Now, I keep getting some UTF-8 error on log level DEBUG with 0.4.2 and 0.3.5:

2017-09-11 22:18:51,240 - ws4py - DEBUG - Error message received (1007) 'b'Invalid UTF-8 bytes''
2017-09-11 22:18:51,241 - root - WARNING - Closed down, code 1006: Going away

Under 0.4.1 and 0.4.0, I don't get the UTF-8 error, but the websocket closes in the same way:

2017-09-11 22:20:24,368 - root - WARNING - Closed down, code 1006: Going away

On 0.3.4, I don't get any errors.

Old subscription message: msg = """{"type": "subscribe","product_ids":["BTC-USD"]}"""
New subscription message: msg = """{"type": "subscribe", "product_ids": ["BTC-USD"], "channels": ["ticker", "level2"]}"""

Let me know if you need anything else.

blbradley added a commit to blbradley/gdax_to_kafka that referenced this issue Sep 12, 2017
* Introduces a flat schema for all messages.
* Subscription message moved to gdax module.
* Fixes bad call to logging module in ws4py implementation.
* Reverted to ws4py 0.3.4 due to [possible bug](Lawouach/WebSocket-for-Python#230) in other recent releases.
@jmichiel
Copy link
Contributor

jmichiel commented Jan 4, 2018

I'm having a similar issue with 0.4.3 trying to use the Bitfinex v2 API:
either this error
DEBUG:ws4py:Error message received (1002) 'b'''
or this one
DEBUG:ws4py:Error message received (1007) 'b'Invalid UTF-8 bytes''

It happens almost every time you try to start multiple subscriptions on the same websocket.
Reverting to ws4py v0.3.4 works perfectly.

@jmichiel
Copy link
Contributor

jmichiel commented Jan 4, 2018

Apparently, it gets broken in v0.3.5 onwards: v0.3.4 works, v0.3.5 and later don't

@jmichiel
Copy link
Contributor

I'm still looking into this and I see weird stuff happening after commit https://github.com/Lawouach/WebSocket-for-Python/search?q=75b88bdecd988694b0429c46cf5c3d6c719cc1f2&type=Commits&utf8=%E2%9C%93
which mainly introduces the _get_from_pending method in websocket.py

@jmichiel
Copy link
Contributor

I think I know what's happening: as soon as more than 1 message is in the queue, the first message is decoded, but the rest of the buffer gets thrown away. If you're "lucky" one or more entire messages are thrown away, but the socket keeps working. As soon as only part of a message is thrown away, the parser will start halfway a frame and will bail out, closing the socket.
I'm looking into fixing it myself, but I'm afraid I don't know the code enough to really understand what's to be changed.
The main problem seems to be that there is no mechanism for the parsers to signal that they have not consumed all bytes. I was thinking of yielding negative numbers in this case, but I can't seem to get it to work...

jmichiel pushed a commit to jmichiel/WebSocket-for-Python that referenced this issue Jan 19, 2018
@jmichiel jmichiel mentioned this issue Jan 19, 2018
@jmichiel
Copy link
Contributor

Damn, if you change the code (see pull request) in opened() to

        self.send(json.dumps({'event': u'subscribe', 'channel': u'candles', 'key':'trade:5m:tBTCUSD'}))
        self.send(json.dumps({'event': u'subscribe', 'channel': u'candles', 'key':'trade:5m:tBCHUSD'}))
        self.send(json.dumps({'event': u'subscribe', 'channel': u'candles', 'key':'trade:5m:tBTGUSD'}))

You still get 'Invalid UTF-8 bytes' errors...

jmichiel added a commit to jmichiel/WebSocket-for-Python that referenced this issue Jan 19, 2018
Lawouach added a commit that referenced this issue Feb 27, 2018
proper fix for #230: on secure, only pass the requested number of bytes to the parsers
@hyperair
Copy link

This should be fixed in master now -- #239 has been merged, and the test I made for this issue in #220 passes. Additionally, this issue might be a dupe of #218.

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

3 participants