Skip to content

retry_if_exception_message: use re.search for the match= pattern so it finds the regex anywhere in the message#652

Open
HrachShah wants to merge 1 commit into
jd:mainfrom
HrachShah:fix/retry-if-exception-message-match-uses-search
Open

retry_if_exception_message: use re.search for the match= pattern so it finds the regex anywhere in the message#652
HrachShah wants to merge 1 commit into
jd:mainfrom
HrachShah:fix/retry-if-exception-message-match-uses-search

Conversation

@HrachShah

Copy link
Copy Markdown
Contributor

Bug

retry_if_exception_message(match=...) compiled the user's regex and called re.Pattern.match(), which only matches at the start of the string. The parameter is named match and the docstring just says "matches" — so users reasonably expect the pattern to be found anywhere in the exception message, the same way re.search does. With the current implementation, a user who wrote:

@retry(retry=retry_if_exception_message(match="rate limit"))
def fetch(): ...

got a retry on ValueError("rate limit hit") (works by accident) but no retry on ValueError("HTTP 429: rate limit hit") (pattern not at the start). That second case is the common real-world one and is exactly the kind of message HTTP libraries produce.

Fix

Use self.match.search(...) instead of self.match.match(...) in retry_if_exception_message._check. The behaviour of message= (full-string equality) is unchanged.

Test

Added test_retry_if_exception_message_match_finds_pattern_anywhere plus a module-level helper that raises CustomError("HTTP 429: rate limit exceeded") and matches on "limit exceeded" (which does not occur at position 0). The pre-existing test test_retry_if_exception_message_match still passes — it uses a pattern prefixed with derived_message[:3] + ".*" so both re.match and re.search accept it.

Full suite: 128 passed.

…t finds the regex anywhere in the exception message, not only at the start
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant