diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index f0b5d9b29a31..999a9c9d4bee 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -7,6 +7,10 @@ - Support "x-ms-retry-after-ms" in response header #10743 +### Bug fixes + +- Not retry if the status code is less than 400 #10778 + ## 1.4.0 (2020-04-06) ### Features 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 28791b8cf420..26219d262f3c 100644 --- a/sdk/core/azure-core/azure/core/pipeline/policies/_retry.py +++ b/sdk/core/azure-core/azure/core/pipeline/policies/_retry.py @@ -263,12 +263,20 @@ def is_retry(self, settings, response): code is on the list of status codes to be retried upon on the presence of the aforementioned header. + The behavior is: + - If status_code < 400: don’t retry + - Else if Retry-After present: retry + - Else: retry based on the safe status code list ([408, 429, 500, 502, 503, 504]) + + :param dict settings: The retry settings. :param response: The PipelineResponse object :type response: ~azure.core.pipeline.PipelineResponse :return: True if method/status code is retryable. False if not retryable. :rtype: bool """ + if response.http_response.status_code < 400: + return False has_retry_after = bool(response.http_response.headers.get("Retry-After")) if has_retry_after and self._respect_retry_after_header: return True diff --git a/sdk/core/azure-core/tests/azure_core_asynctests/test_retry_policy.py b/sdk/core/azure-core/tests/azure_core_asynctests/test_retry_policy.py index a6647ddaf082..a03570c0def0 100644 --- a/sdk/core/azure-core/tests/azure_core_asynctests/test_retry_policy.py +++ b/sdk/core/azure-core/tests/azure_core_asynctests/test_retry_policy.py @@ -113,6 +113,33 @@ async def send(self, request, **kwargs): # type: (PipelineRequest, Any) -> Pipe await pipeline.run(http_request) assert transport._count == 2 +@pytest.mark.asyncio +async def test_no_retry_on_201(): + class MockTransport(AsyncHttpTransport): + def __init__(self): + self._count = 0 + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + async def close(self): + pass + async def open(self): + pass + + async def send(self, request, **kwargs): # type: (PipelineRequest, Any) -> PipelineResponse + self._count += 1 + response = HttpResponse(request, None) + response.status_code = 201 + headers = {"Retry-After": "1"} + response.headers = headers + return response + + http_request = HttpRequest('GET', 'http://127.0.0.1/') + http_retry = AsyncRetryPolicy(retry_total = 1) + transport = MockTransport() + pipeline = AsyncPipeline(transport, [http_retry]) + await pipeline.run(http_request) + assert transport._count == 1 + @pytest.mark.asyncio async def test_retry_seekable_stream(): class MockTransport(AsyncHttpTransport): @@ -132,7 +159,9 @@ async def send(self, request, **kwargs): # type: (PipelineRequest, Any) -> Pipe raise AzureError('fail on first') position = request.body.tell() assert position == 0 - return HttpResponse(request, None) + response = HttpResponse(request, None) + response.status_code = 400 + return response data = BytesIO(b"Lots of dataaaa") http_request = HttpRequest('GET', 'http://127.0.0.1/') @@ -166,7 +195,9 @@ async def send(self, request, **kwargs): # type: (PipelineRequest, Any) -> Pipe if name and body and hasattr(body, 'read'): position = body.tell() assert not position - return HttpResponse(request, None) + response = HttpResponse(request, None) + response.status_code = 400 + return response file = tempfile.NamedTemporaryFile(delete=False) file.write(b'Lots of dataaaa') diff --git a/sdk/core/azure-core/tests/test_retry_policy.py b/sdk/core/azure-core/tests/test_retry_policy.py index 24e4b8e623fa..5c7d0dd50d2c 100644 --- a/sdk/core/azure-core/tests/test_retry_policy.py +++ b/sdk/core/azure-core/tests/test_retry_policy.py @@ -109,6 +109,32 @@ def send(self, request, **kwargs): # type: (PipelineRequest, Any) -> PipelineRe pipeline.run(http_request) assert transport._count == 2 +def test_no_retry_on_201(): + class MockTransport(HttpTransport): + def __init__(self): + self._count = 0 + def __exit__(self, exc_type, exc_val, exc_tb): + pass + def close(self): + pass + def open(self): + pass + + def send(self, request, **kwargs): # type: (PipelineRequest, Any) -> PipelineResponse + self._count += 1 + response = HttpResponse(request, None) + response.status_code = 201 + headers = {"Retry-After": "1"} + response.headers = headers + return response + + http_request = HttpRequest('GET', 'http://127.0.0.1/') + http_retry = RetryPolicy(retry_total = 1) + transport = MockTransport() + pipeline = Pipeline(transport, [http_retry]) + pipeline.run(http_request) + assert transport._count == 1 + def test_retry_seekable_stream(): class MockTransport(HttpTransport): def __init__(self): @@ -127,7 +153,9 @@ def send(self, request, **kwargs): # type: (PipelineRequest, Any) -> PipelineRe raise AzureError('fail on first') position = request.body.tell() assert position == 0 - return HttpResponse(request, None) + response = HttpResponse(request, None) + response.status_code = 400 + return response data = BytesIO(b"Lots of dataaaa") http_request = HttpRequest('GET', 'http://127.0.0.1/') @@ -160,7 +188,9 @@ def send(self, request, **kwargs): # type: (PipelineRequest, Any) -> PipelineRe if name and body and hasattr(body, 'read'): position = body.tell() assert not position - return HttpResponse(request, None) + response = HttpResponse(request, None) + response.status_code = 400 + return response file = tempfile.NamedTemporaryFile(delete=False) file.write(b'Lots of dataaaa')