Skip to content

Commit 17b952f

Browse files
authored
Merge pull request #897 from jackson-asmith/fix/retry-unprocessed-mutations-only
feat(http): add option to retry only unprocessed mutations
2 parents 37ed438 + e31858f commit 17b952f

3 files changed

Lines changed: 221 additions & 1 deletion

File tree

‎packages/http/httpx/kiota_http/middleware/options/retry_handler_option.py‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,17 @@ class RetryHandlerOption(RequestOption):
1111
max_retries (int): number of retries after the first attempt, 0 to
1212
``MAX_MAX_RETRIES``. Exposed as ``max_retry``.
1313
should_retry (bool): whether the handler retries at all.
14+
retry_unprocessed_mutations_only (bool): whether POST and PATCH requests are retried
15+
only after a response showing the server didn't process them. Defaults to False.
16+
POST and PATCH aren't idempotent, so resending one the server already processed
17+
can repeat its side effect, such as sending an email twice. A 504 can arrive after
18+
the server processed the request. When this is True, POST and PATCH requests are
19+
retried only after a 429 or 503 response, and a 504 is returned to the caller.
20+
This relies on the common service convention, followed by Microsoft Graph, that a
21+
429 or 503 response means the server didn't process the request. HTTP doesn't
22+
guarantee this, so a service that returns 429 or 503 after acting on a request can
23+
still see it repeated. PUT and DELETE also change state, but they're idempotent, so
24+
they aren't affected, and neither are other methods.
1425
"""
1526

1627
# Default maxRetries value
@@ -35,7 +46,8 @@ def __init__(
3546
self,
3647
delay: float = DEFAULT_DELAY,
3748
max_retries: int = DEFAULT_MAX_RETRIES,
38-
should_retry: bool = DEFAULT_SHOULD_RETRY
49+
should_retry: bool = DEFAULT_SHOULD_RETRY,
50+
retry_unprocessed_mutations_only: bool = False
3951
) -> None:
4052
if delay > self.MAX_DELAY and max_retries > self.DEFAULT_MAX_RETRIES:
4153
raise ValueError(
@@ -58,6 +70,7 @@ def __init__(
5870
self._max_retry: int = min(max_retries, self.MAX_MAX_RETRIES)
5971
self._max_delay: float = min(delay, self.MAX_DELAY)
6072
self._should_retry: bool = should_retry
73+
self._retry_unprocessed_mutations_only: bool = retry_unprocessed_mutations_only
6174

6275
@property
6376
def max_delay(self) -> float:
@@ -93,6 +106,14 @@ def should_retry(self) -> bool:
93106
def should_retry(self, value: bool) -> None:
94107
self._should_retry = value
95108

109+
@property
110+
def retry_unprocessed_mutations_only(self) -> bool:
111+
return self._retry_unprocessed_mutations_only
112+
113+
@retry_unprocessed_mutations_only.setter
114+
def retry_unprocessed_mutations_only(self, value: bool) -> None:
115+
self._retry_unprocessed_mutations_only = value
116+
96117
@staticmethod
97118
def get_key() -> str:
98119
return RetryHandlerOption.RETRY_HANDLER_OPTION_KEY

‎packages/http/httpx/kiota_http/middleware/retry_handler.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ class RetryHandler(BaseMiddleware):
4343
['HEAD', 'GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS', 'QUERY']
4444
)
4545

46+
# Methods that aren't idempotent, so a retry can repeat their side effect
47+
NON_IDEMPOTENT_METHODS: frozenset[str] = frozenset(['POST', 'PATCH'])
48+
49+
# By common service convention, these status codes mean the server didn't process
50+
# the request, so resending it is treated as safe for any method
51+
# 429 - Too many requests
52+
# 503 - Service unavailable
53+
UNPROCESSED_STATUS_CODES: frozenset[int] = frozenset({429, 503})
54+
4655
def __init__(self, options: Optional[RetryHandlerOption] = None) -> None:
4756
super().__init__()
4857
self.allowed_methods: frozenset[str] = self.DEFAULT_ALLOWED_METHODS
@@ -129,8 +138,25 @@ def should_retry(
129138
return False
130139
if not self._is_request_payload_buffered(request):
131140
return False
141+
if not self._is_retryable_for_method(request, options, response):
142+
return False
132143
return options.max_retry > 0 and response.status_code in self.retry_on_status_codes
133144

145+
def _is_retryable_for_method(
146+
self, request: httpx.Request, options: RetryHandlerOption, response: httpx.Response
147+
) -> bool:
148+
"""
149+
Checks whether the response allows resending the request, given its method.
150+
With retry_unprocessed_mutations_only set, POST and PATCH requests are retried
151+
only after a response showing the server didn't process them, because any other
152+
response may arrive after the server already acted on the request.
153+
"""
154+
if not options.retry_unprocessed_mutations_only:
155+
return True
156+
if request.method.upper() not in self.NON_IDEMPOTENT_METHODS:
157+
return True
158+
return response.status_code in self.UNPROCESSED_STATUS_CODES
159+
134160
def _is_method_retryable(self, request: httpx.Request) -> bool:
135161
"""
136162
Checks if a given request should be retried upon, depending on

‎packages/http/httpx/tests/middleware_tests/test_retry_handler.py‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
PARENT_SPAN_KEY,
1010
REQUEST_OPTIONS_KEY,
1111
MiddlewarePipeline,
12+
RedirectHandler,
1213
RetryHandler,
1314
UserAgentHandler,
1415
)
@@ -444,3 +445,175 @@ def request_handler(request: httpx.Request):
444445
assert len(ua_spans) == 2
445446
for ua_span in ua_spans:
446447
assert ua_span.parent.span_id == expected_parent_id
448+
449+
450+
def _status_sequence_transport(status_codes, methods):
451+
"""Answers each request with the next status code and records its method."""
452+
remaining = list(status_codes)
453+
454+
def request_handler(request: httpx.Request):
455+
methods.append(request.method)
456+
return httpx.Response(remaining.pop(0) if remaining else 200)
457+
458+
return httpx.MockTransport(request_handler)
459+
460+
461+
@pytest.fixture
462+
def no_retry_delay(monkeypatch):
463+
"""Skips the wait between retries so the tests run quickly."""
464+
monkeypatch.setattr(RetryHandler, "get_delay_time", lambda self, *args, **kwargs: 0)
465+
466+
467+
def test_retry_unprocessed_mutations_only_defaults_to_false():
468+
assert not RetryHandlerOption().retry_unprocessed_mutations_only
469+
assert RetryHandlerOption(
470+
retry_unprocessed_mutations_only=True
471+
).retry_unprocessed_mutations_only
472+
473+
474+
@pytest.mark.asyncio
475+
@pytest.mark.parametrize("method", ["POST", "PATCH"])
476+
async def test_non_idempotent_request_retried_on_gateway_timeout_by_default(method, no_retry_delay):
477+
methods = []
478+
handler = RetryHandler()
479+
request = httpx.Request(method, BASE_URL, json={"message": "hello"})
480+
481+
resp = await handler.send(request, _status_sequence_transport([GATEWAY_TIMEOUT, 200], methods))
482+
483+
assert resp.status_code == 200
484+
assert len(methods) == 2
485+
486+
487+
@pytest.mark.asyncio
488+
@pytest.mark.parametrize("method", ["POST", "PATCH", "post"])
489+
async def test_non_idempotent_request_not_retried_on_gateway_timeout_when_enabled(
490+
method, no_retry_delay
491+
):
492+
methods = []
493+
handler = RetryHandler(RetryHandlerOption(retry_unprocessed_mutations_only=True))
494+
request = httpx.Request(method, BASE_URL, json={"message": "hello"})
495+
496+
resp = await handler.send(request, _status_sequence_transport([GATEWAY_TIMEOUT, 200], methods))
497+
498+
assert resp.status_code == GATEWAY_TIMEOUT
499+
assert len(methods) == 1
500+
501+
502+
@pytest.mark.asyncio
503+
@pytest.mark.parametrize("method", ["POST", "PATCH"])
504+
@pytest.mark.parametrize("status_code", [TOO_MANY_REQUESTS, SERVICE_UNAVAILABLE])
505+
async def test_non_idempotent_request_retried_when_not_processed_and_enabled(
506+
method, status_code, no_retry_delay
507+
):
508+
methods = []
509+
handler = RetryHandler(RetryHandlerOption(retry_unprocessed_mutations_only=True))
510+
request = httpx.Request(method, BASE_URL, json={"message": "hello"})
511+
512+
resp = await handler.send(request, _status_sequence_transport([status_code, 200], methods))
513+
514+
assert resp.status_code == 200
515+
assert len(methods) == 2
516+
517+
518+
@pytest.mark.asyncio
519+
@pytest.mark.parametrize("method", ["GET", "PUT", "DELETE", "QUERY"])
520+
async def test_idempotent_request_retried_on_gateway_timeout_when_enabled(method, no_retry_delay):
521+
methods = []
522+
handler = RetryHandler(RetryHandlerOption(retry_unprocessed_mutations_only=True))
523+
request = httpx.Request(method, BASE_URL)
524+
525+
resp = await handler.send(request, _status_sequence_transport([GATEWAY_TIMEOUT, 200], methods))
526+
527+
assert resp.status_code == 200
528+
assert len(methods) == 2
529+
530+
531+
@pytest.mark.asyncio
532+
async def test_non_idempotent_request_stops_when_a_later_response_leaves_outcome_unknown(
533+
no_retry_delay
534+
):
535+
methods = []
536+
handler = RetryHandler(RetryHandlerOption(retry_unprocessed_mutations_only=True))
537+
request = httpx.Request("POST", BASE_URL, json={"message": "hello"})
538+
539+
resp = await handler.send(
540+
request, _status_sequence_transport([SERVICE_UNAVAILABLE, GATEWAY_TIMEOUT, 200], methods)
541+
)
542+
543+
assert resp.status_code == GATEWAY_TIMEOUT
544+
assert len(methods) == 2
545+
546+
547+
@pytest.mark.asyncio
548+
async def test_retry_unprocessed_mutations_only_honoured_from_request_option(no_retry_delay):
549+
methods = []
550+
handler = RetryHandler()
551+
request_option = RetryHandlerOption(retry_unprocessed_mutations_only=True)
552+
request = httpx.Request(
553+
"POST",
554+
BASE_URL,
555+
json={"message": "hello"},
556+
extensions={REQUEST_OPTIONS_KEY: {
557+
RetryHandlerOption.get_key(): request_option
558+
}},
559+
)
560+
561+
resp = await handler.send(request, _status_sequence_transport([GATEWAY_TIMEOUT, 200], methods))
562+
563+
assert resp.status_code == GATEWAY_TIMEOUT
564+
assert len(methods) == 1
565+
566+
567+
def _redirect_then_status_transport(redirect_status, final_statuses, calls):
568+
"""Redirects the first request to /redirected, then answers with each final status."""
569+
remaining = list(final_statuses)
570+
571+
def request_handler(request: httpx.Request):
572+
calls.append(request.method)
573+
if request.url.path != "/redirected":
574+
return httpx.Response(redirect_status, headers={"Location": f"{BASE_URL}/redirected"})
575+
return httpx.Response(remaining.pop(0) if remaining else 200)
576+
577+
return httpx.MockTransport(request_handler)
578+
579+
580+
@pytest.mark.asyncio
581+
async def test_post_redirected_to_get_is_retried_when_enabled(no_retry_delay):
582+
calls = []
583+
pipeline = MiddlewarePipeline(
584+
_redirect_then_status_transport(303, [GATEWAY_TIMEOUT, 200], calls)
585+
)
586+
pipeline.add_middleware(RedirectHandler())
587+
pipeline.add_middleware(RetryHandler(RetryHandlerOption(retry_unprocessed_mutations_only=True)))
588+
request = httpx.Request("POST", f"{BASE_URL}/send", json={"message": "hello"})
589+
590+
resp = await pipeline.send(request)
591+
592+
assert resp.status_code == 200
593+
assert calls == ["POST", "GET", "GET"]
594+
595+
596+
@pytest.mark.asyncio
597+
async def test_post_redirected_as_post_is_not_retried_with_request_option(no_retry_delay):
598+
calls = []
599+
pipeline = MiddlewarePipeline(
600+
_redirect_then_status_transport(307, [GATEWAY_TIMEOUT, 200], calls)
601+
)
602+
pipeline.add_middleware(RedirectHandler())
603+
pipeline.add_middleware(RetryHandler())
604+
request = httpx.Request(
605+
"POST",
606+
f"{BASE_URL}/send",
607+
json={"message": "hello"},
608+
extensions={
609+
REQUEST_OPTIONS_KEY: {
610+
RetryHandlerOption.get_key():
611+
RetryHandlerOption(retry_unprocessed_mutations_only=True)
612+
}
613+
},
614+
)
615+
616+
resp = await pipeline.send(request)
617+
618+
assert resp.status_code == GATEWAY_TIMEOUT
619+
assert calls == ["POST", "POST"]

0 commit comments

Comments
 (0)