Skip to content

Commit 4cdcb94

Browse files
logaretmclaude
andcommitted
fix: prevent duplicate trace events for offline-queued commands
When a command was queued offline, traceCommand wrapped command.promise at queue time. When the connection became ready and the command was re-sent via sendCommand, it was wrapped again — producing duplicate start/end/error trace events and inflated latency in the first span. Move tracing to the write path only so offline-queued commands are traced once when actually sent to Redis. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 150a9ce commit 4cdcb94

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

lib/Redis.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,14 @@ class Redis extends Commander implements DataHandledable {
568568
}
569569
}
570570

571-
// Wrap command promise with tracing (zero-cost when no subscribers)
571+
if (!writable) {
572+
// Offline-queued: skip tracing here. The command will be re-sent via
573+
// sendCommand when the connection is ready, and traced at that point.
574+
return command.promise;
575+
}
576+
577+
// Trace on the write path only, so offline-queued commands that get
578+
// re-sent aren't traced twice.
572579
return traceCommand(
573580
() => command.promise as Promise<unknown>,
574581
() => this._buildCommandContext(command)

test/functional/tracing.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,37 @@ describeOrSkip("tracing", function () {
205205
redis.disconnect();
206206
});
207207

208+
it("should not produce duplicate trace events for offline-queued commands", async function () {
209+
const events: any[] = [];
210+
const subscriber = {
211+
start(message: any) { events.push(message); },
212+
end() {},
213+
asyncStart() {},
214+
asyncEnd() {},
215+
error() {},
216+
};
217+
218+
const channel = dc.tracingChannel("ioredis:command");
219+
channel.subscribe(subscriber);
220+
221+
try {
222+
const redis = new Redis({ lazyConnect: true });
223+
// Start connecting but don't await — puts Redis in "connecting" state
224+
const connectPromise = redis.connect();
225+
// Issue command while still connecting — it goes to the offline queue
226+
const setPromise = redis.set("offline-key", "offline-value");
227+
await connectPromise;
228+
await setPromise;
229+
redis.disconnect();
230+
231+
const setEvents = events.filter((e) => e.command === "set");
232+
// Should trace exactly once, not twice
233+
expect(setEvents.length).to.eql(1);
234+
} finally {
235+
channel.unsubscribe(subscriber);
236+
}
237+
});
238+
208239
it("should trace errors on failed commands", async function () {
209240
const errorEvents: any[] = [];
210241
const subscriber = {

0 commit comments

Comments
 (0)