diff --git a/sdk/core/azure-core/azure/core/pipeline/policies/_retry.py b/sdk/core/azure-core/azure/core/pipeline/policies/_retry.py index 3e8ac4cf0fab..b683e7e08ab2 100644 --- a/sdk/core/azure-core/azure/core/pipeline/policies/_retry.py +++ b/sdk/core/azure-core/azure/core/pipeline/policies/_retry.py @@ -455,7 +455,7 @@ def send(self, request): # succeed--we'll never have a response to it, so propagate the exception raise except AzureError as err: - if self._is_method_retryable(retry_settings, request.http_request): + if absolute_timeout > 0 and self._is_method_retryable(retry_settings, request.http_request): retry_active = self.increment(retry_settings, response=request, error=err) if retry_active: self.sleep(retry_settings, request.context.transport) diff --git a/sdk/core/azure-core/azure/core/pipeline/policies/_retry_async.py b/sdk/core/azure-core/azure/core/pipeline/policies/_retry_async.py index ca37a7bf5565..f2e241dde041 100644 --- a/sdk/core/azure-core/azure/core/pipeline/policies/_retry_async.py +++ b/sdk/core/azure-core/azure/core/pipeline/policies/_retry_async.py @@ -157,7 +157,7 @@ async def send(self, request): # pylint:disable=invalid-overridden-method # succeed--we'll never have a response to it, so propagate the exception raise except AzureError as err: - if self._is_method_retryable(retry_settings, request.http_request): + if absolute_timeout > 0 and self._is_method_retryable(retry_settings, request.http_request): retry_active = self.increment(retry_settings, response=request, error=err) if retry_active: await self.sleep(retry_settings, request.context.transport) diff --git a/sdk/core/azure-core/tests/async_tests/test_retry_policy_async.py b/sdk/core/azure-core/tests/async_tests/test_retry_policy_async.py index 01b8f18090de..e23a1a35ee46 100644 --- a/sdk/core/azure-core/tests/async_tests/test_retry_policy_async.py +++ b/sdk/core/azure-core/tests/async_tests/test_retry_policy_async.py @@ -11,7 +11,13 @@ from unittest.mock import Mock import pytest from azure.core.configuration import ConnectionConfiguration -from azure.core.exceptions import AzureError, ServiceResponseError, ServiceResponseTimeoutError +from azure.core.exceptions import ( + AzureError, + ServiceRequestError, + ServiceRequestTimeoutError, + ServiceResponseError, + ServiceResponseTimeoutError +) from azure.core.pipeline.policies import ( AsyncRetryPolicy, RetryMode, @@ -258,3 +264,26 @@ async def send(request, **kwargs): await pipeline.run(HttpRequest("GET", "http://127.0.0.1/")) assert transport.send.call_count == 1, "policy should not retry: its first send succeeded" + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "transport_error,expected_timeout_error", + ((ServiceRequestError, ServiceRequestTimeoutError), (ServiceResponseError, ServiceResponseTimeoutError)), +) +async def test_does_not_sleep_after_timeout(transport_error, expected_timeout_error): + # With default settings policy will sleep twice before exhausting its retries: 1.6s, 3.2s. + # It should not sleep the second time when given timeout=1 + timeout = 1 + + transport = Mock( + spec=AsyncHttpTransport, + send=Mock(side_effect=transport_error("oops")), + sleep=Mock(wraps=asyncio.sleep), + ) + pipeline = AsyncPipeline(transport, [AsyncRetryPolicy(timeout=timeout)]) + + with pytest.raises(expected_timeout_error): + await pipeline.run(HttpRequest("GET", "http://127.0.0.1/")) + + assert transport.sleep.call_count == 1 diff --git a/sdk/core/azure-core/tests/test_retry_policy.py b/sdk/core/azure-core/tests/test_retry_policy.py index 528c6f5514ce..9980b5367ee2 100644 --- a/sdk/core/azure-core/tests/test_retry_policy.py +++ b/sdk/core/azure-core/tests/test_retry_policy.py @@ -9,7 +9,13 @@ from cStringIO import StringIO as BytesIO import pytest from azure.core.configuration import ConnectionConfiguration -from azure.core.exceptions import AzureError, ServiceResponseError, ServiceResponseTimeoutError +from azure.core.exceptions import ( + AzureError, + ServiceRequestError, + ServiceRequestTimeoutError, + ServiceResponseError, + ServiceResponseTimeoutError, +) from azure.core.pipeline.policies import ( RetryPolicy, RetryMode, @@ -255,3 +261,25 @@ def send(request, **kwargs): pipeline.run(HttpRequest("GET", "http://127.0.0.1/")) assert transport.send.call_count == 1, "policy should not retry: its first send succeeded" + + +@pytest.mark.parametrize( + "transport_error,expected_timeout_error", + ((ServiceRequestError, ServiceRequestTimeoutError), (ServiceResponseError, ServiceResponseTimeoutError)), +) +def test_does_not_sleep_after_timeout(transport_error, expected_timeout_error): + # With default settings policy will sleep twice before exhausting its retries: 1.6s, 3.2s. + # It should not sleep the second time when given timeout=1 + timeout = 1 + + transport = Mock( + spec=HttpTransport, + send=Mock(side_effect=transport_error("oops")), + sleep=Mock(wraps=time.sleep), + ) + pipeline = Pipeline(transport, [RetryPolicy(timeout=timeout)]) + + with pytest.raises(expected_timeout_error): + pipeline.run(HttpRequest("GET", "http://127.0.0.1/")) + + assert transport.sleep.call_count == 1