Skip to content

Commit c6524ca

Browse files
committed
fix(redis-worker): stop losing dead-lettered messages when the DLQ write fails
The message was removed from in-flight before the dead-letter entry was written, and the write went through a pipeline whose per-command errors were never inspected. A failed write therefore lost the message silently: gone from in-flight, absent from the dead-letter queue, invisible to the reclaim loop. Write the entry first and only complete the message once it lands. On failure the message stays in-flight, so the reclaim loop picks it up and it is retried rather than dropped. A persistently failing write now loops visibly instead of discarding work.
1 parent b2ebf18 commit c6524ca

2 files changed

Lines changed: 93 additions & 4 deletions

File tree

packages/redis-worker/src/fair-queue/index.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,14 +1498,28 @@ export class FairQueue<TPayloadSchema extends z.ZodTypeAny = z.ZodUnknown> {
14981498
originalTimestamp: storedMessage.timestamp,
14991499
};
15001500

1501-
// Complete in visibility manager
1502-
await this.visibilityManager.complete(storedMessage.id, storedMessage.queueId);
1503-
15041501
// Add to DLQ
15051502
const pipeline = this.redis.pipeline();
15061503
pipeline.zadd(dlqKey, dlqMessage.deadLetteredAt, storedMessage.id);
15071504
pipeline.hset(dlqDataKey, storedMessage.id, JSON.stringify(dlqMessage));
1508-
await pipeline.exec();
1505+
const dlqResults = await pipeline.exec();
1506+
1507+
const dlqErrors = (dlqResults ?? [])
1508+
.map(([error]) => error)
1509+
.filter((error): error is Error => Boolean(error));
1510+
1511+
if (dlqErrors.length > 0) {
1512+
this.logger.error("Failed to write message to DLQ, leaving it in-flight to be reclaimed", {
1513+
messageId: storedMessage.id,
1514+
queueId: storedMessage.queueId,
1515+
tenantId: storedMessage.tenantId,
1516+
errors: dlqErrors.map((error) => error.message),
1517+
});
1518+
return;
1519+
}
1520+
1521+
// Complete in visibility manager
1522+
await this.visibilityManager.complete(storedMessage.id, storedMessage.queueId);
15091523

15101524
this.telemetry.recordDLQ();
15111525

packages/redis-worker/src/fair-queue/tests/fairQueue.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,6 +1517,81 @@ describe("FairQueue", () => {
15171517
}
15181518
);
15191519

1520+
redisTest(
1521+
"should keep a message in-flight when the DLQ write fails",
1522+
{ timeout: 15000 },
1523+
async ({ redisOptions }) => {
1524+
const started: string[] = [];
1525+
keys = new DefaultFairQueueKeyProducer({ prefix: "test" });
1526+
1527+
const scheduler = new DRRScheduler({
1528+
redis: redisOptions,
1529+
keys,
1530+
quantum: 10,
1531+
maxDeficit: 100,
1532+
});
1533+
1534+
const queue = new TestFairQueueHelper(redisOptions, keys, {
1535+
scheduler,
1536+
payloadSchema: TestPayloadSchema,
1537+
shardCount: 1,
1538+
consumerCount: 1,
1539+
consumerIntervalMs: 20,
1540+
visibilityTimeoutMs: 60000,
1541+
reclaimIntervalMs: 60000,
1542+
concurrencyGroups: [
1543+
{
1544+
name: "tenant",
1545+
extractGroupId: (q) => q.tenantId,
1546+
getLimit: async () => 5,
1547+
defaultLimit: 5,
1548+
},
1549+
],
1550+
startConsumers: false,
1551+
});
1552+
1553+
const redis = createRedisClient(redisOptions);
1554+
const dlqKey = keys.deadLetterQueueKey("t1");
1555+
1556+
try {
1557+
await redis.set(dlqKey, "not-a-zset");
1558+
1559+
queue.onMessage(async (ctx) => {
1560+
started.push(ctx.message.payload.value);
1561+
await ctx.fail(new Error("boom"));
1562+
});
1563+
1564+
await queue.enqueue({
1565+
queueId: "tenant:t1:queue:q1",
1566+
tenantId: "t1",
1567+
payload: { value: "msg-0" },
1568+
});
1569+
1570+
queue.start();
1571+
1572+
await vi.waitFor(
1573+
() => {
1574+
expect(started).toHaveLength(1);
1575+
},
1576+
{ timeout: 10000 }
1577+
);
1578+
1579+
await vi.waitFor(
1580+
async () => {
1581+
expect(await redis.scard(keys.concurrencyKey("tenant", "t1"))).toBe(0);
1582+
},
1583+
{ timeout: 5000 }
1584+
);
1585+
1586+
expect(await redis.zcard(keys.inflightKey(0))).toBe(1);
1587+
} finally {
1588+
await redis.del(dlqKey);
1589+
await redis.quit();
1590+
await queue.close();
1591+
}
1592+
}
1593+
);
1594+
15201595
redisTest(
15211596
"should release the concurrency slot even when the reclaim requeue fails",
15221597
{ timeout: 15000 },

0 commit comments

Comments
 (0)