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
This commit is contained in:
Brad Warren
2025-07-17 12:07:00 -07:00
committed by GitHub
parent eb563ccc1d
commit 57cb40ee9a
2 changed files with 1 additions and 39 deletions
@@ -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."""
+1 -25
View File
@@ -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('
<requests.packages.urllib3.connection.VerifiedHTTPSConnection
object at 0x108356c50>: 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