From e4e3bd44fc24ec297f1ad54dd044c4ccc084e3f7 Mon Sep 17 00:00:00 2001 From: ErikKalkoken Date: Mon, 8 Jun 2020 14:59:22 +0200 Subject: [PATCH] Fix sleep length must be non-negative --- .../modules/discord/discord_client/client.py | 12 +++-- .../discord_client/tests/test_client.py | 46 ++++++++++++++++++- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/allianceauth/services/modules/discord/discord_client/client.py b/allianceauth/services/modules/discord/discord_client/client.py index b5193f51..605473d3 100644 --- a/allianceauth/services/modules/discord/discord_client/client.py +++ b/allianceauth/services/modules/discord/discord_client/client.py @@ -46,6 +46,9 @@ DURATION_CONTINGENCY = 500 # time until next reset is below this threshold WAIT_THRESHOLD = 250 +# Minimum wait duration when doing a blocking wait +MINIMUM_BLOCKING_WAIT = 50 + # If the rate limit resets soon we will wait it out and then retry to # either get a remaining request from our cached counter # or again wait out a short reset time and retry again. @@ -604,8 +607,11 @@ class DiscordClient: name=self._KEY_GLOBAL_RATE_LIMIT_REMAINING, value=RATE_LIMIT_MAX_REQUESTS, px=RATE_LIMIT_RESETS_AFTER + DURATION_CONTINGENCY - ) - resets_in = self._redis.pttl(self._KEY_GLOBAL_RATE_LIMIT_REMAINING) + ) + resets_in = max( + MINIMUM_BLOCKING_WAIT, + self._redis.pttl(self._KEY_GLOBAL_RATE_LIMIT_REMAINING) + ) if requests_remaining >= 0: logger.debug( '%s: Got one of %d remaining requests until reset in %s ms', @@ -615,7 +621,7 @@ class DiscordClient: ) return requests_remaining - elif resets_in < WAIT_THRESHOLD: + elif resets_in < WAIT_THRESHOLD: sleep(resets_in / 1000) logger.debug( '%s: No requests remaining until reset in %d ms. ' diff --git a/allianceauth/services/modules/discord/discord_client/tests/test_client.py b/allianceauth/services/modules/discord/discord_client/tests/test_client.py index 8ae39fcf..0d9437fc 100644 --- a/allianceauth/services/modules/discord/discord_client/tests/test_client.py +++ b/allianceauth/services/modules/discord/discord_client/tests/test_client.py @@ -50,6 +50,12 @@ mock_redis = MagicMock(**{ }) +# default mock function to simulate sleep +def my_sleep(value): + if value < 0: + raise ValueError('sleep length must be non-negative') + + class DiscordClient2(DiscordClient): """Variant that overwrites lua wrappers with dummies for easier testing""" @@ -1034,6 +1040,42 @@ class TestRateLimitMechanic(TestCase): requests_mocker.post( f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles', json=self.my_role ) + mock_sleep.side_effect = my_sleep + my_mock_redis = MagicMock(**{'pttl.side_effect': my_redis_pttl_2}) + mock_redis_decr_or_set.side_effect = my_redis_decr_or_set + client = DiscordClient(TEST_BOT_TOKEN, my_mock_redis) + + result = client.create_guild_role( + guild_id=TEST_GUILD_ID, role_name=self.my_role['name'] + ) + self.assertDictEqual(result, self.my_role) + self.assertTrue(mock_sleep.called) + + @patch(MODULE_PATH + '.sleep') + def test_wait_if_reset_happens_soon_and_sleep_must_not_be_negative( + self, requests_mocker, mock_sleep, mock_redis_decr_or_set + ): + counter = 0 + + def my_redis_pttl_2(name: str): + if name == DiscordClient._KEY_GLOBAL_BACKOFF_UNTIL: + return -1 + else: + return -1 + + def my_redis_decr_or_set(**kwargs): + nonlocal counter + counter += 1 + + if counter < 2: + return -1 + else: + return 5 + + requests_mocker.post( + f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles', json=self.my_role + ) + mock_sleep.side_effect = my_sleep my_mock_redis = MagicMock(**{'pttl.side_effect': my_redis_pttl_2}) mock_redis_decr_or_set.side_effect = my_redis_decr_or_set client = DiscordClient(TEST_BOT_TOKEN, my_mock_redis) @@ -1075,6 +1117,7 @@ class TestRateLimitMechanic(TestCase): requests_mocker.post( f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles', json=self.my_role ) + mock_sleep.side_effect = my_sleep my_mock_redis = MagicMock(**{'pttl.side_effect': my_redis_pttl_2}) mock_redis_decr_or_set.return_value = -1 client = DiscordClient(TEST_BOT_TOKEN, my_mock_redis) @@ -1208,7 +1251,8 @@ class TestBackoffHandling(TestCase): requests_mocker.post( f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles', json=self.my_role ) - retry_after = 50 + retry_after = 50 + mock_sleep.side_effect = my_sleep my_mock_redis = MagicMock(**{'pttl.return_value': retry_after}) mock_redis_decr_or_set.return_value = 5 client = DiscordClient(TEST_BOT_TOKEN, my_mock_redis)