ISSUE-627: (feature/retry-after-sync) Add handling of Retry-After header for 429 and 503 status codes for sync requests

[temsocial]

Details: #627

Added:
1 - a new class in error.py
2 - new logic for status_code in (429, 503) and "Retry-After" in r_headers

[tobixen]

DAVErrpr is the correct parent class for errors.

I think that some sleep-retry-logic should be implemented as well, with a max_sleep defined in the DAVClient object. 3s is probably a good default.

So if the retry-after is before the sleep limit, then sleep (async and sync logic for it), otherwise raise the exception.

(That makes sense in my head at least, I don't know if you have other ideas)

[temsocial]

@tobixen Thank you for your ideas! :)

I have added the parameters rate_limit_handle, rate_limit_default_sleep, and rate_limit_max_sleep.

By default, rate limit handling is disabled. This means that sleeping in case of a rate limit will only occur if the user explicitly sets these parameters in the DAVClient class.

I did this to ensure that if any library users decide to update, and they already have their own logic for handling rate limits on their side, there will be no duplication and their existing scripts will not break.

Initially, I propose to introduce this logic exclusively for the synchronous class, test its operation in my live environment, and confirm its stability. Subsequently, if the results are positive, we can proceed to adapt and migrate the logic to the asynchronous class.

[tobixen]

A small issue with the defaults:

• Default for rate_limit_handle is False
• Default for rate_limit_default_sleep is None
• Default for rate_limit_max_sleep is None

What does None mean?

For rate_limit_default_sleep, I think ithe most obvious would be "don't retry". But do we need a boolean rate_limit_handle then? I'd suggest to either have it to 3s as default (sensible value, I'd say) or have it as None, but skip having a separate boolean for turning on/off the sleep. (Or maybe not, None could mean "don't sleep unless the server requests so").

For rate_limit_max_sleep it may have different meanings, it could be that None means "wait for whatever long the server wants us to wait, without any limits", it could also be the equivalent of 0 - always raise an error, never sleep. So it needs some documentation, if nothing else.

[temsocial]

@tobixen

The current behavior is as follows:

• When rate_limit_handle = True but the sleep parameters are None, we respect the server's Retry-After header without limits.

• If rate_limit_max_sleep is provided, it caps any wait time (including server-requested delays). [For example, Retry-After=7000 seconds, but rate_limit_max_sleep=120, then we will wait 120 seconds]

• When a 429 response lacks a parsable Retry-After and rate_limit_default_sleep is set, we use that fallback value.

• If no fallback [rate_limit_default_sleep] is configured and the server doesn't specify a Retry-After, we raise an error rather than sleeping.

[tobixen]

So if I understand it right, None here will cause it to raise the RateLimitError on 429s without retry-after, this should be explicit in the inline documentation here.

[tobixen]

Here, None means "respect retry-after, no matter if it's some few seconds or some few weeks". Should be explicit in the comments.

[tobixen]

I'll go sleep now, will merge it tomorrow.

[tobixen]

(Disclaimer - text below was written by Claude, the AI)

Thank you for this contribution! The feature has been incorporated in commit 722e959 on master (with some fixes applied during code review).

The following issues were corrected before merging:

• Undefined variable: retry_after was used in two places inside _sync_request but was never defined — the intended variable was retry_after_header
• Unused variable: retry_after_value was assigned but never read — removed
• Wrong super().__init__() call: DAVError.__init__ takes url and reason keyword arguments, not a positional message string; the message was ending up as self.url instead of self.reason. RateLimitError now follows the same convention as AuthorizationError
• None <= 0 TypeError: when rate_limit_handle=True but neither a server Retry-After header nor a rate_limit_default_sleep was configured, retry_after_seconds would be None, causing a TypeError on the <= 0 comparison — fixed with an explicit None guard
• Bare except:: replaced with except (ValueError, TypeError): to avoid swallowing KeyboardInterrupt / SystemExit
• Type annotations: int = None changed to Optional[int] = None for the two new parameters
• Documentation: clarified what None means for rate_limit_default_sleep (raise instead of sleep) and rate_limit_max_sleep (no cap), as requested in review comments

11 unit tests were also added covering the main behaviours (integer and HTTP-date Retry-After parsing, 503 without Retry-After passing through, sleep+retry, default sleep fallback, max-sleep cap, etc.).

This pull request was reviewed and the fixes were vibe-coded with the assistance of Claude Code.

[tobixen]

Closing — changes incorporated in commit 722e959 on master.