Skip to content

fix(queue): durable Redis DLQ wired into all job processors - #23

Merged
ameeribro4-sudo merged 4 commits into
OpenPeerX:mainfrom
DeRossa1:fix/issue-16-durable-dlq
Aug 20, 2026
Merged

fix(queue): durable Redis DLQ wired into all job processors#23
ameeribro4-sudo merged 4 commits into
OpenPeerX:mainfrom
DeRossa1:fix/issue-16-durable-dlq

Conversation

@DeRossa1

Copy link
Copy Markdown
Contributor

Summary

Closes #16

Makes the dead-letter queue durable and actually populated. DeadLetterQueueService now stores DLQ items in Redis hashes (dlq:{queueName} keyed by job id) via the existing RedisPoolService, so permanently failed jobs survive restarts and are shared across horizontally scaled instances — the same store the admin endpoints read and write. The email, notification, report, and cleanup processors now route their permanently failed jobs into it through a shared isPermanentFailure(job) helper (attemptsMade >= opts.attempts), replacing the logging-only @OnQueueFailed handlers. The single most important design decision is the permanent-vs-retryable rule as a single exported helper: @OnQueueFailed fires on every failed attempt, so without the shared rule each processor would either flood the DLQ with retryable failures or need its own copy of the threshold logic.

Why

The DLQ was dead code for the queues that need it most: dlqItems was an in-memory Map, wiped on every restart, and none of the four processors ever called addToDLQ — their failure paths only logged. So a Bull job that exhausted its attempts sat in Bull's own failed set until removeOnFail deleted it (7 days), with no record left for manual recovery, and QueueAdminController.recoverDLQJob could never find anything for the email/cleanup/report queues. The only addToDLQ callers were scheduler-failover.service.ts, which watches just notification and report — and even those entries vanished on restart. Redis (not Postgres) is the right store because the queue layer already depends on Redis for Bull and RedisPoolService is the existing accessor, so this adds no new infrastructure; hashes keyed by job id give O(1) lookup for recoverJob/removeDLQItem and natural per-queue isolation.

What was built

src/queue/dead-letter-queue.service.ts (rewritten; tests in src/queue/dead-letter-queue.service.spec.ts):

File What it contains
dead-letter-queue.service.ts Durable store over RedisPoolService.withClient: addToDLQ HSETs a JSON-serialized DLQItem into dlq:{queueName}; getDLQItems/getDLQItem/getDLQStats read the hash (oldest-first ordering by failedAt); removeDLQItem/clearDLQ operate on the hash; recoverJob HGETs the item, re-enqueues onto the original queue, and only then HDELs the entry — a failed re-enqueue keeps the record. The hourly retention cleanup sweeps the hash instead of the in-memory map, and the timer is unref()'d so it does not keep the process alive. Exports isPermanentFailure(job), the canonical attemptsMade >= opts.attempts rule mirroring SchedulerFailoverService.canRetryJob.
dead-letter-queue.service.spec.ts (new) 8 tests against a faithful in-memory Redis fake (hash + TTL semantics): add-to-DLQ with reason, restart survival (new service instance against the same store), recoverJob re-enqueues-and-removes only on success, recoverJob keeps the item when re-enqueueing fails, unknown-job false, clearDLQ/removeDLQItem on the durable store, per-queue stats.

Processor wiring (all four, mirroring the swap processor's pattern from #15):

File What it contains
email.processor.ts Injects DeadLetterQueueService; onFailed now async and, on isPermanentFailure(job), calls addToDLQ(job, error, MAX_RETRIES_EXCEEDED, QueueName.EMAILS) (keeps the existing admin-notify call).
notification.processor.ts Same wiring for QueueName.NOTIFICATIONS (replaces the log-only permanent-failure branch).
report.processor.ts Same wiring for QueueName.REPORTS (replaces the log-only onFailed).
cleanup.processor.ts Same wiring for QueueName.CLEANUP (replaces the log-only onFailed).
dlq-wiring.spec.ts (new) 6 tests asserting each of the four processors routes a permanently failed job to addToDLQ with MAX_RETRIES_EXCEEDED and the right queue name, and that a retryable failure (attempts < opts.attempts) is not DLQ'd.

Integration changes outside src/queue/processors/

  • src/queue/queue-admin.controller.ts — awaits the now-async getDLQStats (×2), removeDLQItem, and clearDLQ calls (previously sync). No endpoint shape changes.
  • README.md — documents the durable Redis backend, the permanent-vs-retryable rule, and the recovery ordering under the Redis setup section.

Note on the two DLQ classes: this keeps src/queue/dead-letter-queue.service.ts (Bull job DLQ) independent of src/common/services/dead-letter-queue.service.ts (message-level DLQ used by the error dashboard) — the issue allows staying independent, and merging them would couple the queue layer to the error-dashboard contract for no functional gain.

Acceptance criteria coverage

  • A job that exhausts its attempts in the email, notification, report, or cleanup queue is added to the DLQ with a correct DLQReason. (dlq-wiring.spec.ts — all four processors assert addToDLQ(job, error, MAX_RETRIES_EXCEEDED, <queue>); dead-letter-queue.service.spec.ts asserts the reason is stored)
  • recoverJob re-enqueues a DLQ item and removes it from the DLQ only on success. (dead-letter-queue.service.spec.ts — "recoverJob re-enqueues and removes the item only on success" asserts the re-enqueue args and empty store after; "recoverJob keeps the DLQ item when re-enqueueing fails" asserts the entry survives a failed re-enqueue)
  • DLQ items survive a process restart. (dead-letter-queue.service.spec.ts — "persists DLQ items across a service reinstantiation (restart survival)" reinstantiates the service against the same store)
  • A processor spec asserts that a permanently failed job reaches addToDLQ with MAX_RETRIES_EXCEEDED. (dlq-wiring.spec.ts — every processor test asserts the reason)
  • A test verifies recoverJob and clearDLQ operate on the durable store. (dead-letter-queue.service.spec.ts — recoverJob tests use a fresh service instance sharing the store; clearDLQ test asserts the hash is emptied and other queues untouched)
  • The DLQ storage backend and the permanent-vs-retryable failure rule are documented. (README — Redis dlq:{queueName} hashes, attemptsMade >= opts.attempts rule, recovery ordering)

Test plan

  • npm run build — succeeds
  • npx jest src/queue/dead-letter-queue.service.spec.ts src/queue/processors/dlq-wiring.spec.ts — 14/14 passing (14 new tests)
  • npm run test — 464/516 passing vs 450/502 on base; failing suites identical to base (pre-existing ts-jest/uuid resolution failures); zero new failures
  • npx eslint on changed source — no new findings versus base (64 issues on base → 59 after, on the same files; the difference is pre-existing findings removed by typing the getQueueByName switch predicate; my new spec files carry 0 errors and only the same no-unsafe-argument warning class existing e2e specs carry)
  • npx madge --circular --extensions ts src/ — same 3 pre-existing cycles, none involving queue
  • Prettier — all changed files formatted; the README section added is prettier-clean

Env vars / Notes

No new env vars. The DLQ rides the existing Redis connection (REDIS_HOST/REDIS_PORT) already required by Bull and the cache layer; Redis must be reachable for DLQ persistence (the queue layer already requires it). Retention is governed by the existing DLQConfig.maxAge (30 days) enforced by an hourly sweep that now operates on the durable store. Delivery semantics unchanged: @OnQueueFailed fires on every failed attempt, but only final-attempt failures (attemptsMade >= opts.attempts, the same rule SchedulerFailoverService.canRetryJob uses) are recorded, so retryable failures never enter the DLQ. This PR intentionally does not touch retry/backoff policies (out of scope per the issue) nor the message-level DLQ in src/common/services/.

Store DLQ items in Redis hashes (dlq:{queueName} keyed by job id) so
permanently failed jobs survive restarts and are visible across
instances, and wire the email, notification, report, and cleanup
processors to addToDLQ on final-attempt failure via a shared
isPermanentFailure rule. recoverJob re-enqueues before removing the
entry, so a failed recovery never loses the record.
@DeRossa1
DeRossa1 force-pushed the fix/issue-16-durable-dlq branch from 448cab8 to a10c8fe Compare August 20, 2026 15:52

@ameeribro4-sudo ameeribro4-sudo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DeRossa1 resolve conflicts

@ameeribro4-sudo ameeribro4-sudo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix the Conflicting files

@ameeribro4-sudo ameeribro4-sudo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ameeribro4-sudo
ameeribro4-sudo merged commit 26e2746 into OpenPeerX:main Aug 20, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants