fix(run-engine): publish dead-letter redrives to the configured queue channel - #4855
Conversation
… channel redriveMessage published to the hardcoded rq:redrive channel while the subscriber listens on the configured queue name, so redrive silently no-oped for any RunQueue not named rq. Both sides now derive the channel from the queue name, and a redrive that reaches zero subscribers logs an error instead of reporting nothing. Fixes #4854
|
WalkthroughThe run queue now derives its redrive channel from Merge Risk: 🔵 Low · up to This change restores dead-letter redrives for non-default queues and logs when no subscriber receives a redrive. It is mergeable with owner awareness because an interruption or duplicate delivery during the existing multi-step redrive flow could leave a run both active and dead-lettered or reactivate it more than once. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the bug, the fix, the affected test, and references issue Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal-packages/run-engine/src/run-queue/index.ts (1)
674-695: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd temporary crumb markers to all changed redrive code.
The changed production blocks and test override do not contain the required crumb markers. Add them during development, then run
agentcrumbs stripbefore merge.
internal-packages/run-engine/src/run-queue/index.ts#L674-L695: mark the redrive publish and zero-subscriber logging block.internal-packages/run-engine/src/run-queue/index.ts#L1475-L1480: mark the queue-specific redrive channel getter and subscriber setup.internal-packages/run-engine/src/run-queue/index.test.ts#L966-L966: mark the non-default queue-name override.As per coding guidelines, add crumbs as you write code and strip them before merge.
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 00c323a0-548a-4d20-ba2d-1997354118f2
📒 Files selected for processing (2)
internal-packages/run-engine/src/run-queue/index.test.tsinternal-packages/run-engine/src/run-queue/index.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (32)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: typecheck / typecheck
- GitHub Check: runops-guard / runops-guard
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: code-quality / code-quality
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (7)
We use vitest exclusively. **Never mock anything** - use testcontainers instead.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal-packages/run-engine/src/run-queue/index.test.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal-packages/run-engine/src/run-queue/index.test.tsinternal-packages/run-engine/src/run-queue/index.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal-packages/run-engine/src/run-queue/index.test.tsinternal-packages/run-engine/src/run-queue/index.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
internal-packages/run-engine/src/run-queue/index.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
internal-packages/run-engine/src/run-queue/index.test.tsinternal-packages/run-engine/src/run-queue/index.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
internal-packages/run-engine/src/run-queue/index.test.tsinternal-packages/run-engine/src/run-queue/index.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
internal-packages/run-engine/src/run-queue/index.test.tsinternal-packages/run-engine/src/run-queue/index.ts
2 participants
Summary
Redriving a run out of the dead letter queue published to the hardcoded Redis channel
rq:redrive, while the subscriber listens on${options.name}:redrive. For anyRunQueuenot namedrqthe publish reached zero subscribers, Redis reported success, and the run stayed in the dead letter queue with no log, error, or metric.Both sides now derive the channel from the same expression, and
redriveMessagelogs an error when a redrive publish reaches zero subscribers instead of failing silently.The existing "Dead Letter Queue" test now constructs its queue as
rq-redrive, so it fails against the old code (verified red before the fix, green after) and stops the channel names from silently re-locking to a single magic value.Fixes #4854