sleepy-discord icon indicating copy to clipboard operation
sleepy-discord copied to clipboard

Possibly wrong conditional

Open tlaik opened this issue 6 years ago • 6 comments

Is the following check required in some cases? https://github.com/yourWaifu/sleepy-discord/blob/master/sleepy_discord/client.cpp#L109

I've had getServers() and getRoles() fail on me with TOO_MANY_REQUESTS error from this code.

But I'm not sure if it's right to treat it as an error when Discord API tells you that you've 0 messages left but doesn't set an error status on its own. Doesn't this mean that this was the last correctly processed command?

When I completely disabled this conditional, the methods mentioned above continued to work just fine.

So far, my impression from reading about Discord API is that this check is unneeded. Sorry if I'm not understanding something here, got started with this only few days ago.

tlaik avatar Feb 11 '18 10:02 tlaik

According to the docs, (https://discordapp.com/developers/docs/topics/rate-limits#header-format) X-RateLimit-Remaining is used to tell if you are about to hit a rate limit. I have questioned before if it should be a warning instead as the same error as a real rate limited as you get data back.

The library locks the request function when reaching 0, as any more will not work. It also sends a fake error, that I've also questioned if it should display a different error message.So, this check is needed to prevent the user from making too many requests to Discord.

yourWaifu avatar Feb 11 '18 14:02 yourWaifu

I also think it should be a warning, since I've been getting a silly case where I call getServers(), receive a valid list of servers that my bot is on, but also receive those fake TOO_MANY_REQUESTS errors. The library should probably raise an error only when isRateLimited is true and new request is made.

tlaik avatar Feb 11 '18 14:02 tlaik

A short example:

Source (slightly modified hello project):

#include "sleepy_discord/websocketpp_websocket.h"

class myClientClass : public SleepyDiscord::DiscordClient {
public:
	using SleepyDiscord::DiscordClient::DiscordClient;

	void onReady(std::string* jsonMsg)
	{
		printf("Ready!\n");
		std::vector<SleepyDiscord::Server> servers = getServers();
		printf("Number of servers: %d\n", servers.size());
		if(servers.size()) printf("First server name: %s\n", servers[0].name.c_str());
	}
};

int main() {
	myClientClass client("my_token", 2);
	client.run();
}

Result: [2018-02-11 17:49:52] [connect] Successful connection [2018-02-11 17:49:52] [connect] WebSocket Connection 104.16.60.37:443 v-2 "WebSocket++/0.7.0" /?v=6 101 Ready! Error 429: You've made too many requests Number of servers: 1 First server name: Nepgear Did Nothing Wrong

Should such a simple code even get to the point where my message quota runs out, and a fake error has to be raised? It's rather strange.

tlaik avatar Feb 11 '18 15:02 tlaik

Well, if it otherwise causes you to get rate limited, a warning could be displayed and any further requests will just return with an error. I think that's reasonable.

yourWaifu avatar Feb 11 '18 16:02 yourWaifu

I see. Do you know what might be causing the example above to hit the rate limit? Is there some way to see the actual number of messages sent to Discord API for debug purposes?

tlaik avatar Feb 11 '18 17:02 tlaik

I don't really know why it's causing it. However if the number is a available, you can do this getSever().header["X-RateLimit-Remaining"]. Note, that the value returned is a string, so you could do a stoi but make sure that it's available first.

yourWaifu avatar Feb 11 '18 17:02 yourWaifu