fix: extend socket_timeout for blocking commands to prevent premature timeouts - #4151
fix: extend socket_timeout for blocking commands to prevent premature timeouts#4151C1-BA-B1-F3 wants to merge 1 commit into
Conversation
… timeouts Blocking commands (BRPOP, BLPOP, BRPOPLPUSH, BLMOVE, BZPOPMIN, BZPOPMAX, BZMPOP, XREAD/XREADGROUP with BLOCK) specify their own timeout at the Redis protocol level. When the command timeout exceeds the client's socket_timeout, the socket layer raises TimeoutError before the blocking command can complete. This fix detects blocking commands in _send_command_parse_response and temporarily extends the connection's socket_timeout to cover the command's blocking duration. The original timeout is restored after the command completes, even if an exception occurs. Fixes redis#2807
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 4 potential issues.
Reviewed by Cursor Bugbot for commit 4bd5e17. Configure here.
|
|
||
| try: | ||
| conn.send_command(*args, **options) | ||
| return self.parse_response(conn, command_name, **options) |
There was a problem hiding this comment.
Sync timeout not applied
High Severity
Blocking-command handling only assigns conn.socket_timeout before parse_response. The sync connection still reads using the OS socket timeout and parser state set at connect time, so BRPOP-style calls can keep raising TimeoutError at the original socket_timeout despite this change.
Reviewed by Cursor Bugbot for commit 4bd5e17. Configure here.
| arg_str = ( | ||
| arg.upper() if isinstance(arg, bytes) else str(arg).upper() | ||
| ) | ||
| if arg_str == "BLOCK": |
There was a problem hiding this comment.
XREAD BLOCK bytes mismatch
High Severity
_get_blocking_timeout treats a BLOCK token as the string "BLOCK", but xread and xreadgroup emit b"BLOCK". That comparison never matches, so stream blocking reads skip socket timeout extension even though the PR lists XREAD/XREADGROUP as fixed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4bd5e17. Configure here.
| return self.parse_response(conn, command_name, **options) | ||
| finally: | ||
| if orig_socket_timeout is not None: | ||
| conn.socket_timeout = orig_socket_timeout |
There was a problem hiding this comment.
Timeout not restored when None
Medium Severity
When the connection’s original socket_timeout is None and a blocking command raises the timeout, the finally block skips restoration because it only runs when orig_socket_timeout is not None. The pooled connection can keep an inflated timeout after the command finishes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4bd5e17. Configure here.
| return self.parse_response(conn, command_name, **options) | ||
| blocking_timeout = self._get_blocking_timeout(command_name, args) | ||
| orig_socket_timeout = None | ||
| if blocking_timeout and blocking_timeout > 0: |
There was a problem hiding this comment.
Zero timeout not extended
Medium Severity
Socket extension runs only when blocking_timeout > 0, but Redis list blocking commands use timeout=0 to block indefinitely. Those calls still fail early against a finite socket_timeout, which the PR aims to prevent for blocking commands generally.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4bd5e17. Configure here.
|
Please add all related changes to blocking timeout in the other PR --> #4143 |


Description
Fixes #2807
Blocking commands (BRPOP, BLPOP, BRPOPLPUSH, BLMOVE, BZPOPMIN, BZPOPMAX, BZMPOP, XREAD/XREADGROUP with BLOCK) specify their own timeout at the Redis protocol level. When the command timeout exceeds the client's
socket_timeout, the socket layer raisesTimeoutErrorbefore the blocking command can complete.Problem
The
socket_timeoutis a low-level socket setting that should not prevent application-level blocking timeouts from being honored.Solution
This fix detects blocking commands in
_send_command_parse_responseand temporarily extends the connection'ssocket_timeoutto cover the command's blocking duration (plus a 1-second buffer). The original timeout is restored after the command completes, even if an exception occurs.Changes
redis/client.py: Added_get_blocking_timeout()method and socket timeout adjustment in_send_command_parse_response()redis/asyncio/client.py: Same fix for the async clienttests/test_client.py: AddedTestBlockingCommandTimeouttest class with 15 tests covering all blocking commandsCommands Fixed
Testing
All 25 tests in
tests/test_client.pypass, including 15 new tests specifically for the blocking timeout fix.Note
Medium Risk
Changes the shared command-send path by mutating per-connection
socket_timeout, which could affect pooled connections if restore failed; behavior is narrow, guarded, and covered by new tests.Overview
Fixes premature
TimeoutErrorwhen a blocking command’s Redis-level wait is longer than the client’ssocket_timeout(e.g.brpop(..., timeout=8)withsocket_timeout=4).Sync and async
Redisclients now parse the block duration for commands likeBRPOP/BLPOP,BRPOPLPUSH,BLMOVE,BZMPOP, andXREAD/XREADGROUPwithBLOCK. In_send_command_parse_response, the connection’ssocket_timeoutis temporarily raised to that duration plus one second (only if needed), then restored in afinallyblock—including on errors. Non-blocking commands and timeouts of0are unchanged.Adds
TestBlockingCommandTimeoutintests/test_client.pyfor parsing and socket-timeout behavior.Reviewed by Cursor Bugbot for commit 4bd5e17. Bugbot is set up for automated code reviews on this repo. Configure here.