Skip to content

recognise read only error returned from Lua script#2770

Closed
zhengjilei wants to merge 2 commits into
redis:masterfrom
zhengjilei:fix/close-readonly-connections-for-luascript
Closed

recognise read only error returned from Lua script#2770
zhengjilei wants to merge 2 commits into
redis:masterfrom
zhengjilei:fix/close-readonly-connections-for-luascript

Conversation

@zhengjilei

Copy link
Copy Markdown
Contributor

No description provided.

@ofekshenawa

Copy link
Copy Markdown
Collaborator

Hey,
Do you think you might be able to write a unit test that validates this? WDYT?

@ndyakov ndyakov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check and address the comments that were left from @ofekshenawa and me.

Comment thread error.go

// For a Lua script that includes a write command, the error string
// contains "-READONLY" rather than beginning with "READONLY "
return strings.Contains(redisError, "-READONLY")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we be more strict here? I assume one can easily use a command with working on a "something-readonly" key and trigger an error that will return true for containing "-readonly"

@ndyakov

ndyakov commented Nov 19, 2025

Copy link
Copy Markdown
Member

Closing this for inactivity.

@ndyakov ndyakov closed this Nov 19, 2025
zhengjilei added a commit to zhengjilei/go-redis that referenced this pull request Apr 8, 2026
…icas

When a Lua script containing a write command is executed against a
read-only replica in older Redis versions (< 7.0), the error is wrapped:
"ERR Error running script ...: @user_script:N: -READONLY ..."

The existing code only checked for the "READONLY " prefix, missing
this embedded format. This caused shouldRetry() and isReadOnlyError()
to fail, preventing automatic retry and connection pool cleanup.

The fix uses strings.Contains(s, " -READONLY ") with surrounding
spaces to match the Lua error wrapping pattern while avoiding false
positives from key names containing "-READONLY" (addressing reviewer
feedback from PR redis#2770).

Closes redis#2770

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zhengjilei added a commit to zhengjilei/go-redis that referenced this pull request Apr 8, 2026
…icas

When a Lua script containing a write command is executed against a
read-only replica in older Redis versions (< 7.0), the error is wrapped:
"ERR Error running script ...: @user_script:N: -READONLY ..."

The existing code only checked for the "READONLY " prefix, missing
this embedded format. This caused shouldRetry() and isReadOnlyError()
to fail, preventing automatic retry and connection pool cleanup.

The fix checks for both "ERR Error running script" prefix and
"-READONLY" substring, ensuring only Lua script errors are matched
(addressing reviewer feedback from PR redis#2770).

Closes redis#2770

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zhengjilei added a commit to zhengjilei/go-redis that referenced this pull request Apr 8, 2026
…icas

When a Lua script containing a write command is executed against a
read-only replica in a Redis Cluster, the error may contain
"-READONLY You can't write against a read only replica" embedded
within a larger error message rather than starting with "READONLY ".

The existing code only checked for the "READONLY " prefix, missing
this format. This caused shouldRetry() and isReadOnlyError() to fail,
preventing automatic retry and connection pool cleanup.

The fix checks for the exact substring
"-READONLY You can't write against a read only replica" which is
specific enough to avoid false positives while catching the Lua
script error format.

Closes redis#2770

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ndyakov pushed a commit that referenced this pull request Apr 9, 2026
…icas (#3769)

When a Lua script containing a write command is executed against a
read-only replica in a Redis Cluster, the error may contain
"-READONLY You can't write against a read only replica" embedded
within a larger error message rather than starting with "READONLY ".

The existing code only checked for the "READONLY " prefix, missing
this format. This caused shouldRetry() and isReadOnlyError() to fail,
preventing automatic retry and connection pool cleanup.

The fix checks for the exact substring
"-READONLY You can't write against a read only replica" which is
specific enough to avoid false positives while catching the Lua
script error format.

Closes #2770

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants