levrik / node-modern-rcon

A modern RCON client implementation written in ES2015
MIT License
29 stars 5 forks source link

Crash when reciving response split into multiple packets #3

Open BamaJoe411 opened 6 years ago

BamaJoe411 commented 6 years ago

`/home/bamajoe411/Documents/discord-bot/node_modules/modern-rcon/rcon.js:100 throw new RconError('Responded id did not match sent id'); ^

RconError: Responded id did not match sent id at Rcon._handleResponse (/home/bamajoe411/Documents/discord-bot/node_modules/modern-rcon/rcon.js:100:13) at emitOne (events.js:116:13) at Socket.emit (events.js:211:7) at addChunk (_stream_readable.js:263:12) at readableAddChunk (_stream_readable.js:250:11) at Socket.Readable.push (_stream_readable.js:208:10) at TCP.onread (net.js:597:20) `

levrik commented 6 years ago

Ah, yeah. Splitted packets aren't supported at the moment. Which server-side implementation are you using for your project?

BamaJoe411 commented 6 years ago

Minecraft 1.13.1 Java Edition

levrik commented 6 years ago

What did you exactly do to trigger a splitted packet? I tested this with Minecraft 1.11.2 Java Edition back then and that seemed to didn't use splitted packets.

BamaJoe411 commented 6 years ago

If the response is larger than the maximum allowed in one packet then the response is split up into multiple packets

https://wiki.vg/RCON#Implementation_details

levrik commented 6 years ago

Ah, okay. Since I don't have that much time at the moment it would be awesome if you could prepare a PR for me to review. Let me know if it's not possible for you to do. Then I can take care of it but it will take time.

BamaJoe411 commented 6 years ago

PR?

levrik commented 6 years ago

Pull Request

BamaJoe411 commented 6 years ago

i see in the code that the id of the sending and receiving packets is the 5th field. where do i find which field is what? (if you cant tell i have no clue what im doing xD)

levrik commented 5 years ago

Not sure what you're asking. Can you rephrase your question?

lewck commented 3 years ago

I spent the best part of today attempting to implement this but got nowhere unfortunately, I'll dump my findings so anyone else looking to fix this can avoid the issues I ran into.

The simplest solution I found was checking the length in _handleResponse, if greater than max packet length (4096+10), add to an ephemeral mapping of id => messages and don't callback. When the next message comes in < (4096+10), join everything in the ephemeral mapping and callback.

This method worked well and was reliable as far as I could tell, however if a message came in at exactly max bytes, the service would hang and timeout waiting for more data. McTools gets around this by utilising the queuing nature of the RCON service and sending a dummy packet, once that packet has been received you can assume the previous message has completed. I'm not sure if it's a lower level network difference between the languages, this projects design or my poor attempt that meant I couldn't get this type of implementation to work reliably!

levrik commented 3 years ago

@lewck Could you let me know how I can reproduce this issue, meaning how to trigger a split packet from a MC RCON server? If I find some time I'll try to fix this.

lewck commented 3 years ago

Sure, the method I use to get a larger packet is running data get entity [username], but the player must be connected