Merge pull request #841 from philipp-sontag-by/handle-async-timeouter…

…ror-during-requests

Retry asyncio.TimeoutErrors during API requests

kopf/_cogs/clients/api.py

@@ -84,7 +84,7 @@ async def request(
  )
  await errors.check_response(response) # but do not parse it!
 
- except (aiohttp.ClientConnectionError, errors.APIServerError) as e:
+ except (aiohttp.ClientConnectionError, errors.APIServerError, asyncio.TimeoutError) as e:
  if backoff is None: # i.e. the last or the only attempt.
  logger.error(f"Request attempt {idx} failed; escalating: {what} -> {e!r}")
  raise

tests/apis/test_api_requests.py

@@ -149,6 +149,9 @@ async def serve_slowly():
 
  with timer, pytest.raises(asyncio.TimeoutError):
  timeout = aiohttp.ClientTimeout(total=0.1)
+ # aiohttp raises an asyncio.TimeoutError which is automatically retried.
+ # To reduce the test duration we disable retries for this test.
+ settings.networking.error_backoffs = None
  await fn('/url', timeout=timeout, settings=settings, logger=logger)
 
  assert 0.1 < timer.seconds < 0.2
@@ -172,6 +175,9 @@ async def serve_slowly():
 
  with timer, pytest.raises(asyncio.TimeoutError):
  settings.networking.request_timeout = 0.1
+ # aiohttp raises an asyncio.TimeoutError which is automatically retried.
+ # To reduce the test duration we disable retries for this test.
+ settings.networking.error_backoffs = None
  await fn('/url', settings=settings, logger=logger)
 
  assert 0.1 < timer.seconds < 0.2
@@ -190,6 +196,9 @@ async def serve_slowly():
 
  with timer, pytest.raises(asyncio.TimeoutError):
  timeout = aiohttp.ClientTimeout(total=0.1)
+ # aiohttp raises an asyncio.TimeoutError which is automatically retried.
+ # To reduce the test duration we disable retries for this test.
+ settings.networking.error_backoffs = None
  async for _ in stream('/url', timeout=timeout, settings=settings, logger=logger):
  pass
 
@@ -209,6 +218,9 @@ async def serve_slowly():
 
  with timer, pytest.raises(asyncio.TimeoutError):
  settings.networking.request_timeout = 0.1
+ # aiohttp raises an asyncio.TimeoutError which is automatically retried.
+ # To reduce the test duration we disable retries for this test.
+ settings.networking.error_backoffs = None
  async for _ in stream('/url', settings=settings, logger=logger):
  pass
 

tests/apis/test_error_retries.py

@@ -1,3 +1,5 @@
+import asyncio
+
 import aiohttp.web
 import pytest
 
@@ -97,6 +99,25 @@ async def test_connection_errors_escalate_with_retries(
  ])
 
 
+async def test_timeout_errors_escalate_with_retries(
+ caplog, assert_logs, settings, logger, resp_mocker, aresponses, hostname, request_fn):
+ caplog.set_level(0)
+
+ request_fn.side_effect = asyncio.TimeoutError()
+
+ settings.networking.error_backoffs = [0, 0, 0]
+ with pytest.raises(asyncio.TimeoutError):
+ await request('get', '/url', settings=settings, logger=logger)
+
+ assert request_fn.call_count == 4
+ assert_logs([
+ "attempt #1/4 failed; will retry",
+ "attempt #2/4 failed; will retry",
+ "attempt #3/4 failed; will retry",
+ "attempt #4/4 failed; escalating",
+ ])
+
+
 async def test_retried_until_succeeded(
  caplog, assert_logs, settings, logger, resp_mocker, aresponses, hostname):
  caplog.set_level(0)