Skip to content

Commit 94cbb84

Browse files
committed
Revert "Merge pull request #353 from sorlen008/feat/retry-backoff"
This reverts commit 77e301a, reversing changes made to aaf687f.
1 parent 77e301a commit 94cbb84

3 files changed

Lines changed: 41 additions & 516 deletions

File tree

garminconnect/__init__.py

Lines changed: 41 additions & 234 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,10 @@
1414

1515
import requests
1616
from requests import HTTPError
17-
from tenacity import (
18-
Retrying,
19-
before_sleep_log,
20-
retry_if_exception,
21-
stop_after_attempt,
22-
wait_exponential_jitter,
23-
)
2417

2518
from . import client
2619
from .fit import FitEncoderWeight # type: ignore
2720

28-
# Regex used to extract an HTTP status code from the client's error messages.
29-
# The underlying client raises GarminConnectConnectionError with a message of
30-
# the form "API Error {status} - {detail}", so we parse it to decide whether
31-
# a failure is retryable (5xx) or not (4xx, auth, etc.).
32-
_STATUS_CODE_RE = re.compile(r"(?:API Error|Error|HTTP)\s*(\d{3})")
33-
3421
logger = logging.getLogger(__name__)
3522

3623
# Constants for validation
@@ -111,85 +98,6 @@ def _validate_json_exists(response: requests.Response) -> dict[str, Any] | None:
11198
return response.json()
11299

113100

114-
def _extract_status_code(exc: BaseException) -> int | None:
115-
"""Best-effort extraction of an HTTP status code from an exception.
116-
117-
Checks a ``status_code`` attribute, then a ``response.status_code``
118-
attribute, and finally parses common status-code patterns out of the
119-
exception message (the client raises ``GarminConnectConnectionError``
120-
with messages like ``"API Error 503 - ..."``).
121-
"""
122-
status = getattr(exc, "status_code", None)
123-
if isinstance(status, int):
124-
return status
125-
126-
resp = getattr(exc, "response", None)
127-
status = getattr(resp, "status_code", None)
128-
if isinstance(status, int):
129-
return status
130-
131-
match = _STATUS_CODE_RE.search(str(exc))
132-
if match:
133-
return int(match.group(1))
134-
return None
135-
136-
137-
def _has_network_cause(exc: BaseException) -> bool:
138-
"""Return True if the exception's cause chain includes a raw network error.
139-
140-
Walks ``__cause__`` / ``__context__`` looking for ``requests.ConnectionError``
141-
or ``requests.Timeout``. Used to decide whether a status-less
142-
``GarminConnectConnectionError`` is actually a transient network failure
143-
(retry) or a deterministic logic / decode error (do not retry).
144-
"""
145-
seen: set[int] = set()
146-
cur: BaseException | None = exc
147-
while cur is not None and id(cur) not in seen:
148-
seen.add(id(cur))
149-
if isinstance(cur, (requests.ConnectionError, requests.Timeout)):
150-
return True
151-
cur = cur.__cause__ or cur.__context__
152-
return False
153-
154-
155-
def _is_retryable_error(exc: BaseException) -> bool:
156-
"""Return True for transient errors worth retrying.
157-
158-
Retries:
159-
- ``GarminConnectConnectionError`` with a 5xx status (server errors).
160-
- ``GarminConnectConnectionError`` whose cause chain contains a raw
161-
``requests.ConnectionError`` or ``requests.Timeout`` (true network
162-
failures where the request never got a response).
163-
- Raw ``requests`` network errors (connection / timeout) from lower
164-
layers that escape without being wrapped.
165-
166-
Does NOT retry:
167-
- ``GarminConnectAuthenticationError`` (401 — user must re-login).
168-
- ``GarminConnectTooManyRequestsError`` (429 — caller should back off).
169-
- Any error with a 4xx status code.
170-
- Status-less ``GarminConnectConnectionError`` with no network cause
171-
(e.g. JSON decode errors, ``AttributeError``, programming bugs).
172-
Retrying these would be pointless and waste time.
173-
"""
174-
# Never retry auth failures or rate-limit errors.
175-
if isinstance(
176-
exc, (GarminConnectAuthenticationError, GarminConnectTooManyRequestsError)
177-
):
178-
return False
179-
180-
if isinstance(exc, GarminConnectConnectionError):
181-
status = _extract_status_code(exc)
182-
if status is None:
183-
# Only retry when we can confirm the underlying cause was an
184-
# actual network failure — not a deterministic error wrapped in
185-
# a generic GarminConnectConnectionError.
186-
return _has_network_cause(exc)
187-
return 500 <= status < 600
188-
189-
# Raw network errors from requests that somehow escaped the wrappers.
190-
return isinstance(exc, (requests.ConnectionError, requests.Timeout))
191-
192-
193101
class Garmin:
194102
"""Class for fetching data from Garmin Connect."""
195103

@@ -200,20 +108,8 @@ def __init__(
200108
is_cn: bool = False,
201109
prompt_mfa: Callable[[], str] | None = None,
202110
return_on_mfa: bool = False,
203-
max_retries: int = 3,
204-
retry_min_wait: float = 1.0,
205-
retry_max_wait: float = 10.0,
206111
) -> None:
207-
"""Create a new class instance.
208-
209-
:param max_retries: Number of retries for transient network / 5xx
210-
errors on ``connectapi``, ``connectwebproxy`` and ``download``.
211-
Set to ``0`` to disable retries entirely. Defaults to ``3``.
212-
:param retry_min_wait: Initial backoff in seconds between retries
213-
(exponential with jitter). Defaults to ``1.0``.
214-
:param retry_max_wait: Upper bound on the backoff in seconds between
215-
retries. Defaults to ``10.0``.
216-
"""
112+
"""Create a new class instance."""
217113
# Validate input types
218114
if email is not None and not isinstance(email, str):
219115
raise ValueError("email must be a string or None")
@@ -223,29 +119,12 @@ def __init__(
223119
raise ValueError("is_cn must be a boolean")
224120
if not isinstance(return_on_mfa, bool):
225121
raise ValueError("return_on_mfa must be a boolean")
226-
if isinstance(max_retries, bool) or not isinstance(max_retries, int):
227-
raise ValueError("max_retries must be an integer")
228-
if max_retries < 0:
229-
raise ValueError("max_retries must be non-negative")
230-
if isinstance(retry_min_wait, bool) or not isinstance(
231-
retry_min_wait, (int, float)
232-
):
233-
raise ValueError("retry_min_wait must be a number")
234-
if isinstance(retry_max_wait, bool) or not isinstance(
235-
retry_max_wait, (int, float)
236-
):
237-
raise ValueError("retry_max_wait must be a number")
238-
if retry_min_wait < 0 or retry_max_wait < 0:
239-
raise ValueError("retry waits must be non-negative")
240122

241123
self.username = email
242124
self.password = password
243125
self.is_cn = is_cn
244126
self.prompt_mfa = prompt_mfa
245127
self.return_on_mfa = return_on_mfa
246-
self.max_retries = max_retries
247-
self.retry_min_wait = float(retry_min_wait)
248-
self.retry_max_wait = float(retry_max_wait)
249128

250129
self.garmin_connect_user_settings_url = (
251130
"/userprofile-service/userprofile/user-settings"
@@ -442,52 +321,18 @@ def __init__(
442321
self.full_name = None
443322
self.unit_system = None
444323

445-
def _retry_wrapper(self, func: Callable[[], Any], op: str, path: str) -> Any:
446-
"""Invoke ``func`` with exponential-backoff retries for transient errors.
447-
448-
Retries only when ``_is_retryable_error`` returns True (5xx server
449-
errors and bare network failures). 4xx, auth and rate-limit errors
450-
propagate immediately. Uses ``reraise=True`` so the original
451-
exception is preserved for callers.
452-
"""
453-
if self.max_retries <= 0:
454-
return func()
455-
456-
retrying = Retrying(
457-
stop=stop_after_attempt(self.max_retries + 1),
458-
wait=wait_exponential_jitter(
459-
initial=self.retry_min_wait, max=self.retry_max_wait
460-
),
461-
retry=retry_if_exception(_is_retryable_error),
462-
before_sleep=before_sleep_log(logger, logging.WARNING),
463-
reraise=True,
464-
)
465-
for attempt in retrying:
466-
with attempt:
467-
return func()
468-
# Unreachable — Retrying either returns a value or reraises.
469-
raise RuntimeError(f"{op} {path}: retry loop exited without a result")
470-
471324
def connectapi(self, path: str, **kwargs: Any) -> Any:
472-
"""Wrapper for native connectapi with error handling and retries."""
473-
return self._retry_wrapper(
474-
lambda: self._connectapi_once(path, **kwargs), "connectapi", path
475-
)
476-
477-
def _connectapi_once(self, path: str, **kwargs: Any) -> Any:
478-
"""Single (non-retried) connectapi call with error translation.
479-
480-
On auth (401) and rate-limit (429) we translate to the dedicated
481-
exception types. All other ``GarminConnectConnectionError`` instances
482-
are re-raised unchanged so callers (e.g. ``get_gear_stats``) can still
483-
read ``.response.status_code`` and any other attributes attached by
484-
the lower-level client.
485-
"""
325+
"""Wrapper for native connectapi with error handling."""
486326
try:
487327
return self.client.connectapi(path, **kwargs)
488328

489-
except GarminConnectConnectionError as e:
490-
status = _extract_status_code(e)
329+
except (HTTPError, GarminConnectConnectionError) as e:
330+
# For GarminConnectConnectionError, extract status from the wrapped HTTPError
331+
if isinstance(e, GarminConnectConnectionError):
332+
status = getattr(getattr(e, "response", None), "status_code", None)
333+
else:
334+
status = getattr(getattr(e, "response", None), "status_code", None)
335+
491336
logger.exception(
492337
"API call failed for path '%s': %s (status=%s)", path, e, status
493338
)
@@ -499,49 +344,22 @@ def _connectapi_once(self, path: str, **kwargs: Any) -> Any:
499344
raise GarminConnectTooManyRequestsError(
500345
f"Rate limit exceeded: {e}"
501346
) from e
502-
# Re-raise original so .response / existing attributes survive.
503-
raise
504-
except HTTPError as e:
505-
status = getattr(getattr(e, "response", None), "status_code", None)
506-
logger.exception(
507-
"API call failed for path '%s': %s (status=%s)", path, e, status
508-
)
509-
if status == 401:
510-
raise GarminConnectAuthenticationError(
511-
f"Authentication failed: {e}"
512-
) from e
513-
if status == 429:
514-
raise GarminConnectTooManyRequestsError(
515-
f"Rate limit exceeded: {e}"
347+
if status and 400 <= status < 500:
348+
# Client errors (400-499) - API endpoint issues, bad parameters, etc.
349+
raise GarminConnectConnectionError(
350+
f"API client error ({status}): {e}"
516351
) from e
517-
new_exc = GarminConnectConnectionError(f"HTTP error: {e}")
518-
# Preserve response so callers can still introspect .response.status_code.
519-
if getattr(e, "response", None) is not None:
520-
new_exc.response = e.response # type: ignore[attr-defined]
521-
raise new_exc from e
522-
except (requests.ConnectionError, requests.Timeout) as e:
523-
logger.exception("Network error during connectapi path=%s", path)
352+
raise GarminConnectConnectionError(f"HTTP error: {e}") from e
353+
except Exception as e:
354+
logger.exception("Connection error during connectapi path=%s", path)
524355
raise GarminConnectConnectionError(f"Connection error: {e}") from e
525356

526357
def connectwebproxy(self, path: str, **kwargs: Any) -> Any:
527-
"""Wrapper for web proxy requests with error handling and retries."""
528-
return self._retry_wrapper(
529-
lambda: self._connectwebproxy_once(path, **kwargs),
530-
"connectwebproxy",
531-
path,
532-
)
533-
534-
def _connectwebproxy_once(self, path: str, **kwargs: Any) -> Any:
535-
"""Single (non-retried) web proxy call with error translation.
536-
537-
Translates auth (401) / rate-limit (429) to the dedicated exception
538-
types. Other ``GarminConnectConnectionError`` instances propagate
539-
unchanged so ``.response`` and any attached attributes survive.
540-
"""
358+
"""Wrapper for web proxy requests to connect.garmin.com with error handling."""
541359
try:
542360
return self.client.request("GET", "connect", path, **kwargs).json()
543361
except GarminConnectConnectionError as e:
544-
status = _extract_status_code(e)
362+
status = getattr(getattr(e, "response", None), "status_code", None)
545363
logger.exception(
546364
"API call failed for web proxy path '%s' (status=%s)",
547365
path,
@@ -555,50 +373,39 @@ def _connectwebproxy_once(self, path: str, **kwargs: Any) -> Any:
555373
raise GarminConnectTooManyRequestsError(
556374
f"Web proxy rate limit: {e}"
557375
) from e
558-
# Re-raise original so .response / attributes survive.
559-
raise
560-
except (requests.ConnectionError, requests.Timeout) as e:
561-
logger.exception("Network error during web proxy path=%s", path)
376+
if status and 400 <= status < 500:
377+
raise GarminConnectConnectionError(
378+
f"Web proxy client error ({status}): {e}"
379+
) from e
380+
raise GarminConnectConnectionError(f"Web proxy error: {e}") from e
381+
except Exception as e:
382+
logger.exception("Connection error during web proxy path=%s", path)
562383
raise GarminConnectConnectionError(f"Connection error: {e}") from e
563384

564385
def download(self, path: str, **kwargs: Any) -> Any:
565-
"""Wrapper for native download with error handling and retries."""
566-
return self._retry_wrapper(
567-
lambda: self._download_once(path, **kwargs), "download", path
568-
)
569-
570-
def _download_once(self, path: str, **kwargs: Any) -> Any:
571-
"""Single (non-retried) download call with error translation.
572-
573-
Translates auth (401) / rate-limit (429) to the dedicated exception
574-
types. Other ``GarminConnectConnectionError`` instances propagate
575-
unchanged so callers retain access to ``.response`` and any attached
576-
attributes.
577-
"""
386+
"""Wrapper for native download with error handling."""
578387
try:
579388
return self.client.download(path, **kwargs)
580-
except GarminConnectConnectionError as e:
581-
status = _extract_status_code(e)
582-
logger.exception("Download failed for path '%s' (status=%s)", path, status)
583-
if status == 401:
584-
raise GarminConnectAuthenticationError(f"Download error: {e}") from e
585-
if status == 429:
586-
raise GarminConnectTooManyRequestsError(f"Download error: {e}") from e
587-
# Re-raise original so .response / attributes survive.
588-
raise
589-
except HTTPError as e:
590-
status = getattr(getattr(e, "response", None), "status_code", None)
389+
except (HTTPError, GarminConnectConnectionError) as e:
390+
# For GarminConnectConnectionError, extract status from the wrapped HTTPError
391+
if isinstance(e, GarminConnectConnectionError):
392+
status = getattr(getattr(e, "response", None), "status_code", None)
393+
else:
394+
status = getattr(getattr(e, "response", None), "status_code", None)
395+
591396
logger.exception("Download failed for path '%s' (status=%s)", path, status)
592397
if status == 401:
593398
raise GarminConnectAuthenticationError(f"Download error: {e}") from e
594399
if status == 429:
595400
raise GarminConnectTooManyRequestsError(f"Download error: {e}") from e
596-
new_exc = GarminConnectConnectionError(f"Download error: {e}")
597-
if getattr(e, "response", None) is not None:
598-
new_exc.response = e.response # type: ignore[attr-defined]
599-
raise new_exc from e
600-
except (requests.ConnectionError, requests.Timeout) as e:
601-
logger.exception("Network error during download path=%s", path)
401+
if status and 400 <= status < 500:
402+
# Client errors (400-499) - API endpoint issues, bad parameters, etc.
403+
raise GarminConnectConnectionError(
404+
f"Download client error ({status}): {e}"
405+
) from e
406+
raise GarminConnectConnectionError(f"Download error: {e}") from e
407+
except Exception as e:
408+
logger.exception("Download failed for path '%s'", path)
602409
raise GarminConnectConnectionError(f"Download error: {e}") from e
603410

604411
def login(self, /, tokenstore: str | None = None) -> tuple[str | None, str | None]:

pyproject.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ authors = [
88
dependencies = [
99
"curl_cffi>=0.6",
1010
"requests>=2.28",
11-
"tenacity>=8.2",
1211
"ua-generator>=1.0",
1312
]
1413
readme = "README.md"
@@ -72,7 +71,6 @@ linting = [
7271
testing = [
7372
"coverage",
7473
"pytest",
75-
"pytest-mock>=3.10",
7674
"pytest-vcr>=1.0.2",
7775
"vcrpy>=7.0.0",
7876
]
@@ -242,7 +240,6 @@ linting = [
242240
testing = [
243241
"coverage",
244242
"pytest",
245-
"pytest-mock>=3.10",
246243
"pytest-vcr>=1.0.2",
247244
"vcrpy>=7.0.0",
248245
]

0 commit comments

Comments
 (0)