Dead-letter queue is never populated: permanently failed jobs are unrecoverable and lost on restart

[ameeribro4-sudo]

Labels / Complexity: bug · High — 600 points

Problem

The dead-letter queue (DLQ) is effectively dead code for the job queues that need it most. DeadLetterQueueService (src/queue/dead-letter-queue.service.ts) keeps its items in an in-memory map:

// src/queue/dead-letter-queue.service.ts
private dlqItems: Map<string, DLQItem[]> = new Map();

More importantly, none of the four Bull processors ever populate it. Each processor's failure path is @OnQueueFailed, which only logs:

// src/queue/processors/email.processor.ts
@OnQueueFailed()
onFailed(job: Job<EmailJobData>, error: Error): void {
  // ... logs "permanently failed" ...
  this.notifyAdminOfFailure(job, error);   // another log
}

The same logging-only pattern appears in notification.processor.ts, report.processor.ts, and cleanup.processor.ts. The only callers of addToDLQ(...) are in scheduler-failover.service.ts (src/queue/scheduler-failover.service.ts), which watches only the notification and report queues for stalled jobs and auto-retry failures.

The consequences are concrete:

• Permanently failed jobs are not persisted. When a Bull job exhausts its attempts, it stays in Bull's own failed set subject to removeOnFail: { age: 7 days } (configured in src/queue/queue.module.ts), then is deleted. The DLQ that is supposed to hold them for manual recovery is never written.
• recoverJob has nothing to recover. QueueAdminController.recoverDLQJob (src/queue/queue-admin.controller.ts) calls dlqService.recoverJob(queueName, jobId), but for the email/cleanup/report queues the lookup will always miss, because those items were never added.
• Whatever is stored is lost on restart. Even the scheduler-failover entries are held only in dlqItems (in-memory), so a restart wipes the DLQ.

Root cause

// src/queue/processors/email.processor.ts
@OnQueueFailed()
onFailed(job, error) {
  // ← no DeadLetterQueueService.addToDLQ(job, error, ...) call
}

Why this is architecturally hard

1. The shortcut is wrong. Adding a log line in onFailed does not make a DLQ. A real DLQ needs a durable store (Redis or Postgres) and a canonical "permanent failure" signal, not a per-processor log branch.
2. Detection of "permanent" is subtle. @OnQueueFailed fires on every failed attempt, not just the final one. The processor must distinguish a retryable failure from a permanent one (job.attemptsMade >= job.opts.attempts) and only then enqueue to the DLQ, matching scheduler-failover.service.ts's canRetryJob logic.
3. Recovery must re-enqueue safely. recoverJob (src/queue/dead-letter-queue.service.ts) adds the job back via queue.add(item.jobData, ...). If the DLQ becomes durable, recovery must also be idempotent and remove the item only after a successful re-enqueue.
4. There are two DLQ implementations. src/common/services/dead-letter-queue.service.ts is a separate class referenced by src/common/controllers/error-dashboard.controller.ts. The contributor must decide whether the queue DLQ should align with or stay independent of that one, and document the split.

Proposed design

Make dlqItems durable (Redis list/hash keyed by queueName, or a DLQItem table) and wire every processor's final failure into it. A clean boundary is a shared helper, for example:

// on final attempt failure:
if ((job.attemptsMade ?? 0) >= (job.opts.attempts ?? 3)) {
  await this.dlqService.addToDLQ(job, error, DLQReason.MAX_RETRIES_EXCEEDED, queueName);
}

recoverJob should atomically pop the item only after the re-enqueue succeeds, and clearDLQ/removeDLQItem must operate on the durable store so the admin endpoints in queue-admin.controller.ts actually manage what the processors produce.

Acceptance criteria

Service

• A job that exhausts its attempts in the email, notification, report, or cleanup queue is added to the DLQ with a correct DLQReason.
• recoverJob re-enqueues a DLQ item and removes it from the DLQ only on success.
• DLQ items survive a process restart.

Tests

• A processor spec asserts that a permanently failed job reaches addToDLQ with MAX_RETRIES_EXCEEDED.
• A test verifies recoverJob and clearDLQ operate on the durable store.

Documentation

• The DLQ storage backend and the permanent-vs-retryable failure rule are documented.

Out of scope

Do not change the retry/backoff policies themselves, and do not merge the two DLQ classes unless that is a natural consequence of choosing a shared store.

Getting started

Files in scope: src/queue/dead-letter-queue.service.ts, the four processors under src/queue/processors/, src/queue/queue-admin.controller.ts, and (for the durable store) the Redis utilities under src/common/cache/.

Build and test with:

npm run build
npm run test

Good first files to read: src/queue/scheduler-failover.service.ts (the one place addToDLQ is called today), src/queue/processors/email.processor.ts.

[DeRossa1]

Hello maintainer, please assign me this issue, I am a professional. I will do the best. I understand the scope

[grantfox-oss]

🦊 GrantFox — @DeRossa1 has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #16)
2. Your PR will be reviewed by the OpenPeerX maintainers

Good luck! Track your progress on GrantFox.

[grantfox-oss]

🎉 This issue has been marked as completed on GrantFox as part of the Third Campaign campaign!

@DeRossa1's PR #23 was approved and merged by @ameeribro4-sudo.

🏆 @DeRossa1: You earned 35 FoxPoints for this contribution! Your current tier: Explorer (274 total points). Track your full progress on GrantFox.

👏 Great work, @DeRossa1! Keep contributing to OpenPeerX.