Skip to content

Refine suppression list to include only affected recipients#339

Merged
KMKoushik merged 1 commit intousesend:mainfrom
infordoc:main
Jan 17, 2026
Merged

Refine suppression list to include only affected recipients#339
KMKoushik merged 1 commit intousesend:mainfrom
infordoc:main

Conversation

@tpraxedes
Copy link
Copy Markdown
Contributor

@tpraxedes tpraxedes commented Jan 12, 2026

Refactor suppression list logic to only include recipients that actually bounced or complained, improving accuracy in handling email suppression.


Summary by cubic

Limit suppression list updates to only recipients that actually bounced or complained, based on SES event data. Prevents suppressing unaffected recipients and improves accuracy.

  • Bug Fixes
    • Use bounce.bouncedRecipients and complaint.complainedRecipients instead of all message recipients.
    • Apply suppression only for permanent (hard) bounces and complaints.
    • Skip when no affected recipients are found and log a warning; improve error logging without failing webhook processing.

Written for commit d21f7ca. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes
    • Improved email suppression handling to only suppress actually affected recipients instead of all email recipients.
    • Enhanced error handling in suppression logic to prevent failures from interrupting the process.
    • Added more targeted logging for bounce and complaint events.

✏️ Tip: You can customize this high-level summary in your review settings.

Refactor suppression list logic to only include recipients that actually bounced or complained, improving accuracy in handling email suppression.
@vercel
Copy link
Copy Markdown

vercel bot commented Jan 12, 2026

@tpraxedes is attempting to deploy a commit to the kmkoushik's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 12, 2026

Walkthrough

Modified the SES webhook parser to populate suppression lists using only the actually affected recipients from SES event data instead of all recipients from email headers. For hard bounces, the parser now extracts recipients from the bounce data; for complaints, from the complaint data. Added a guard condition to proceed only when affected recipients exist, wrapped suppression calls in error handling, and refined logging to distinguish between bounce and complaint scenarios while maintaining the existing suppression reason selection logic.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: refining suppression list logic to include only affected recipients from SES event data instead of all email recipients.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/web/src/server/service/ses-hook-parser.ts (1)

139-172: Consider using Promise.allSettled for better partial failure handling.

With Promise.all, if one suppression fails, the entire batch rejects and you lose visibility into which recipients succeeded vs failed. Using Promise.allSettled would allow all suppressions to be attempted and provide per-recipient success/failure status in logs.

Given that addSuppression uses upsert and webhooks can be retried, the current approach is acceptable, but Promise.allSettled would improve observability.

♻️ Optional improvement with Promise.allSettled
-      try {
-        await Promise.all(
-          recipientEmails.map((recipientEmail) =>
-            SuppressionService.addSuppression({
-              email: recipientEmail,
-              teamId: email.teamId,
-              reason: isHardBounced
-                ? SuppressionReason.HARD_BOUNCE
-                : SuppressionReason.COMPLAINT,
-              source: email.id,
-            }),
-          ),
-        );
-
-        logger.info(
-          {
-            emailId: email.id,
-            recipients: recipientEmails,
-            reason: isHardBounced ? "HARD_BOUNCE" : "COMPLAINT",
-          },
-          "Added emails to suppression list due to bounce/complaint",
-        );
-      } catch (error) {
-        logger.error(
-          {
-            emailId: email.id,
-            recipients: recipientEmails,
-            error: error instanceof Error ? error.message : "Unknown error",
-          },
-          "Failed to add emails to suppression list",
-        );
-        // Don't throw error - continue processing the webhook
-      }
+      const results = await Promise.allSettled(
+        recipientEmails.map((recipientEmail) =>
+          SuppressionService.addSuppression({
+            email: recipientEmail,
+            teamId: email.teamId,
+            reason: isHardBounced
+              ? SuppressionReason.HARD_BOUNCE
+              : SuppressionReason.COMPLAINT,
+            source: email.id,
+          }),
+        ),
+      );
+
+      const succeeded = recipientEmails.filter((_, i) => results[i].status === "fulfilled");
+      const failed = recipientEmails.filter((_, i) => results[i].status === "rejected");
+
+      if (succeeded.length > 0) {
+        logger.info(
+          {
+            emailId: email.id,
+            recipients: succeeded,
+            reason: isHardBounced ? "HARD_BOUNCE" : "COMPLAINT",
+          },
+          "Added emails to suppression list due to bounce/complaint",
+        );
+      }
+
+      if (failed.length > 0) {
+        logger.error(
+          {
+            emailId: email.id,
+            failedRecipients: failed,
+          },
+          "Failed to add some emails to suppression list",
+        );
+      }
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68d951c and d21f7ca.

📒 Files selected for processing (1)
  • apps/web/src/server/service/ses-hook-parser.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{tsx,ts,jsx,js}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Include all required imports and ensure proper naming of key components in React/NextJS code

Files:

  • apps/web/src/server/service/ses-hook-parser.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use TypeScript-first approach with 2-space indent and semicolons enabled by Prettier in apps/web (Next.js), apps/marketing, apps/smtp-server, and all packages
Never use dynamic imports; always import on the top level
Run ESLint via @usesend/eslint-config and ensure no warnings remain before submitting PRs

Files:

  • apps/web/src/server/service/ses-hook-parser.ts
apps/web/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use alias ~/ for src imports in apps/web (e.g., import { x } from "~/utils/x")

Files:

  • apps/web/src/server/service/ses-hook-parser.ts
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/web/**/*.{ts,tsx}: Prefer to use TRPC for client-server communication unless explicitly asked otherwise in apps/web
Use Prisma for database access in apps/web

Files:

  • apps/web/src/server/service/ses-hook-parser.ts
**/*.{ts,tsx,md}

📄 CodeRabbit inference engine (AGENTS.md)

Run Prettier 3 for code formatting on TypeScript, TSX, and Markdown files

Files:

  • apps/web/src/server/service/ses-hook-parser.ts
🧬 Code graph analysis (1)
apps/web/src/server/service/ses-hook-parser.ts (2)
apps/web/src/server/logger/log.ts (1)
  • logger (31-63)
apps/web/src/server/service/suppression-service.ts (1)
  • SuppressionService (28-393)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: cubic · AI code reviewer
🔇 Additional comments (2)
apps/web/src/server/service/ses-hook-parser.ts (2)

123-136: LGTM - Recipient extraction logic is well-structured.

The conditional extraction using optional chaining ensures null safety, and the logic correctly distinguishes between bounce recipients and complaint recipients.


173-181: Good defensive logging for edge cases.

This warning appropriately catches scenarios where a bounce or complaint event arrives without the expected recipient data, which could indicate malformed SES payloads or unexpected event structures.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Copy link
Copy Markdown
Member

@KMKoushik KMKoushik left a comment

Choose a reason for hiding this comment

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

great catch

@KMKoushik KMKoushik merged commit 83119f9 into usesend:main Jan 17, 2026
2 of 3 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.

2 participants