redouane59 / twittered

Twitter API client for Java developers
Apache License 2.0
238 stars 65 forks source link

Rate limit exceeded message displaying milliseconds instead of seconds #238

Closed calves07 closed 3 years ago

calves07 commented 3 years ago

Hey,

In AbstractRequestHelper, you have: LOGGER.info("Rate limit exceeded, new retry in " + 1000L * retryAfter + "s"); You are multiplying seconds by 1000, displaying milliseconds instead of seconds.

redouane59 commented 3 years ago

Well seen, I'm going to correct that ;)

calves07 commented 3 years ago

If you're going to change that, I also suggest displaying the time, for example: Rate limit exceeded, new retry in 300s (at 15:23:00) Or even better, instead of displaying in seconds, you could display the information in hours/minutes/seconds, according to the value.

For example, the following method will return "5m" for time=300 and "5m1s" for time=301

public static String getSecondsAsText(int time) {
        String str = "";
        int hours, minutes, seconds;

        hours = time / 3600;
        minutes = (time - hours * 3600) / 60;
        seconds = time - hours * 3600 - minutes * 60;

        if (hours > 0) {
            str = str.concat(hours + "h");
        }
        if (minutes > 0) {
            str = str.concat(minutes + "m");
        }
        if (seconds > 0) {
            str = str.concat(seconds + "s");
        }
        return str;
    }
redouane59 commented 3 years ago

About the current time, is it not to set up on your side ? For example, with my log configuration I have this message displayed : 15:28:12.459 [main] INFO io.github.redouane59.twitter.helpers.AbstractRequestHelper - Rate limit exceeded, new retry in 300s

calves07 commented 3 years ago

Well, that's the current time (at the time of logging), I also have that. I'm suggesting to include the time for the reset. Because it's more user friendly to say it resets at 15:35 rather than in 400 seconds, for example.

redouane59 commented 3 years ago

I see, let me check it :)

redouane59 commented 3 years ago
16:00:57.520 [main] INFO io.github.redouane59.twitter.helpers.AbstractRequestHelper - Rate limit exceeded, new retry in 5m at 16:07

done with latest commit

calves07 commented 3 years ago

Awesome, thanks.

calves07 commented 3 years ago

Looks like the time is wrong... I got this message at 21:28, it should indicate 21:33, give or take, not a time from the past.

[main] INFO io.github.redouane59.twitter.helpers.AbstractRequestHelper - Rate limit exceeded, new retry in 5m at 21:07

redouane59 commented 3 years ago

Looks like the time is wrong... I got this message at 21:28, it should indicate 21:33, give or take, not a time from the past.

So strange, on my side it is still working well :

22:59:43.806 [main] INFO io.github.redouane59.twitter.helpers.AbstractRequestHelper - Rate limit exceeded, new retry in 5m at 23:07

I don't know what could explain that hmmm..

Which endpoint were you using ?

redouane59 commented 3 years ago

Hey, Could you please try the function minutesBeforeNow in ConverterHelper and told me if it works correctly ? Because I saw in another user log that the endtime was wrong, i cannot understand why it only work on my side :/

redouane59 commented 3 years ago

Ok I found the issue : https://github.com/redouane59/twittered/pull/261