Skip to content

Commit e8d7a54

Browse files
committed
runtime: wait the interval the service asked for, on every retry path
Microsoft's throttling guidance has one instruction before all the others: If the response includes a Retry-After header, wait for the specified interval before retrying. Don't retry the request immediately. Treat it as the minimum time to wait. The library honoured that in two of three places. `_send_with_retry` and `_send_with_retry_async` (the parallel and async paths) return the parsed `Retry-After` from their `on_failure` hook, so a throttled request there waits what the service asked. `execute_query_retry` -- the sequential entry point, and the one `docs/` and `examples/` recommend for a polite retry -- passed `failure_callback=None` by default and therefore returned `None` from its hook, which the retry loop reads as "no opinion" and replaces with the exponential backoff. So `Retry-After: 120` was answered with `sleep(5)` and an immediate re-send: the retry storm the header exists to prevent, emitted by the call the documentation points you at. Two changes, one rule each. The default policy is now the server's. `runtime.retry.retry_after_policy(attempt, ex)` is a two-argument adapter over the existing `retry_after_delay(ex)`, whose signature did not match what the retry loops promise to call; the loops use it when no `on_failure` was supplied, and `execute_query_retry` / `execute_query_async_retry` fall back to it when `failure_callback is None`. A callback you pass still wins -- the policy is a default, not a ceiling on what a caller may decide. `max_delay` now bounds whichever delay is chosen. It used to cap only `backoff_delay`, so a caller who had already said "never wait longer than 30 seconds" was still handed an hour by a `Retry-After: 3600`. One helper, `delay_before_attempt`, is the only place the choice is made, and it clamps the server's value too: a caller with a ceiling keeps it, a caller without one waits as long as the service asked. That is also the difference the new test pins -- `Retry-After: 600` with `max_delay=30` sleeps 30, and `Retry-After: 7` with no cap sleeps 7. Proved red first, as required: with the two library files stashed, `test_retry_after_is_waited_by_default` and `test_max_delay_bounds_the_wait_the_server_asks_for` fail with `sleep(4.076850252856038)` where 30 was expected and `sleep(5)` where 7 was expected; with the change they pass, and the 33 tests already in `tests/unit/test_runtime_retry.py` -- including the one that pins "no `Retry-After` means exponential backoff" -- are untouched. Measured: `tests/unit` + `tests/tooling` 1012 passed, 5 subtests; offline suite 1015 passed / 983 skipped; pyright 0 errors, 0 warnings, 0 informations; ruff clean; a live `ctx.web.get().execute_query_retry(max_retry=2)` against the team site still answers the web title, so the untrottled path is unchanged. Not changed, deliberately: the per-service request windows (Outlook's 10,000 per 10 minutes per mailbox, Teams' per-resource rates, Files' concurrent-write limits) are documented as *thresholds*, not as signals a response carries. The library reacts to the signals the service sends and does not simulate the token bucket; mirroring those tables into code would give callers a number to believe without giving the runtime anything to enforce. Where they do belong -- beside the model, as `Limit(...)` rows -- the catalogs for Outlook/Excel/Reports/Teams are missing and are a data change, not a code path.
1 parent ae6f087 commit e8d7a54

4 files changed

Lines changed: 106 additions & 35 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,23 @@ with `author_id=`/`editor_id=` now raises `TypeError` — pass
378378

379379
### Fixed
380380

381+
**Runtime**
382+
383+
- **A throttled request now waits the interval the service asked for.**
384+
`execute_query_retry` retried a `429`/`503` on its exponential backoff and ignored
385+
`Retry-After` unless the caller passed a `failure_callback` of their own, so the
386+
one entry point the docs recommend for polite retries was the one that re-sent a
387+
throttled request at the worst moment — Microsoft's first instruction is *wait the
388+
interval named in the header; do not retry immediately*. The transport paths
389+
(`execute_query_retry_parallel`, the async sibling) already honoured it, which made
390+
the same library answer two ways. The default `on_failure` policy is now the
391+
server's `Retry-After` (`runtime.retry.retry_after_policy`), falling back to the
392+
backoff only when the response names no interval, and `max_delay` bounds *any*
393+
delay rather than just the exponential one — so a caller who passes a ceiling keeps
394+
it even when the service asks for an hour, and a caller who passes none waits as
395+
long as the service asked. Pinned offline: `Retry-After: 7` sleeps exactly 7 s
396+
before the second attempt, and `max_delay=30` turns a `Retry-After: 600` into 30 s.
397+
381398
**SharePoint (client)**
382399

383400
- **A query bound to a subweb addresses the subweb.** `ClientQuery.url` pasted the

‎office365/runtime/client_runtime_context.py‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -170,25 +170,26 @@ def execute_query_retry(
170170
are retried. Permanent failures (e.g. HTTP 400/401/403/404) are re-raised
171171
immediately, and the last exception is re-raised once retries are exhausted.
172172
173-
Delays between attempts use exponential backoff with jitter; a delay
174-
returned by ``failure_callback`` (e.g. the server's ``Retry-After`` via
175-
``retry_after_delay``) overrides the backoff.
173+
Delays between attempts use exponential backoff with jitter, and the wait a
174+
throttled response asks for (``Retry-After``) overrides the backoff — a
175+
``failure_callback`` replaces that default policy.
176176
177177
Args:
178178
max_retry: Maximum number of retry attempts
179179
timeout_secs: Base delay for exponential backoff (seconds)
180-
max_delay: Optional cap on the exponential delay (seconds)
181-
jitter: Whether to randomize the delay (default True)
180+
max_delay: Optional cap on any delay, the server's included (seconds)
181+
jitter: Whether to randomize the backoff delay (default True)
182182
success_callback: Called on successful execution
183183
failure_callback: Called after each failed attempt; may return a
184-
retry delay in seconds to override the backoff
184+
retry delay in seconds to override the backoff. Defaults to the
185+
server's ``Retry-After`` (:func:`~office365.runtime.retry.retry_after_policy`).
185186
exceptions: Exception types that trigger retries
186187
is_retriable: Optional predicate deciding whether a caught exception
187188
is retried. Defaults to :func:`~office365.runtime.retry.is_transient_error`;
188189
pass :func:`~office365.runtime.retry.retry_on` to also retry
189190
otherwise-permanent errors such as a locked file (HTTP 423).
190191
"""
191-
from office365.runtime.retry import is_transient_error, retry, should_reset_connections
192+
from office365.runtime.retry import is_transient_error, retry, retry_after_policy, should_reset_connections
192193

193194
def _on_failure(_attempt: int, ex: Exception) -> Optional[int]:
194195
# Drop a poisoned keep-alive socket before retrying a 503/connection
@@ -199,7 +200,10 @@ def _on_failure(_attempt: int, ex: Exception) -> Optional[int]:
199200
# otherwise the context is left with a stale, un-executed query.
200201
if _attempt < max_retry and self.current_query is not None:
201202
self.add_query(self.current_query)
202-
return failure_callback(_attempt, ex) if callable(failure_callback) else None
203+
# Microsoft's first instruction for a throttled request: wait the
204+
# interval the server named. Without a ``Retry-After`` (or with a
205+
# ``failure_callback`` of your own) the backoff decides.
206+
return (failure_callback or retry_after_policy)(_attempt, ex)
203207

204208
def _on_success(_) -> None:
205209
if callable(success_callback) and self.current_query is not None:
@@ -550,23 +554,25 @@ async def execute_query_async_retry(
550554
"""Executes pending queries with retry logic, without blocking the loop.
551555
552556
Async counterpart of :meth:`execute_query_retry` — same retry policy
553-
(transient-only by default, exponential backoff with jitter), but the
554-
delay between attempts uses ``asyncio.sleep``.
557+
(transient-only by default, exponential backoff with jitter, the server's
558+
``Retry-After`` ahead of it), but the delay between attempts uses
559+
``asyncio.sleep``.
555560
556561
Args:
557562
max_retry: Maximum number of retry attempts
558563
timeout_secs: Base delay for exponential backoff (seconds)
559-
max_delay: Optional cap on the exponential delay (seconds)
560-
jitter: Whether to randomize the delay (default True)
564+
max_delay: Optional cap on any delay, the server's included (seconds)
565+
jitter: Whether to randomize the backoff delay (default True)
561566
success_callback: Called on successful execution
562567
failure_callback: Called after each failed attempt; may return a
563-
retry delay in seconds to override the backoff
568+
retry delay in seconds to override the backoff. Defaults to the
569+
server's ``Retry-After`` (:func:`~office365.runtime.retry.retry_after_policy`).
564570
exceptions: Exception types that trigger retries
565571
is_retriable: Optional predicate deciding whether a caught exception
566572
is retried. Defaults to
567573
:func:`~office365.runtime.retry.is_transient_error`.
568574
"""
569-
from office365.runtime.retry import is_transient_error, retry_async, should_reset_connections
575+
from office365.runtime.retry import is_transient_error, retry_after_policy, retry_async, should_reset_connections
570576

571577
def _on_failure(_attempt: int, ex: Exception) -> Optional[int]:
572578
# Drop a poisoned keep-alive socket before retrying a 503/connection
@@ -577,7 +583,7 @@ def _on_failure(_attempt: int, ex: Exception) -> Optional[int]:
577583
# otherwise the context is left with a stale, un-executed query.
578584
if _attempt < max_retry and self.current_query is not None:
579585
self.add_query(self.current_query)
580-
return failure_callback(_attempt, ex) if callable(failure_callback) else None
586+
return (failure_callback or retry_after_policy)(_attempt, ex)
581587

582588
def _on_success(_) -> None:
583589
if callable(success_callback) and self.current_query is not None:

‎office365/runtime/retry.py‎

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,16 @@ def response_retry_after(response: Any) -> Optional[int]:
128128
return max(0, ceil(delay))
129129

130130

131+
def retry_after_policy(_attempt: int, ex: Exception) -> Optional[int]:
132+
"""The default ``on_failure`` policy: wait for what the server asked of you.
133+
134+
Signature-compatible with ``on_failure``/``failure_callback`` so a caller can
135+
pass it as-is; it exists because :func:`retry_after_delay` reads one argument
136+
and the retry loops promise two.
137+
"""
138+
return retry_after_delay(ex)
139+
140+
131141
def backoff_delay(attempt: int, base: int, max_delay: Optional[int] = None, jitter: bool = True) -> float:
132142
"""Compute the delay before retry ``attempt``.
133143
@@ -152,6 +162,26 @@ def backoff_delay(attempt: int, base: int, max_delay: Optional[int] = None, jitt
152162
return delay
153163

154164

165+
def delay_before_attempt(
166+
attempt: int,
167+
retry_after: Optional[int],
168+
timeout_secs: int,
169+
max_delay: Optional[int],
170+
jitter: bool,
171+
) -> float:
172+
"""Seconds to wait before retry ``attempt``: the server's request, else the backoff.
173+
174+
``max_delay`` bounds whichever is chosen, so a caller that caps its retries
175+
also caps the wait a server is allowed to ask for — a ``Retry-After`` of an
176+
hour is honored by a caller who asked for it and bounded by one who asked
177+
for a ceiling.
178+
"""
179+
delay = retry_after if retry_after is not None else backoff_delay(attempt, timeout_secs, max_delay, jitter)
180+
if max_delay is not None:
181+
delay = min(delay, max_delay)
182+
return delay
183+
184+
155185
def _run_with_retry(
156186
func: Callable[[], Any],
157187
*,
@@ -175,10 +205,8 @@ def _run_with_retry(
175205
if not callable(is_retriable) or not is_retriable(e):
176206
raise
177207
last_ex = e
178-
retry_after: Optional[int] = None
179-
if callable(on_failure):
180-
retry_after = on_failure(attempt, e)
181-
delay = retry_after if retry_after is not None else backoff_delay(attempt, timeout_secs, max_delay, jitter)
208+
retry_after = on_failure(attempt, e) if callable(on_failure) else retry_after_policy(attempt, e)
209+
delay = delay_before_attempt(attempt, retry_after, timeout_secs, max_delay, jitter)
182210
sleep(delay)
183211
assert last_ex is not None
184212
raise last_ex
@@ -198,10 +226,11 @@ def retry(
198226
"""Run ``func`` with retries — or, called without ``func``, return a decorator.
199227
200228
Permanent failures re-raise immediately; the last exception is re-raised
201-
once retries are exhausted. The delay between attempts is exponential with
202-
jitter (:func:`backoff_delay`); when ``on_failure`` returns a delay (e.g.
203-
the server's ``Retry-After`` via :func:`retry_after_delay`) that value
204-
overrides the backoff.
229+
once retries are exhausted. The delay between attempts is the one the server
230+
named (``Retry-After``, via :func:`retry_after_policy`) and falls back to
231+
exponential backoff with jitter (:func:`backoff_delay`) when it named none;
232+
``on_failure`` replaces that policy, and ``max_delay`` bounds whichever delay
233+
results.
205234
206235
Two forms::
207236
@@ -215,12 +244,13 @@ def do_request(): ...
215244
func: Callable to execute. Omit to get a decorator.
216245
max_retry: Maximum number of retry attempts
217246
timeout_secs: Base delay for exponential backoff (seconds)
218-
max_delay: Optional cap for the exponential delay (seconds)
219-
jitter: Whether to randomize the delay (default True)
247+
max_delay: Optional cap for any delay, the server's included (seconds)
248+
jitter: Whether to randomize the backoff delay (default True)
220249
exceptions: Exception types that are candidates for retry
221250
is_retriable: Classifier deciding whether a caught exception is retried
222251
on_failure: Called after each failed attempt with ``(attempt, ex)``;
223-
may return a retry delay (seconds) to override the backoff
252+
may return a retry delay (seconds) to override the policy. Defaults
253+
to :func:`retry_after_policy` — the server's ``Retry-After``
224254
on_success: Called with ``func()`` result on success
225255
"""
226256
options = {
@@ -261,20 +291,21 @@ async def retry_async(
261291
"""Run an awaitable ``func`` with retries, without blocking the event loop.
262292
263293
Async counterpart of :func:`retry`: identical retry/backoff semantics
264-
(transient-only by default, exponential backoff with jitter, ``Retry-After``
265-
override), but the wait between attempts uses ``asyncio.sleep`` so
294+
(transient-only by default, the server's ``Retry-After`` ahead of exponential
295+
backoff with jitter), but the wait between attempts uses ``asyncio.sleep`` so
266296
concurrently-running tasks keep making progress.
267297
268298
Args:
269299
func: Awaitable callable to execute.
270300
max_retry: Maximum number of retry attempts
271301
timeout_secs: Base delay for exponential backoff (seconds)
272-
max_delay: Optional cap for the exponential delay (seconds)
273-
jitter: Whether to randomize the delay (default True)
302+
max_delay: Optional cap for any delay, the server's included (seconds)
303+
jitter: Whether to randomize the backoff delay (default True)
274304
exceptions: Exception types that are candidates for retry
275305
is_retriable: Classifier deciding whether a caught exception is retried
276306
on_failure: Called after each failed attempt with ``(attempt, ex)``;
277-
may return a retry delay (seconds) to override the backoff
307+
may return a retry delay (seconds) to override the policy. Defaults
308+
to :func:`retry_after_policy` — the server's ``Retry-After``
278309
on_success: Called with ``await func()`` result on success
279310
"""
280311
last_ex: Exception | None = None
@@ -288,10 +319,8 @@ async def retry_async(
288319
if not callable(is_retriable) or not is_retriable(e):
289320
raise
290321
last_ex = e
291-
retry_after: Optional[int] = None
292-
if callable(on_failure):
293-
retry_after = on_failure(attempt, e)
294-
delay = retry_after if retry_after is not None else backoff_delay(attempt, timeout_secs, max_delay, jitter)
322+
retry_after = on_failure(attempt, e) if callable(on_failure) else retry_after_policy(attempt, e)
323+
delay = delay_before_attempt(attempt, retry_after, timeout_secs, max_delay, jitter)
295324
await asyncio.sleep(delay)
296325
assert last_ex is not None
297326
raise last_ex

‎tests/unit/test_runtime_retry.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,16 @@ def test_incremental_retry_falls_back_to_exponential_backoff(self):
106106
self.assertEqual(execute_query.call_count, 2)
107107
sleep_mock.assert_called_once_with(5)
108108

109+
def test_retry_after_is_waited_by_default(self):
110+
"""Microsoft's first instruction for a throttled request: wait what it asked for."""
111+
ctx, execute_query = _make_context([_make_exception(429, {"Retry-After": "7"}), None])
112+
113+
with mock.patch("office365.runtime.retry.sleep") as sleep_mock:
114+
ctx.execute_query_retry(max_retry=5, timeout_secs=1, jitter=False)
115+
116+
self.assertEqual(execute_query.call_count, 2)
117+
sleep_mock.assert_called_once_with(7)
118+
109119

110120
class TestRetryFunction(unittest.TestCase):
111121
def test_retries_exhausted_raises_last_error(self):
@@ -128,6 +138,15 @@ def test_invalid_url_is_not_retried(self):
128138
self.assertEqual(func.call_count, 1)
129139
sleep_mock.assert_not_called()
130140

141+
def test_max_delay_bounds_the_wait_the_server_asks_for(self):
142+
"""A caller who names a ceiling keeps it, whatever ``Retry-After`` says."""
143+
func = mock.Mock(side_effect=[_make_exception(429, {"Retry-After": "600"}), None])
144+
145+
with mock.patch("office365.runtime.retry.sleep") as sleep_mock:
146+
retry(func, max_retry=5, max_delay=30)
147+
148+
sleep_mock.assert_called_once_with(30)
149+
131150

132151
class TestTransportErrorClassification(unittest.TestCase):
133152
def test_connection_and_timeout_are_transient(self):

0 commit comments

Comments
 (0)