Repository navigation
test(redis): prove the relay keeps publishing after the cache loses its scripts - #1003
Merged
Merged
Conversation
#1002 reported a relay that answered HTTP 200 and never acknowledged another event once its cache restarted: Lua scripts live in Redis memory, so every EVALSHA after the restart answered NOSCRIPT. The adapter retries with a fresh scriptLoad now, but nothing proved it — the unit test stubs evalSha, and the production failure was in the workers that call it. The scenario flushes the script cache between two publishes with a message rate limit configured, because both limiters return early on an empty list and a scenario without one would pass whether or not the fallback existed.
🦋 Changeset detectedLatest commit: 4086a82 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Collaborator
Coverage Report for CI Build 41224717814Coverage increased (+0.006%) to 76.754%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
chappie-daemon
force-pushed
the
test/1002-script-cache-recovery
branch
from
October 31, 2026 17:11
aded16f to
80b0a9e
Compare
cameri
marked this pull request as ready for review
October 31, 2026 17:18
 [Low impact] The PR appears safe to merge; the settings leak is fixed and no new actionable issues were found.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The @script-cache hook rewrites SettingsStatic._settings so the message limiter actually runs, but one worker serves every scenario in the process and controllers read that object through createSettings() on each request. The mutation therefore outlived its scenario, and every scenario after it ran with a 100/min message limit, an empty ipWhitelist and the ewma strategy. Save the previous object in the tagged Before and put it back in a tagged After, which also runs when the scenario fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an integration scenario for #1002.
What #1002 actually was
The relay answered
HTTP 200, accepted websocket upgrades, and never acknowledged another event after its Redis cache was restarted. The cause is that the relay issuesEVALSHA, and Lua scripts live in the cache's memory, so every call after a restart answersNOSCRIPT.The production failure was an old image, not missing code. The retry landed on 2026-10-21 in #920 (
src/adapters/redis-adapter.ts,isNoScriptError+ ascriptLoadretry). The relay that failed was built 2026-09-03 from revision3006eee3, five minor releases behindlatest(3.8.0, built 2026-10-24).So what was missing is proof, not a fix: the unit test stubs
evalSha, which cannot catch a retry that stops working through the workers — the path that actually broke — and nothing exercised a real script cache.What this adds
test/integration/features/redis-script-cache/— one scenario that publishes, runsSCRIPT FLUSHagainst the same Redis, and publishes twice more.SCRIPT FLUSHis the whole of a restart as far as the relay can tell, and it is deterministic: no container to restart, no reconnect to wait for.The
@script-cachehook configures a message rate limit on purpose. Both limiters return early whenrateLimitsis empty, so a scenario without one would pass whether or not the fallback existed — the guard that makes this test mean something.Verification
The middle row is the fail-before: the scenario goes red exactly when the fallback goes away, and green when it returns. The probe commit was dropped from the branch afterwards, so the diff here is the test and its changeset only.
cucumber-js --dry-runover the full feature set reports 123 scenarios with no undefined steps.Refs #1002.