From 85ae88072ef4f02ef06e3d31d31d189e99c31886 Mon Sep 17 00:00:00 2001 From: annatisch Date: Tue, 1 Oct 2019 14:09:32 -0700 Subject: [PATCH 1/4] Handle requests connection timeout error --- .../azure-core/azure/core/pipeline/transport/requests_basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py b/sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py index 95757edb63dd..5b3d278efb6b 100644 --- a/sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py +++ b/sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py @@ -248,7 +248,7 @@ def send(self, request, **kwargs): # type: ignore except urllib3.exceptions.NewConnectionError as err: error = ServiceRequestError(err, error=err) - except requests.exceptions.ReadTimeout as err: + except (requests.exceptions.ReadTimeout, urllib3.exceptions.ConnectTimeoutError) as err: error = ServiceResponseError(err, error=err) except requests.exceptions.ConnectionError as err: if err.args and isinstance(err.args[0], urllib3.exceptions.ProtocolError): From 66039d8659ec9746cb0cca7e8c428f75153ebe55 Mon Sep 17 00:00:00 2001 From: annatisch Date: Tue, 1 Oct 2019 14:16:44 -0700 Subject: [PATCH 2/4] Moved catch --- .../azure/core/pipeline/transport/requests_basic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py b/sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py index 5b3d278efb6b..d18d63a436f1 100644 --- a/sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py +++ b/sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py @@ -246,9 +246,9 @@ def send(self, request, **kwargs): # type: ignore allow_redirects=False, **kwargs) - except urllib3.exceptions.NewConnectionError as err: + except (urllib3.exceptions.NewConnectionError, urllib3.exceptions.ConnectTimeoutError) as err: error = ServiceRequestError(err, error=err) - except (requests.exceptions.ReadTimeout, urllib3.exceptions.ConnectTimeoutError) as err: + except requests.exceptions.ReadTimeout as err: error = ServiceResponseError(err, error=err) except requests.exceptions.ConnectionError as err: if err.args and isinstance(err.args[0], urllib3.exceptions.ProtocolError): From f423cf10124c49d589edc891834754bfe6bb3b9e Mon Sep 17 00:00:00 2001 From: annatisch Date: Tue, 1 Oct 2019 14:25:35 -0700 Subject: [PATCH 3/4] Added test --- sdk/core/azure-core/tests/test_pipeline.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/sdk/core/azure-core/tests/test_pipeline.py b/sdk/core/azure-core/tests/test_pipeline.py index e9df06b0d72a..452601d6c9c9 100644 --- a/sdk/core/azure-core/tests/test_pipeline.py +++ b/sdk/core/azure-core/tests/test_pipeline.py @@ -58,7 +58,7 @@ RequestsTransport ) -from azure.core.configuration import Configuration +from azure.core.exceptions import ServiceRequestError def test_sans_io_exception(): @@ -107,6 +107,17 @@ def test_basic_requests(self): assert pipeline._transport.session is None assert response.http_response.status_code == 200 + def test_requests_socket_timeout(self): + conf = Configuration() + request = HttpRequest("GET", "https://bing.com") + policies = [ + UserAgentPolicy("myusergant"), + RedirectPolicy() + ] + with pytest.raises(ServiceRequestError): + with Pipeline(RequestsTransport(), policies=policies) as pipeline: + response = pipeline.run(request, connection_timeout=0.000001) + def test_basic_requests_separate_session(self): session = requests.Session() From 0f57ff53982df5edaf0afe82272a2fe56048df02 Mon Sep 17 00:00:00 2001 From: annatisch Date: Wed, 2 Oct 2019 08:47:37 -0700 Subject: [PATCH 4/4] Update test to be more stable --- sdk/core/azure-core/tests/test_pipeline.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core/tests/test_pipeline.py b/sdk/core/azure-core/tests/test_pipeline.py index 452601d6c9c9..0c8df68e03bd 100644 --- a/sdk/core/azure-core/tests/test_pipeline.py +++ b/sdk/core/azure-core/tests/test_pipeline.py @@ -58,7 +58,7 @@ RequestsTransport ) -from azure.core.exceptions import ServiceRequestError +from azure.core.exceptions import AzureError def test_sans_io_exception(): @@ -114,7 +114,10 @@ def test_requests_socket_timeout(self): UserAgentPolicy("myusergant"), RedirectPolicy() ] - with pytest.raises(ServiceRequestError): + # Sometimes this will raise a read timeout, sometimes a socket timeout depending on timing. + # Either way, the error should always be wrapped as an AzureError to ensure it's caught + # by the retry policy. + with pytest.raises(AzureError): with Pipeline(RequestsTransport(), policies=policies) as pipeline: response = pipeline.run(request, connection_timeout=0.000001)