Avoid possible negative values of delay in sleep(delay)

[dbuendiab]

Found this problem using twitter.retry = True in a multiple query that exceeded current window. I don't know why delay can be negative (maybe time() and reset not in sync, somehow?) but this validation seems to be a defensive strategy not too expensive.

[RouxRC]

Actually my understanding would be that if delay is negative, we were really close to the reset limits time and could probably just not even sleep at all before retry.
So waiting 30 seconds would be quite unefficient imho and confusing for a user, I'd rather set delay to a maximum of 5 seconds or such then, what do you think @sixohsix ?

[dbuendiab]

You're right, @RouxRC, but if I chose the 30 seconds option it was just because we come from a 429 error. The other option for me was to ignore the sleep() statement if the delay was negative. But I ended in the more conservative option. It can be discussed, I suppose.

[miv-ableton]

Ooh, this is a fun bug! Looks like a classic error caused by differing values of time. I suspect you can reproduce it by setting your computer's time into the future an hour or so. The issue is that we compare the epoch time in the header from Twitter with the local computer time returned by time().

The gold-standard bugfix here would be to also get the current time from Twitter. If there exists a header that reports the time of request, subtract that from the reset time to get the true delay value.

If there does not exist such a header, then there's not much we can do. The underlying problem is that the user should set their time correctly (ideally with NTP). I would probably just raise an exception that stops the program with the message "Please set your system time correctly".

If we want to be lenient and just try and work around the problem the difference between 5 and 30 seconds in this case is negligible since we have no idea how far off the local clock is from reality.

So, merging this as it is is fine by me. It's an incremental fix.

EDIT: ah, darn. Wrong account. This is @sixohsix writing.

[dbuendiab]

I don't think it was a set time problem. It was a spot incident, maybe malformed response, so the only question is avoid the sleep() error because of a negative parameter to continue. For me it would be good any solution that keeps the program running in those circumstances.

[dbuendiab]

By the way, I'm complete novice in collaborative GitHub, so if you think the pull request must go on, I left it on your hands. Or please explain to me what steps I should have to take.

[miv-ableton]

@dbuendiab you don't have to make any changes right now. I think the code is fine as it is. Thanks!

@RouxRC I recommend we merge this as is, and try other fixes if the issue returns. What do you think?

[dbuendiab]

OK. Thank you.

[RouxRC]

that's fine with me, as I said my concern was to consider a somehow unexpected behavior although I guess you expect such weird things to happen when you activate an autoretry setting, so merging now :)