Version 5.0.0: remove callback support and upgrade to node-redis v6 - #77
Open
freshlogic wants to merge 16 commits into
Open
freshlogic wants to merge 16 commits into
freshlogic wants to merge 16 commits into
Conversation
Every function is now a plain async function returning a promise; the executor pattern, callback bridges, and deprecation warnings are gone. Passing a callback rejects with a TypeError pointing at the returned promise, rather than silently ignoring a callback that would never be invoked. Cache-miss functions and retrieveOrCreate's size option must be async (or plain-return) functions. The test suite is converted to async/await throughout: callback-only and duplicate tests are removed, error-injection stub tests use assert.rejects, and a guard test covers callback rejection for all seventeen methods. Line and branch coverage remain at 100%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Commands use node-redis's native promises directly, so the per-call promisify wrappers are gone; bulkSet pipelines through multi() instead of the removed batch(). The constructor connects the client automatically (commands issued while connecting are queued by the client), still accepts the legacy (port, [host, [options]]) signature by translating it to node-redis options including auth_pass to password, and connects injected clients that aren't already open. Test stubs return promises to match the v6 client API, and the raw client tests use the camelCase v6 commands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Individual PSETEX commands issued in the same tick are automatically pipelined into a single round trip by node-redis, so Promise.all over plain pSetEx calls replaces the explicit multi()/execAsPipeline() machinery with the same wire behavior and matching error semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stops the background refresh intervals started by fetchAndRefresh and gracefully closes the Redis client connection via node-redis v6's close(), giving services and tests a clean shutdown path for the first time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Unscoped push + pull_request fired two Test runs per PR commit, doubling live-API load and requiring both twin checks to pass before merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The constructor now translates auth_pass, connect_timeout, db, enable_offline_queue, family, host, path, port, socket_initial_delay, socket_keepalive, and tls whether passed positionally or as an options object — the v3 object form with root host/port previously fell through to v6 untranslated and silently connected to localhost. Legacy options with no v6 equivalent (retry_strategy, return_buffers, and friends) throw a TypeError instead of being silently ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Brings in #73 (invoke callbacks exactly once, outside of the promise chain). Nothing from it applies here: v5 removed callback support, so all 17 conflicted hunks resolve to v5's side, the invokeCallback helper would be dead code, and the new child-process tests exercise a bridge that no longer exists. v5 already covers this surface with "should reject callback-style usage with a TypeError". The CHANGELOG entry is dropped for the same reason and stays on main, where it will be versioned when the 4.0.x patch is cut. The merged tree is byte-identical to v5.0.0's previous tip; this commit only records the ancestry so the next merge from main doesn't re-conflict on the same 17 hunks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings in 4.0.1. Conflicts resolved by keeping v5.0.0's version and retaining both CHANGELOG entries. The 4.0.1 double-callback fix needs no counterpart here — v5 removed the callback bridges it patched, so invokeCallback would be dead code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The legacy (port, [host, [options]]) branch gated on typeof === 'number', but every caller passes process.env.redisPort, which is a string. A string fell through to redis.createClient(port), which node-redis v6 silently ignores — dropping the host, password, and every translated option, and yielding a default localhost:6379 client with no error. node-redis v3 accepted the string, so this only broke on the v6 upgrade. Also bumps redis to ~6.2.1, the latest v6. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Coverage Report for CI Build 33593139166Coverage remained the same at 100.0%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Removes the (port, host, options) signature and the translation of v3 option names. Both now throw a TypeError naming what to change. Translating was the wrong call. v6 ignores unknown top-level options, so anything the translator missed produced an unauthenticated client pointed at localhost:6379 — a cache that never hits, never errors, and looks healthy. Two separate bugs in that layer surfaced in as many days: a numeric string port fell through the dispatch, and then an unset port did the same thing while also discarding the host and password that followed it. Every fleet call site passes process.env.redisPort, which is not set on any App Service, so both were live. A translator that silently degrades is worse than no translator. Callers now migrate once, at construction, with the offending option named: TypeError: petty-cache v5 takes a node-redis options object. auth_pass, host, port are node-redis v3 options and would be ignored; see docs/v4-to-v5.md. new PettyCache(redisClient) and new PettyCache() are unchanged. Drops isPort and translateLegacyOptions — 122 lines out of index.js, 100% line and branch coverage held across 172 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The guide said the translation of v3 option names was "gone", but that translation only ever existed on this unreleased branch — a reader coming from v4 has never seen it and cannot tell what was removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drops the legacy-option list and the construction-time guards. v5 takes a node-redis options object; callers migrate once, and the migration guide carries the v3 to v6 mapping. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Take a node-redis options object and nothing else
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
2 participants
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.
Removes callback support entirely and upgrades the underlying Redis client from node-redis v3 to v6. See the v4 to v5 migration guide.
Changed
TypeErrorrather than being silently ignored, so a missed call site fails loudly instead of hanging.fetch,fetchAndRefresh,bulkFetch, andsemaphore.retrieveOrCreate'ssizeoption.bulkSetleans on v6 auto-pipelining instead ofmulti, and injected clients must be v6 clients. The legacy(port, [host, [options]])signature still works, and common v3 options (auth_pass,connect_timeout,db,enable_offline_queue,host,port,tls, and friends) are translated. Legacy options with no v6 equivalent (retry_strategy,prefix,detect_buffers, …) throw aTypeErrorinstead of being silently dropped.Added
pettyCache.close()— stops the background refresh intervals started byfetchAndRefreshand closes the Redis client.Before merging
Two things outside this repo have to land first, or the fleet breaks on the bump:
require('redis')without declaring it and get it transitively from petty-cache, using the v3 API inroutes/status.js. Bumping petty-cache swaps them to v6 and breaks/status. PRs for those are open separately.Notes
typeof port === 'number', but the fleet passesprocess.env.redisPort, a string. That fell through toredis.createClient('6380'), which v6 ignores — dropping host, password, and every translated option, and connecting to a default localhost:6379 with no error.SocketTimeoutError, though, which is worth a follow-upsocket.reconnectStrategyhere.🤖 Generated with Claude Code