fix: surface failed campaign recipients#415
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds transactional recording for campaign contact email failures, including failed email status, campaign links, and email events. Campaign processing now propagates campaign-email creation errors to per-contact handling, logs failures, records them, and continues processing subsequent contacts. Email queue job creation is now awaited. Unit tests cover existing, new, and reused email records. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Deploying usesend with
|
| Latest commit: |
06bca69
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://692be94a.usesend.pages.dev |
| Branch Preview URL: | https://codex-campaign-failure-event.usesend.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/server/service/campaign-contact-failure-service.ts`:
- Line 2: Update the db import in the campaign contact failure service to use
the configured `~/` alias, replacing the relative `../db` path with
`~/server/db`.
In `@apps/web/src/server/service/campaign-service.ts`:
- Around line 1044-1061: recordCampaignContactFailure can throw from the
contact-processing catch block and abort batch iteration. Wrap its invocation
within a nested try/catch, log failures with the contact and campaign
identifiers, and allow execution to continue so the cursor advancement after the
catch still occurs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79ae3fd1-59a7-4eed-8a02-b1bbfe905c28
📒 Files selected for processing (4)
apps/web/src/server/service/campaign-contact-failure-service.tsapps/web/src/server/service/campaign-contact-failure-service.unit.test.tsapps/web/src/server/service/campaign-service.tsapps/web/src/server/service/email-queue-service.ts
| @@ -0,0 +1,103 @@ | |||
| import { Campaign, Contact } from "@prisma/client"; | |||
| import { db } from "../db"; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use ~/ alias for the db import.
The coding guidelines for apps/web/src/**/*.{ts,tsx} require the ~/ alias for imports from src. Use ~/server/db instead of the relative ../db.
🔧 Proposed fix
-import { db } from "../db";
+import { db } from "~/server/db";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { db } from "../db"; | |
| import { db } from "~/server/db"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/server/service/campaign-contact-failure-service.ts` at line 2,
Update the db import in the campaign contact failure service to use the
configured `~/` alias, replacing the relative `../db` path with `~/server/db`.
Source: Coding guidelines
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/server/service/campaign-service.unit.test.ts (1)
103-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider adding coverage for the batch continue-on-failure path.
These tests exercise
recordCampaignContactFailurepersistence in isolation, but the central behavior of this PR — a per-contact failure being caught, recorded, and the loop continuing to subsequent contacts (and the cursor still advancing) — isn't covered. A worker-level test asserting that one throwingprocessContactEmaildoesn't abort the batch would guard against regressing issue#408.As per path instructions, add tests when changes impact logic or behavior in apps/web.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/server/service/campaign-service.unit.test.ts` around lines 103 - 107, Add a worker-level test covering the batch continue-on-failure behavior: mock one contact’s processContactEmail call to throw, assert recordCampaignContactFailure is invoked, verify later contacts are still processed, and confirm the cursor advances. Place it alongside the existing recordCampaignContactFailure tests and follow the repository’s app/web test conventions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/src/server/service/campaign-service.unit.test.ts`:
- Around line 103-107: Add a worker-level test covering the batch
continue-on-failure behavior: mock one contact’s processContactEmail call to
throw, assert recordCampaignContactFailure is invoked, verify later contacts are
still processed, and confirm the cursor advances. Place it alongside the
existing recordCampaignContactFailure tests and follow the repository’s app/web
test conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08ec1da1-6c4e-45ac-acfe-f742d24a04a9
📒 Files selected for processing (2)
apps/web/src/server/service/campaign-service.tsapps/web/src/server/service/campaign-service.unit.test.ts
Closes #408
Builds on #411 by @tejassinghbhati. Their original skip-and-continue commit is preserved in this branch so its authorship remains in the Git history.
What changed
FAILEDemail andEmailEventcontaining the failure message for every skipped contact.queue.add()so asynchronous enqueue failures reach the campaign failure handler.CampaignEmailpersistence failures.User impact
Failed campaign recipients now appear with a
FAILEDstatus in campaign activity and the email list. Opening the email shows the underlying message in the existing Events History UI. Other contacts in the batch continue processing.Verification
pnpm --filter web exec vitest run -c vitest.unit.config.ts src/server/service/campaign-service.unit.test.tspnpm --filter web exec tsc --noEmitSummary by cubic
Keep campaign batches moving when individual contacts fail. Each failed contact is recorded as a
FAILEDemail with an event and error message; enqueue errors fromBullMQare surfaced.FAILED, and add an event with the error message.CampaignEmaillinking errors; let them reach the batch handler to be recorded and continue.BullMQqueue.add()so enqueue errors are captured instead of being lost.Written for commit 06bca69. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests