Skip to content

Commit 60051f1

Browse files
committed
Rewrite urllib3 retry warnings to be understandable, if possible
Alright, so this is 100% a hack, but we don't really have any alternatives for improving the truly user-hostile warnings that urllib3 emits when retrying after a connection error. Upstream improvements aren't possible until we are able to catch up with urllib3 2.x, but we're stuck on the 1.x series until at least 2025 due to OpenSSL incompatibilties. Thus, the approach taken here is to install a logging filter on the connectionpool.py urllib3 module (don't worry, the hacks continue[^1]) which "sniffs" for the appropriate warnings and rewrites them as best as possible with the limited context urllib3 gives us. Under verbose mode, the original error is appended as a "here's all of the context, sorry that it's unreadable" aid. [^1]: I did have to resort to parsing the timeout error strings with regex in order to get the configured timeout (since logically they're set per request, not globally or per session... doing it this way results in less buggy behaviour and tests a lot less annoying to write).
1 parent 203780b commit 60051f1

File tree

2 files changed

+90
-1
lines changed

2 files changed

+90
-1
lines changed

src/pip/_internal/network/session.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
from pip._internal.models.link import Link
4444
from pip._internal.network.auth import MultiDomainBasicAuth
4545
from pip._internal.network.cache import SafeFileCache
46+
from pip._internal.network.utils import Urllib3RetryFilter
4647

4748
# Import ssl from compat so the initial import occurs in only one place.
4849
from pip._internal.utils.compat import has_tls
@@ -64,6 +65,8 @@
6465
# Ignore warning raised when using --trusted-host.
6566
warnings.filterwarnings("ignore", category=InsecureRequestWarning)
6667

68+
# Install rewriting filter for urllib3's retrying warnings.
69+
logging.getLogger("pip._vendor.urllib3.connectionpool").addFilter(Urllib3RetryFilter())
6770

6871
SECURE_ORIGINS: List[SecureOrigin] = [
6972
# protocol, hostname, port

src/pip/_internal/network/utils.py

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
1+
import logging
2+
import re
3+
from http.client import RemoteDisconnected
14
from typing import Dict, Generator
25

6+
from pip._vendor import urllib3
37
from pip._vendor.requests.models import Response
48

5-
from pip._internal.exceptions import NetworkConnectionError
9+
from pip._internal.exceptions import (
10+
NetworkConnectionError,
11+
)
12+
from pip._internal.utils.logging import VERBOSE
613

714
# The following comments and HTTP headers were originally added by
815
# Donald Stufft in git commit 22c562429a61bb77172039e480873fb239dd8c03.
@@ -27,6 +34,8 @@
2734

2835
DOWNLOAD_CHUNK_SIZE = 256 * 1024
2936

37+
logger = logging.getLogger(__name__)
38+
3039

3140
def raise_for_status(resp: Response) -> None:
3241
http_error_msg = ""
@@ -96,3 +105,80 @@ def response_chunks(
96105
if not chunk:
97106
break
98107
yield chunk
108+
109+
110+
class Urllib3RetryFilter:
111+
"""A logging filter which attempts to rewrite urllib3's retrying
112+
warnings to be more readable and less technical.
113+
114+
This is essentially one large hack. Please enjoy...
115+
"""
116+
117+
def filter(self, record: logging.LogRecord) -> bool:
118+
# Attempt to "sniff out" the retrying warning.
119+
if not isinstance(record.args, tuple):
120+
return True
121+
122+
retry = next(
123+
(a for a in record.args if isinstance(a, urllib3.util.Retry)), None
124+
)
125+
if record.levelno != logging.WARNING or retry is None:
126+
# Not the right warning, leave it alone.
127+
return True
128+
129+
error = next((a for a in record.args if isinstance(a, Exception)), None)
130+
if error is None:
131+
# No error information available, leave it alone.
132+
return True
133+
134+
original_message = record.msg
135+
if isinstance(error, urllib3.exceptions.NewConnectionError):
136+
connection = error.pool
137+
record.msg = f"failed to connect to {connection.host}"
138+
if isinstance(connection, urllib3.connection.HTTPSConnection):
139+
record.msg += " via HTTPS"
140+
elif isinstance(connection, urllib3.connection.HTTPConnection):
141+
record.msg += " via HTTP"
142+
# After this point, urllib3 gives us very little information to work with
143+
# so the rewritten warnings will be light on details.
144+
elif isinstance(error, urllib3.exceptions.SSLError):
145+
record.msg = "SSL verification failed"
146+
elif isinstance(error, urllib3.exceptions.TimeoutError):
147+
# Ugh.
148+
pattern = r"""
149+
timeout=(?P<value>
150+
\d+ # Whole number
151+
(\.\d+)? # Decimal component (optional)
152+
)"""
153+
if match := re.search(pattern, str(error), re.VERBOSE):
154+
timeout = match.group("value")
155+
record.msg = f"server didn't respond within {timeout} seconds"
156+
else:
157+
record.msg = "server took too long to respond"
158+
elif isinstance(error, urllib3.exceptions.ProtocolError):
159+
try:
160+
reason = error.args[1]
161+
except IndexError:
162+
pass
163+
else:
164+
if isinstance(reason, (RemoteDisconnected, ConnectionResetError)):
165+
record.msg = "the connection was closed unexpectedly"
166+
elif isinstance(error, urllib3.exceptions.ProxyError):
167+
record.msg = "failed to connect to proxy"
168+
169+
if record.msg != original_message:
170+
# The total remaining retries is already decremented when this
171+
# warning is raised.
172+
retries_left = retry.total + 1
173+
if retries_left > 1:
174+
record.msg += f", retrying {retries_left} more times"
175+
elif retries_left == 1:
176+
record.msg += ", retrying 1 last time"
177+
178+
if logger.isEnabledFor(VERBOSE):
179+
# As it's hard to provide enough detail, show the original
180+
# error under verbose mode.
181+
record.msg += f": {error!s}"
182+
record.args = ()
183+
184+
return True

0 commit comments

Comments
 (0)