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

onConnect() does not indentify incorrect port usage #15

Closed
naishweb opened this issue Aug 11, 2018 · 4 comments
Closed

onConnect() does not indentify incorrect port usage #15

naishweb opened this issue Aug 11, 2018 · 4 comments

Comments

@naishweb
Copy link

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
Copy link
Owner

nikeee commented Aug 11, 2018

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

@naishweb
Copy link
Author

naishweb commented Aug 11, 2018

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
Copy link
Author

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
Copy link
Owner

nikeee commented Aug 11, 2018

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.

@nikeee nikeee closed this as completed in 4776399 Aug 11, 2018
nikeee added a commit that referenced this issue Mar 2, 2019
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

2 participants