Skip to content

Add NOSCRIPT error recovery for async operations#192

Merged
yedidyakfir merged 12 commits intodevelopfrom
feature/190-recover-from-no-script-error-for-async-actions
Feb 3, 2026
Merged

Add NOSCRIPT error recovery for async operations#192
yedidyakfir merged 12 commits intodevelopfrom
feature/190-recover-from-no-script-error-for-async-actions

Conversation

@yedidyakfir
Copy link
Collaborator

Summary

Extends NOSCRIPT error recovery to async operations outside pipeline context. When arun_sha() encounters a NOSCRIPT error (e.g., after Redis restart), it now automatically re-registers Lua scripts and retries.

Changes

  • arun_sha() recovery: Added try/catch for NoScriptError with automatic script re-registration and retry
  • PersistentNoScriptError: Raised when scripts fail to execute even after re-registration, indicating a server-side issue
  • flush_scripts fixture: Added test fixture to flush scripts before tests rather than mid-pipeline
  • Test cleanup: Simplified NOSCRIPT recovery tests to use the new fixture

Testing

  • Existing integration tests updated to use flush_scripts fixture
  • Tests verify recovery works for pipeline operations after script flush
  • Tests verify PersistentNoScriptError is raised when recovery fails

Closes #190

@yedidyakfir yedidyakfir linked an issue Feb 1, 2026 that may be closed by this pull request
2 tasks
@yedidyakfir yedidyakfir changed the base branch from main to develop February 1, 2026 16:43
Copilot AI review requested due to automatic review settings February 1, 2026 18:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request extends NOSCRIPT error recovery to async operations outside of pipeline contexts. The main change is adding automatic script re-registration and retry logic to the arun_sha() function, which is used by dict operations like apop() and apopitem(). The PR also introduces test fixtures to simplify NOSCRIPT recovery testing.

Changes:

  • Added NOSCRIPT error recovery to arun_sha() with automatic script re-registration and retry
  • Introduced PersistentNoScriptError exception for cases where recovery fails
  • Added flush_scripts and disable_noscript_recovery test fixtures for cleaner testing

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
rapyer/scripts/registry.py Implements NOSCRIPT error recovery in arun_sha() with try-catch logic and raises PersistentNoScriptError on persistent failures
rapyer/base.py Updates import to use module reference (scripts_registry) instead of direct function import
tests/integration/conftest.py Adds flush_scripts fixture to flush scripts before tests and disable_noscript_recovery fixture to mock recovery for failure testing
tests/integration/pipeline/test_pipeline_noscript_recovery.py Simplifies tests by using new fixtures instead of inline script flushing; removes unused imports
tests/integration/dct/test_redis_dict.py Adds test for apop raising PersistentNoScriptError when recovery is disabled
CHANGELOG.md Documents the new NOSCRIPT error recovery feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yedidyakfir yedidyakfir merged commit 46e6cd1 into develop Feb 3, 2026
66 of 79 checks passed
@yedidyakfir yedidyakfir deleted the feature/190-recover-from-no-script-error-for-async-actions branch February 3, 2026 09:37
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.

Recover from no script error for async actions

1 participant