-
Notifications
You must be signed in to change notification settings - Fork 2
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
Support for chunked TCP packets #1
Comments
We should just be able to change this transport to send/receive whatever protocol we want, both client+server are encapsulated in this module! A PR would be fantastic (along with tests showing it's broken if possible) |
@SomeoneWeird It's hard to test such specific issue because on loopback interface MTU value is very large and it's hard to compose a message big enough. But I can try. Or possibly it will be easier to simulate TCP fragmentation just by concatenating packages in one buffer and then slicing it into smaller buffers of 50 bytes. This will be not quiet fair test, but it definitely will be shorter. |
Hello. I've looked through this transport code and found that if messages sent through tcp will have size bigger than network MTU message will be cut in two or more chunks. This will cause an exception here:
https://github.com/hudson-taylor/tcp-transport/blob/master/src/index.js#L8 because JSON will be malformed, because rest part will come in next chunk.
If you interested in fixing this, I can either submit a PR, which will be potentially breaking change for 3rd party clients (not using tcp-transport implementation), because it will be needed to modify plain JSON to some custom protocol with packet delimiters, etc. Probably some streaming json parsers can do the trick, but I'm not pretty sure. Or either I can write a module based on this one, but with chunked decoding support. What will be better?
*Note: With custom protocol it will be also possible to transmit not only JSON string, but also binary (Buffers) + checksum or data size verification.
The text was updated successfully, but these errors were encountered: