Skip to content

Commit

Permalink
RAI-17360 Expanded retry mechanism to do retries on a ConnectionError (
Browse files Browse the repository at this point in the history
…#137)

* expanded retry mechanism to do retries on ConnectionError as well

* updated CHANGELOG and version for new release
  • Loading branch information
antikus authored Oct 6, 2023
1 parent 0f23579 commit cae1991
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 22 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## v0.6.16

* Expanded the retry mechanism for HTTP failures raised as `ConnectionError`.

## v0.6.15

* Increase auth token expiration buffer from 5s to 60s.
Expand Down
2 changes: 1 addition & 1 deletion railib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@
# See the License for the specific language governing permissions and
# limitations under the License.

__version_info__ = (0, 6, 15)
__version_info__ = (0, 6, 16)
__version__ = ".".join(map(str, __version_info__))
9 changes: 2 additions & 7 deletions railib/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
import json
import logging
from os import path
import socket
import time
from urllib.error import URLError
from urllib.parse import urlencode, urlsplit, quote
from urllib.request import Request, urlopen
Expand Down Expand Up @@ -228,11 +226,8 @@ def _urlopen_with_retry(req: Request, retries: int = 0):
for attempt in range(attempts):
try:
return urlopen(req)
except URLError as e:
if isinstance(e.reason, socket.timeout):
logger.warning(f"Timeout occurred (attempt {attempt + 1}/{attempts}): {req.full_url}")
else:
logger.warning(f"URLError occurred {e.reason} (attempt {attempt + 1}/{attempts}): {req.full_url}")
except (URLError, ConnectionError) as e:
logger.warning(f"URL/Connection error occured {req.full_url} (attempt {attempt + 1}/{attempts}). Error message: {str(e)}")

if attempt == attempts - 1:
logger.error(f"Failed to connect to {req.full_url} after {attempts} attempt{'s' if attempts > 1 else ''}")
Expand Down
32 changes: 18 additions & 14 deletions test/test_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ def test_validation(self):
@patch('railib.rest.urlopen')
class TestURLOpenWithRetry(unittest.TestCase):

WARNING_LOG_PREFIX = "URL/Connection error occured"
ERROR_LOG_PREFIX = "Failed to connect to"

def test_successful_response(self, mock_urlopen):
# Set up the mock urlopen to return a successful response
mock_response = MagicMock()
Expand All @@ -55,7 +58,7 @@ def test_negative_retries(self, _):

self.assertIn("Retries must be a non-negative integer", str(e.exception))

def test_timeout_retry(self, mock_urlopen):
def test_url_error_retry(self, mock_urlopen):
# Set up the mock urlopen to raise a socket timeout error
mock_urlopen.side_effect = URLError(socket.timeout())

Expand All @@ -66,26 +69,27 @@ def test_timeout_retry(self, mock_urlopen):

self.assertEqual(mock_urlopen.call_count, 3) # Expect 1 original call and 2 calls for retries
self.assertEqual(len(log.output), 4) # Expect 3 log messages for retries and 1 for failure to connect
self.assertIn('Timeout occurred', log.output[0])
self.assertIn('Timeout occurred', log.output[1])
self.assertIn('Timeout occurred', log.output[2])
self.assertIn('Failed to connect to', log.output[3])
self.assertIn(TestURLOpenWithRetry.WARNING_LOG_PREFIX, log.output[0])
self.assertIn(TestURLOpenWithRetry.WARNING_LOG_PREFIX, log.output[1])
self.assertIn(TestURLOpenWithRetry.WARNING_LOG_PREFIX, log.output[2])
self.assertIn(TestURLOpenWithRetry.ERROR_LOG_PREFIX, log.output[3])

def test_other_error_retry(self, mock_urlopen):
# Set up the mock urlopen to raise a non-timeout URLError
mock_urlopen.side_effect = URLError('Some other error')
def test_connection_error_retry(self, mock_urlopen):
# Set up the mock urlopen to raise a error that is subclass of ConnectonError
mock_urlopen.side_effect = ConnectionResetError("connection reset by peer")

req = Request('https://example.com')
with self.assertLogs() as log:
with self.assertRaises(Exception):
_urlopen_with_retry(req, retries=2)
_urlopen_with_retry(req, 2)

self.assertEqual(mock_urlopen.call_count, 3) # Expect 3 calls for retries
self.assertEqual(mock_urlopen.call_count, 3) # Expect 1 original call and 2 calls for retries
self.assertEqual(len(log.output), 4) # Expect 3 log messages for retries and 1 for failure to connect
self.assertIn('URLError occurred', log.output[0])
self.assertIn('URLError occurred', log.output[1])
self.assertIn('URLError occurred', log.output[2])
self.assertIn('Failed to connect to', log.output[3])
self.assertIn(TestURLOpenWithRetry.WARNING_LOG_PREFIX, log.output[0])
self.assertIn(TestURLOpenWithRetry.WARNING_LOG_PREFIX, log.output[1])
self.assertIn(TestURLOpenWithRetry.WARNING_LOG_PREFIX, log.output[2])
self.assertIn(TestURLOpenWithRetry.ERROR_LOG_PREFIX, log.output[3])



if __name__ == '__main__':
Expand Down

0 comments on commit cae1991

Please sign in to comment.