feat(sentinel): emit lifecycle events#3276
Conversation
|
Hi @raashish1601, thanks for the PR, i will look into it, hopefully soon! |
nkaradzhov
left a comment
There was a problem hiding this comment.
Needs rework before merge.
1. Wire emits to state transitions, not method bodies
Current PR emits from connect()/close()/destroy() directly. Standalone client emits from socket transitions (client/index.ts:946-973). Follow same pattern via #isOpen/#isReady setters:
#setOpen(value: boolean) {
if (this.#isOpen === value) return;
this.#isOpen = value;
this.emit(value ? 'connect' : 'end');
}
#setReady(value: boolean) {
if (this.#isReady === value) return;
this.#isReady = value;
if (value) {
this.emit('ready');
} else if (this.#isOpen && !this.#destroy) {
this.emit('reconnecting');
}
}Replace raw #isOpen =/#isReady = assignments in connect(), close(), destroy(), #reset() with these. Single source of truth. end fires regardless of caller. reconnecting (currently missing — issue #3012 calls it out) falls out via #reset(). No async destroy() signature change needed.
2. Docs
No update to README.md events table (L360-377, RedisClient-only) or docs/sentinel.md (no events section). Add sentinel events table; note any divergence from standalone.
3. Tests
Spec covers happy connect → close → end only. Add:
destroy()path.- Double-close / double-destroy:
endfires exactly once. - Failover:
reconnecting+ re-emittedready.
Summary
Fixes #3012.
createSentinel()exposesisOpenandisReady, but it did not emit the lifecycle events that standalone clients expose. This addsconnectandreadyemissions after Sentinel finishes connecting, and emitsendafter closing or destroying an instance that was open.The change is additive and keeps the event ordering aligned with the state a Sentinel consumer can observe.
Validation
.\node_modules\.bin\tsc.cmd --build packages/client --force --verbosenpm run lint:changedgit diff --checkNote: the focused Sentinel mocha test could not run locally because Docker is not installed in this Windows environment (
spawn docker ENOENT).Note
Low Risk
Low risk additive behavior change: emits
connect/readyafter successfulRedisSentinel.connect()and emitsendonclose()/destroy()when previously open; could affect consumers that assume no events or different ordering.Overview
RedisSentinelnow emits lifecycle events consistent with standalone clients:connectandreadyare emitted afterconnect()completes, andendis emitted afterclose()ordestroy()when the instance was previously open.Adds a focused sentinel test (
lifecycle-events.spec.ts) asserting theconnect→ready→endevent sequence and thatendis observable viaonce()after shutdown.Reviewed by Cursor Bugbot for commit ffdc7b5. Bugbot is set up for automated code reviews on this repo. Configure here.