Added fragmented text message handling in WebSocketClientHandler #360
Conversation
(if the websocket message was fragmented, only the first part was sent to the message handler and the rest of the message was ignored). See RFC 6455 section 5.4 and 5.2 for more information.
* develop: (35 commits) Bump netty-codec-http from 4.1.36.Final to 4.1.37.Final Bump log4j-slf4j-impl from 2.11.2 to 2.12.0 Bump log4j-core from 2.11.2 to 2.12.0 Bump netty-handler from 4.1.36.Final to 4.1.37.Final Bump netty-all from 4.1.36.Final to 4.1.37.Final Bump rxjava2-interop from 0.13.6 to 0.13.7 Bump maven-javadoc-plugin from 3.1.0 to 3.1.1 Bump mockito-core from 2.28.2 to 3.0.0 Bump fluent-hc from 4.5.5 to 4.5.9 Bump xchange.version from 4.3.19 to 4.3.20 Revert previous changes Revert previous changes Add ws client handler Update lib to use root project update parent lib update parent lib version update update Update update ...
|
Issues mentioning intermittent failures or lost messages may be related |
|
@Nephtys: The change looks sensible. Did you encounter this issue with a specific exchange, or is this just precautionary? |
|
We encountered this issue with snapshot messages on the LGO exchange websocket, but it's not an exchange specific case, it's how the websocket protocol was designed and any exchange using message framing will encounter this issue. |
|
All understood; I did read up on this part of the protocol before asking. I'm just wondering which issues this fix might close, since the original reporters might find it interesting. #284 perhaps. I found it odd that I've never seen it, but I agree; it's probably that it's manifesting in lots of different ways. In any case, this looks sensible, as I said, so LGTM. |
There was a problem hiding this comment.
Confirmed that this doesn't need to be a StringBuffer:
https://stackoverflow.com/questions/46508433/concurrency-in-netty
Approved
|
Yes, it looks it could help. I am still running my custom xchange-stream bitfinex version with checksums. Still checksum error a few times a day. I'll try changes from this PR, after a few days I can say if it helped. |
|
Thanks @Nephtys and @davidjirovec |
|
Yes you're right @badgerwithagun, maybe #284 and maybe #183 and #137 Thank you for your review ! |
Added fragmented text message handling in WebSocketClientHandler
|
Testing, checksum errors still sometimes show up :-| |
When the received message was fragmented, the first part was sent to the message handler and the rest of the message was ignored.
The first message has type TextWebSocketFrame and the following have type ContinuationWebSocketFrame. Each frame has a boolean finalFragment to indicate if the message was fully received.
See RFC 6455 section 5.4 and 5.2, and Netty WebSocket08FrameDecoder for more information.