Skip to content

Commit 8534615

Browse files
committed
refactor: unify error classification into shared _classify_transient helper
Extract the per-exception type matching from _classify_chunk_error into a new _classify_transient(exc) helper that classifies a SINGLE exception (no chain walk) into (interrupted_class, retry_after) | None. _classify_chunk_error now calls _classify_transient on each step of its __cause__ walk. _retryable continues to inspect only the top-level exception (preserving the intentional divergence: initial-page raw transients are cheap to retry; wrapped mid-pagination failures escalate to a resumable ChunkInterrupted instead). When a new transient error type needs recognition, it now goes in ONE place (_classify_transient) and both the retry and escalation paths pick it up automatically.
1 parent 3eda250 commit 8534615

1 file changed

Lines changed: 39 additions & 6 deletions

File tree

dataretrieval/ogc/retry.py

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,38 @@ def backoff(self, attempt: int, retry_after: float | None) -> float:
190190
_NO_RETRY = RetryPolicy(max_retries=0)
191191

192192

193+
def _classify_transient(
194+
exc: BaseException,
195+
) -> tuple[type[ChunkInterrupted], float | None] | None:
196+
"""
197+
Classify a SINGLE exception as a known transient (resumable) failure.
198+
199+
Does NOT walk the ``__cause__`` chain — inspects only the exception
200+
passed in. This is the shared taxonomy that both
201+
:func:`_classify_chunk_error` (chain-walking) and :func:`_retryable`
202+
(top-level only) delegate to, so a new error type need only be added
203+
in one place.
204+
205+
Parameters
206+
----------
207+
exc : BaseException
208+
A single exception to classify.
209+
210+
Returns
211+
-------
212+
tuple[type[ChunkInterrupted], float or None] or None
213+
``(interrupted_class, retry_after)`` for a recognized transient
214+
failure; ``None`` otherwise.
215+
"""
216+
if isinstance(exc, RateLimited):
217+
return QuotaExhausted, exc.retry_after
218+
if isinstance(exc, ServiceUnavailable):
219+
return ServiceInterrupted, exc.retry_after
220+
if isinstance(exc, (httpx.HTTPError, httpx.InvalidURL)):
221+
return ServiceInterrupted, None
222+
return None
223+
224+
193225
def _classify_chunk_error(
194226
exc: BaseException,
195227
) -> tuple[type[ChunkInterrupted], float | None] | None:
@@ -231,12 +263,9 @@ def _classify_chunk_error(
231263
"""
232264
cur: BaseException | None = exc
233265
while cur is not None:
234-
if isinstance(cur, RateLimited):
235-
return QuotaExhausted, cur.retry_after
236-
if isinstance(cur, ServiceUnavailable):
237-
return ServiceInterrupted, cur.retry_after
238-
if isinstance(cur, (httpx.HTTPError, httpx.InvalidURL)):
239-
return ServiceInterrupted, None
266+
result = _classify_transient(cur)
267+
if result is not None:
268+
return result
240269
cur = cur.__cause__
241270
return None
242271

@@ -262,6 +291,10 @@ def _retryable(exc: BaseException) -> tuple[bool, float | None]:
262291
``(retryable, retry_after)`` — the server ``Retry-After`` hint
263292
(seconds) when the transient carried one, else ``None``.
264293
"""
294+
# Only initial-page raw transients (TransientError, TransportError) are
295+
# retryable — not wrapped mid-pagination DataRetrievalError. We use
296+
# _classify_transient for the typed status errors and handle bare
297+
# TransportError (which isn't a TransientError) separately.
265298
if isinstance(exc, TransientError):
266299
return True, exc.retry_after
267300
if isinstance(exc, httpx.TransportError):

0 commit comments

Comments
 (0)