From 57cb40ee9a7ad964140650b0656ccdc9c4252810 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 17 Jul 2025 12:07:00 -0700 Subject: [PATCH] simplify request exception handling (#10363) my desire to do this came from the discussion at https://github.com/certbot/certbot/pull/10358#discussion_r2198273164 the code i'm deleting here came from https://github.com/certbot/certbot/pull/4733 i think doing string parsing on the exception like this is convoluted and overkill. i also agree with erica from the linked thread above that we shouldn't be raising a ValueError here, especially when the docstring for this function says `:raises requests.exceptions.RequestException: in case of any problems` and doesn't mention ValueError i prefer we do the simple thing and just delete the code. in the my opinion unlikely event we decide polishing this important, i think we can reconsider more complex approaches here --- acme/src/acme/_internal/tests/client_test.py | 14 ----------- acme/src/acme/client.py | 26 +------------------- 2 files changed, 1 insertion(+), 39 deletions(-) diff --git a/acme/src/acme/_internal/tests/client_test.py b/acme/src/acme/_internal/tests/client_test.py index 521427b9c..885c6ac5b 100644 --- a/acme/src/acme/_internal/tests/client_test.py +++ b/acme/src/acme/_internal/tests/client_test.py @@ -884,20 +884,6 @@ class ClientNetworkTest(unittest.TestCase): with pytest.raises(requests.exceptions.RequestException): self.net._send_request('GET', 'uri') - def test_urllib_error(self): - # Using a connection error to test a properly formatted error message - try: - # pylint: disable=protected-access - self.net._send_request('GET', "http://localhost:19123/nonexistent.txt") - - # Value Error Generated Exceptions - except ValueError as y: - assert "Requesting localhost/nonexistent: " \ - "Connection refused" == str(y) - - # Requests Library Exceptions - except requests.exceptions.ConnectionError as z: #pragma: no cover - assert "'Connection aborted.'" in str(z) or "[WinError 10061]" in str(z) class ClientNetworkWithMockedResponseTest(unittest.TestCase): """Tests for acme.client.ClientNetwork which mock out response.""" diff --git a/acme/src/acme/client.py b/acme/src/acme/client.py index 765025cf4..238ebd27b 100644 --- a/acme/src/acme/client.py +++ b/acme/src/acme/client.py @@ -6,7 +6,6 @@ import http.client as http_client import logging import math import random -import re import time from typing import Any from typing import cast @@ -745,30 +744,7 @@ class ClientNetwork: kwargs.setdefault('headers', {}) kwargs['headers'].setdefault('User-Agent', self.user_agent) kwargs.setdefault('timeout', self._default_timeout) - try: - response = self.session.request(method, url, *args, **kwargs) - except requests.exceptions.RequestException as e: - # pylint: disable=pointless-string-statement - """Requests response parsing - - The requests library emits exceptions with a lot of extra text. - We parse them with a regexp to raise a more readable exceptions. - - Example: - HTTPSConnectionPool(host='acme-v01.api.letsencrypt.org', - port=443): Max retries exceeded with url: /directory - (Caused by NewConnectionError(' - : Failed to establish a new connection: - [Errno 65] No route to host',))""" - - # pylint: disable=line-too-long - err_regex = r".*host='(\S*)'.*Max retries exceeded with url\: (\/\w*).*(\[Errno \d+\])([A-Za-z ]*)" - m = re.match(err_regex, str(e)) - if m is None: - raise # pragma: no cover - host, path, _err_no, err_msg = m.groups() - raise ValueError(f"Requesting {host}{path}:{err_msg}") + response = self.session.request(method, url, *args, **kwargs) # If an Accept header was sent in the request, the response may not be # UTF-8 encoded. In this case, we don't set response.encoding and log