search

nikeee / node-ts

Node.js TeamSpeak® 3 Server Query client implemented in TypeScript
GNU Lesser General Public License v3.0
38 stars 6 forks source link

onConnect() does not indentify incorrect port usage #15

Closed naishweb closed 6 years ago

naishweb commented 6 years ago

The function onConnect does not recognize a foreign telnet service. In case you provide the Port of another telnet service as a parameter to establish the connection to (e.g mailserver ("220 Welcome(\n)")), The function emits ("correctly") an connect. Every/most further communication aka send() will appear correct. Imho there should either be a possibilty to check the first answer ("TS3\nWelcome to the TeamSpeak 3 ServerQuery interface, type "help" for a list of commands and "help <command>" for information on a specific command.") myself or the function should proof that it is a correct servicepartner instead of ignoring the "two first lines sent by server ("TS3" and information message)"

node-ts.js:42 node-ts.js:51 ff.

nikeee commented 6 years ago

How would you handle this error? Would you rather emit an error event or throw an exception in onConnect?

naishweb commented 6 years ago

OK, after i wrote an essay XD here is my conclusion: i would emit an error. (like you already improved the connection error handling from gwTumm. Then its handled in this code area on a similar way...)

naishweb commented 6 years ago

wait, didn't you throwed an exception in case the connection could not be established?! I am not able to check this atm. but you know what i mean. i would do it the same way you already did with the "ERRCONNECTION".

nikeee commented 6 years ago

Currently, there is no throw statement in the entire file. I'd also prefer emitting an error event.

Edit: I see, there is a promise rejection in the beginning of send. Also, every error that the server returns is thrown as an exception in send. This cannot be done on onConnect(), since the user cannot handle an exception appropriately (it would be insice a cllback of a private function).

You might want to look at 4776399 to see if it fits what you need. Haven't tested it.

Despite that, I personally think that this library needs a re-write. However, this won't happen in the near future.