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

Handle bad token #71

Closed
ianagbip1oti opened this issue Mar 15, 2018 · 12 comments
Closed

Handle bad token #71

ianagbip1oti opened this issue Mar 15, 2018 · 12 comments
Labels
enhancement New feature or request good first issue Good for newcomers s T-Shirt size S
Milestone

Comments

@ianagbip1oti
Copy link
Member

Handle cleanly case when the bot token is bad.

Perhaps by throwing an ErisCasperFatalException?

@ianagbip1oti ianagbip1oti added enhancement New feature or request good first issue Good for newcomers labels Mar 15, 2018
@ianagbip1oti ianagbip1oti added the s T-Shirt size S label Mar 28, 2018
@ianagbip1oti ianagbip1oti added this to the 0.4.0 milestone Mar 29, 2018
@WangzoR
Copy link

WangzoR commented Apr 3, 2018

assuming that EC_TOKEN will be the permanent method of handling the bot token
worthwhile putting in an exception and check for null values returned as part of the get system.env on top of the primary check which is to gracefully handle if token is totally invalid

@WangzoR
Copy link

WangzoR commented Apr 3, 2018

@ianagbip1oti, some guidance please, so I made an attempt at the null check, but after I finished and started digging deeper it seems intrinsically built in already as part of the BotToken object, so not so sure at what 'level' this kind of exception check should be handled in

My Code:
image
image

It appears that this already throws a semi suitable exception, just change the message to suit?
Existing lower level code:
image

Format validity?
In addition, for token validity sake, aside from DiscordAPI throwing a fit at connecting, is there any baseline integrity format check? found the below, no idea what this represents and significance of 5381 and if this is to utilised to verify anything?
image

@ianagbip1oti
Copy link
Member Author

EC_TOKEN will always be a way of passing in the token, it may not be the only way (see #106), but since #106 is not done yet, just handling EC_TOKEN here is fine.

I don't think we need to check the token format. If a token is provided we attempt to login with it, and handle the error message Discord sends us back.

Otherwise yeah, we just want more user friendly error messages in the exception.

There is no significance to the 5381 in hashCode. Choosing some kind of randomish seed for use in hashCode is fairly common.

@CoreyShupe
Copy link
Member

Handled bad token by catching authentication issues given by discord. Starts to go bad once you hit the rate limit but that should be worried about by the client. I do null checks for the EC_TOKEN however if the token is for example test it's allowed through but caught immediately by discord.

@ianagbip1oti
Copy link
Member Author

What rate limit are you referring to here?

@CoreyShupe
Copy link
Member

If you try to connect too many times within a certain period time with identify it will rate limit you and spam you with OP 10 instead of throw an auth error, at which point we can't resume so the websocket falls out and it just starts throwing broken pipe errors. (found this when testing what happens when invalid tokens are authenticated)

@ianagbip1oti
Copy link
Member Author

Is the app doing these identifys, or is the app being run multiple times by the user to hit this rate limit?

@CoreyShupe
Copy link
Member

Being run multiple times by the user, hence why it's not really our problem. The identifies are only being run once per app however if the user kind of spams the app it will start not throwing auth errors and instead broken pipe errors. Nothing we have to deal with but I thought it was worth mentioning if you want to watch for that staff on app startup.

@ianagbip1oti
Copy link
Member Author

As long as we're accurately reporting to them that they're having issues because their token is wrong, then there's not much we can do.

@CoreyShupe
Copy link
Member

That's the issue, once they hit the rate limit we can't accurately tell them what's going wrong.

@ianagbip1oti
Copy link
Member Author

Do you mean opcode 9 invalid session? opcode 10 is the hello message.

I'm ok with that to start with, we would have given them a bad token message many times before they hit the rate limit.

We can handle cases of invalid session followed by a failure to resume later.

@CoreyShupe
Copy link
Member

No, I don't mean opcode 9, it gives us an opcode 10 hello message and spams it. I've done pretty extensive testing with it. Trying to give me an invalid session, but it always just spams 10 or closes with an auth error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers s T-Shirt size S
Development

No branches or pull requests

3 participants