Skip to content

fix: catch and retry on ConnectionError/Timeout from niquests (issue #647) #648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
tobixen wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix: catch and retry on ConnectionError/Timeout from niquests (issue #647) #648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 105 additions & 84 deletions caldav/async_davclient.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def auth_flow(self, request):
from caldav.base_client import BaseDAVClient
from caldav.base_client import get_davclient as _base_get_davclient
from caldav.compatibility_hints import FeatureSet
from caldav.davclient import _SAFE_METHODS
from caldav.lib import error
from caldav.lib.python_utilities import to_wire
from caldav.lib.url import URL
Expand All @@ -92,6 +93,12 @@ def auth_flow(self, request):

log = logging.getLogger("caldav")

# Network-level exceptions for the active async HTTP library.
if _USE_HTTPX:
_NETWORK_EXCEPTIONS: tuple = (httpx.ConnectError, httpx.TimeoutException, httpx.NetworkError)
else:
_NETWORK_EXCEPTIONS = (niquests.exceptions.ConnectionError, niquests.exceptions.Timeout)

if sys.version_info < (3, 11):
from typing_extensions import Self
else:
Expand Down Expand Up @@ -404,6 +411,7 @@ async def _async_request(
method: str = "GET",
body: str = "",
headers: Mapping[str, str] | None = None,
_connection_retried: bool = False,
) -> AsyncDAVResponse:
"""
Async HTTP request implementation with auth negotiation.
Expand Down Expand Up @@ -441,96 +449,109 @@ async def _async_request(
}

try:
r = await self.session.request(**request_kwargs)
reason = r.reason_phrase if _USE_HTTPX else r.reason
log.debug(f"server responded with {r.status_code} {reason}")
if (
try:
r = await self.session.request(**request_kwargs)
reason = r.reason_phrase if _USE_HTTPX else r.reason
log.debug(f"server responded with {r.status_code} {reason}")
if (
r.status_code == 401
and "text/html" in self.headers.get("Content-Type", "")
and not self.auth
):
msg = (
"No authentication object was provided. "
"HTML was returned when probing the server for supported authentication types. "
"To avoid logging errors, consider passing the auth_type connection parameter"
)
if r.headers.get("WWW-Authenticate"):
auth_types = [
t
for t in self.extract_auth_types(r.headers["WWW-Authenticate"])
if t in ["basic", "digest", "bearer"]
]
if auth_types:
msg += "\nSupported authentication types: {}".format(
", ".join(auth_types)
)
log.warning(msg)
response = AsyncDAVResponse(r, self)
except Exception:
# Workaround for servers that abort connection on unauthenticated requests
# ref https://github.com/python-caldav/caldav/issues/158
if self.auth or not self.password:
raise
# Build minimal request for auth detection
if _USE_HTTPX:
r = await self.session.request(
method="GET",
url=str(url_obj),
headers=combined_headers,
timeout=self.timeout,
)
else:
proxies = None
if self.proxy is not None:
proxies = {url_obj.scheme: self.proxy}
r = await self.session.request(
method="GET",
url=str(url_obj),
headers=combined_headers,
timeout=self.timeout,
proxies=proxies,
verify=self.ssl_verify_cert,
cert=self.ssl_cert,
)
reason = r.reason_phrase if _USE_HTTPX else r.reason
log.debug(f"auth type detection: server responded with {r.status_code} {reason}")
if r.status_code == 401 and r.headers.get("WWW-Authenticate"):
auth_types = self.extract_auth_types(r.headers["WWW-Authenticate"])
self.build_auth_object(auth_types)
# Retry original request with auth
request_kwargs["auth"] = self.auth
r = await self.session.request(**request_kwargs)
response = AsyncDAVResponse(r, self)

# Handle 429/503 rate-limit responses
error.raise_if_rate_limited(r.status_code, str(url_obj), r.headers.get("Retry-After"))

# Handle 401: negotiate auth then retry
if self._should_negotiate_auth(r.status_code, r.headers):
self._build_auth_from_401(r.headers["WWW-Authenticate"])
return await self._async_request(url, method, body, headers)

elif (
r.status_code == 401
and "text/html" in self.headers.get("Content-Type", "")
and not self.auth
and "WWW-Authenticate" in r.headers
and self.auth
and self.features.is_supported("http.multiplexing", return_defaults=False) is None
):
msg = (
"No authentication object was provided. "
"HTML was returned when probing the server for supported authentication types. "
"To avoid logging errors, consider passing the auth_type connection parameter"
)
if r.headers.get("WWW-Authenticate"):
auth_types = [
t
for t in self.extract_auth_types(r.headers["WWW-Authenticate"])
if t in ["basic", "digest", "bearer"]
]
if auth_types:
msg += "\nSupported authentication types: {}".format(", ".join(auth_types))
log.warning(msg)
response = AsyncDAVResponse(r, self)
except Exception:
# Workaround for servers that abort connection on unauthenticated requests
# ref https://github.com/python-caldav/caldav/issues/158
if self.auth or not self.password:
raise
# Build minimal request for auth detection
if _USE_HTTPX:
r = await self.session.request(
method="GET",
url=str(url_obj),
headers=combined_headers,
timeout=self.timeout,
)
else:
proxies = None
if self.proxy is not None:
proxies = {url_obj.scheme: self.proxy}
r = await self.session.request(
method="GET",
url=str(url_obj),
headers=combined_headers,
timeout=self.timeout,
proxies=proxies,
verify=self.ssl_verify_cert,
cert=self.ssl_cert,
)
reason = r.reason_phrase if _USE_HTTPX else r.reason
log.debug(f"auth type detection: server responded with {r.status_code} {reason}")
if r.status_code == 401 and r.headers.get("WWW-Authenticate"):
auth_types = self.extract_auth_types(r.headers["WWW-Authenticate"])
self.build_auth_object(auth_types)
# Retry original request with auth
request_kwargs["auth"] = self.auth
r = await self.session.request(**request_kwargs)
response = AsyncDAVResponse(r, self)
# Handle HTTP/2 multiplexing issue: most likely wrong username/password, but could
# be an HTTP/2 problem. Retry with HTTP/2 disabled if multiplexing was auto-detected.
await self.close()
self._http2 = False
self._create_session()
# Set multiplexing to False BEFORE retry to prevent infinite loop
self.features.set_feature("http.multiplexing", False)
return await self._async_request(str(url_obj), method, body, headers)

# Handle 429/503 rate-limit responses
error.raise_if_rate_limited(r.status_code, str(url_obj), r.headers.get("Retry-After"))
# Raise AuthorizationError for 401/403 responses
if response.status in (401, 403):
self._raise_authorization_error(str(url_obj), response)

# Handle 401: negotiate auth then retry
if self._should_negotiate_auth(r.status_code, r.headers):
self._build_auth_from_401(r.headers["WWW-Authenticate"])
return await self._async_request(url, method, body, headers)

elif (
r.status_code == 401
and "WWW-Authenticate" in r.headers
and self.auth
and self.features.is_supported("http.multiplexing", return_defaults=False) is None
):
# Handle HTTP/2 multiplexing issue: most likely wrong username/password, but could
# be an HTTP/2 problem. Retry with HTTP/2 disabled if multiplexing was auto-detected.
await self.close()
self._http2 = False
self._create_session()
# Set multiplexing to False BEFORE retry to prevent infinite loop
self.features.set_feature("http.multiplexing", False)
return await self._async_request(str(url_obj), method, body, headers)
if error.debug_dump_communication:
error._dump_communication(method, url, combined_headers, body, response)

# Raise AuthorizationError for 401/403 responses
if response.status in (401, 403):
self._raise_authorization_error(str(url_obj), response)
return response

if error.debug_dump_communication:
error._dump_communication(method, url, combined_headers, body, response)

return response
except error.DAVError:
raise
except _NETWORK_EXCEPTIONS as e:
if not _connection_retried and method.upper() in _SAFE_METHODS:
log.warning("Network error on %s %s, retrying once", method, url)
return await self._async_request(
url, method, body, headers, _connection_retried=True
)
raise error.DAVNetworkError(str(url_obj), str(e)) from e

# ==================== HTTP Method Wrappers ====================
# Query methods (URL optional - defaults to self.url)
Expand Down
75 changes: 50 additions & 25 deletions caldav/davclient.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@

from caldav.config import resolve_features as _resolve_features

# CalDAV/WebDAV methods that are safe to retry on transient network errors.
# POST is the only non-idempotent method and is excluded. All others are
# idempotent: re-sending yields the same server state. Note that retrying
# DELETE after a silent success will return 404; this is arguably correct
# (the resource is gone as intended), though callers may observe the error.
_SAFE_METHODS: frozenset[str] = frozenset(
{"GET", "HEAD", "OPTIONS", "PROPFIND", "REPORT", "PUT", "DELETE", "MKCOL", "MKCALENDAR"}
)


def _auto_url(
url,
Expand Down Expand Up @@ -857,6 +866,7 @@ def request(
body: str = "",
headers: Mapping[str, str] = None,
rate_limit_time_slept=0,
_connection_retried: bool = False,
) -> DAVResponse:
"""
Send a generic HTTP request.
Expand Down Expand Up @@ -891,6 +901,13 @@ def request(
raise
time.sleep(sleep_seconds)
return self.request(url, method, body, headers, rate_limit_time_slept + sleep_seconds)
except error.DAVNetworkError:
if not _connection_retried and method.upper() in _SAFE_METHODS:
log.warning("Network error on %s %s, retrying once", method, url)
return self.request(
url, method, body, headers, rate_limit_time_slept, _connection_retried=True
)
raise

def _sync_request(
self,
Expand All @@ -909,38 +926,46 @@ def _sync_request(
proxies = {url_obj.scheme: self.proxy}
log.debug("using proxy - %s" % (proxies))

r = self.session.request(
method,
str(url_obj),
data=to_wire(body),
headers=combined_headers,
proxies=proxies,
auth=self.auth,
timeout=self.timeout,
verify=self.ssl_verify_cert,
cert=self.ssl_cert,
)
try:
r = self.session.request(
method,
str(url_obj),
data=to_wire(body),
headers=combined_headers,
proxies=proxies,
auth=self.auth,
timeout=self.timeout,
verify=self.ssl_verify_cert,
cert=self.ssl_cert,
)

r_headers = CaseInsensitiveDict(r.headers)
r_headers = CaseInsensitiveDict(r.headers)

# Handle 429/503 responses: raise RateLimitError so the caller can decide whether to retry
error.raise_if_rate_limited(r.status_code, str(url_obj), r_headers.get("Retry-After"))
# Handle 429/503 responses: raise RateLimitError so the caller can decide whether to retry
error.raise_if_rate_limited(r.status_code, str(url_obj), r_headers.get("Retry-After"))

# Handle 401: negotiate auth then retry
if self._should_negotiate_auth(r.status_code, r_headers):
self._build_auth_from_401(r_headers["WWW-Authenticate"])
return self._sync_request(url, method, body, headers)
# Handle 401: negotiate auth then retry
if self._should_negotiate_auth(r.status_code, r_headers):
self._build_auth_from_401(r_headers["WWW-Authenticate"])
return self._sync_request(url, method, body, headers)

# Raise AuthorizationError for 401/403 after auth attempt
if r.status_code in (401, 403):
self._raise_authorization_error(str(url_obj), r)
# Raise AuthorizationError for 401/403 after auth attempt
if r.status_code in (401, 403):
self._raise_authorization_error(str(url_obj), r)

response = DAVResponse(r, self)
response = DAVResponse(r, self)

if error.debug_dump_communication:
error._dump_communication(method, url, combined_headers, body, response)
if error.debug_dump_communication:
error._dump_communication(method, url, combined_headers, body, response)

return response
return response

except error.DAVError:
raise
except requests.exceptions.ConnectionError as e:
raise error.DAVNetworkError(str(url_obj), str(e)) from e
except requests.exceptions.Timeout as e:
raise error.DAVNetworkError(str(url_obj), str(e)) from e


def get_calendars(**kwargs) -> list["Calendar"]:
Expand Down
13 changes: 13 additions & 0 deletions caldav/lib/error.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,19 @@ def __init__(
self.retry_after_seconds = retry_after_seconds


class DAVNetworkError(DAVError):
"""Raised on a network-level failure (timeout, connection reset, etc.)
while communicating with the server.

This typically wraps a ``requests.exceptions.ConnectionError`` or
``requests.exceptions.Timeout`` from the underlying HTTP library. The
original exception is chained via ``__cause__`` so the full traceback is
still available when debugging.
"""

pass


def parse_retry_after(retry_after_header: str) -> float | None:
"""Parse a Retry-After header value into seconds from now.

Expand Down
Loading
Loading