Retry sending emails on transient errors

[nygrenh]

Summary by CodeRabbit

• Bug Fixes
  • Improved email delivery reliability with automatic retries for transient SMTP errors and exponential backoff.
  • Enhanced logging and diagnostics for clearer visibility into email sending attempts.
  • Ensured proper cleanup of sending resources to reduce stuck or failed deliveries.

[coderabbitai]

Warning

Rate limit exceeded

@nygrenh has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 30 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

📝 Walkthrough

🚥 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 'Retry sending emails on transient errors' directly and concisely describes the main change across both modified files: adding retry logic to email sending with transient error classification.

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

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch email-transient-errors-retry

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.33%. Comparing base (b959e8d) to head (478169f).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1309      +/-   ##
==========================================
- Coverage   65.65%   65.33%   -0.33%     
==========================================
  Files         154      154              
  Lines        6619     6652      +33     
  Branches     1639     1650      +11     
==========================================
  Hits         4346     4346              
- Misses       2120     2151      +31     
- Partials      153      155       +2     

Flag	Coverage Δ
backend	65.33% <ø> (-0.33%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@backend/util/sendMail.ts`:
- Around line 34-35: The assigned logger variable (const logger =
context?.logger ?? console) creates a union type (winston.Logger | Console) that
breaks type-checking when passed into withRetries; remove passing logger into
withRetries and drop the logger field from RetryOptions (or alternatively change
RetryOptions.logger to a minimal shape like { info: (...args:any[]) => void } if
you need it later), and keep using logInfo (logger.info.bind(logger)) where
needed; update the withRetries call sites to stop supplying logger and adjust
the RetryOptions definition and any consumers accordingly.

🧹 Nitpick comments (2)

backend/util/sendMail.ts (2)

79-86: Unused logger parameter and misleading maxRetries name.

Two issues in RetryOptions / withRetries:

1. Dead parameter: logger (line 81/90) is destructured but never referenced in the function body — only logInfo is used. Remove it to avoid the type mismatch described above and reduce confusion.

2. Naming: maxRetries: 3 actually produces 3 total attempts (loop: attempt <= maxRetries). Someone setting maxRetries: 1 would expect one retry but get only one attempt total (i.e., zero retries). Consider renaming to maxAttempts, or adjusting the loop to attempt <= maxRetries + 1.

Suggested diff

 interface RetryOptions {
-  maxRetries: number
-  logger?: winston.Logger
-  logInfo?: (message: string) => void
+  maxAttempts: number
+  log?: (message: string) => void
   operationName: string
   operation: (attempt: number) => Promise<void>
   isTransientError: (error: unknown) => boolean
 }

 async function withRetries({
-  maxRetries,
-  logger,
-  logInfo,
+  maxAttempts,
+  log: logFn,
   operationName,
   operation,
   isTransientError,
 }: RetryOptions) {
   const BASE_DELAY_MS = 1000
   const jitter = () => Math.floor(Math.random() * 250)
-  const log = logInfo ?? ((message: string) => console.log(message))
+  const log = logFn ?? ((message: string) => console.log(message))

-  for (let attempt = 1; attempt <= maxRetries; attempt++) {
+  for (let attempt = 1; attempt <= maxAttempts; attempt++) {
     try {
       await operation(attempt)
       return
     } catch (error: any) {
-      if (attempt >= maxRetries || !isTransientError(error)) {
+      if (attempt >= maxAttempts || !isTransientError(error)) {
         throw error
       }
       log(
-        `${operationName} failed (attempt ${attempt}/${maxRetries}, retrying): ${
+        `${operationName} failed (attempt ${attempt}/${maxAttempts}, retrying): ${
           error?.message ?? error
         }`,
       )

And update the call site accordingly:

   await withRetries({
-    maxRetries: 3,
-    logger,
-    logInfo,
+    maxAttempts: 3,
+    log: logInfo,
     operationName: "SMTP send",

Also applies to: 88-98

---

108-112: Consider warn level for retry-failure log messages.

Transient failures being retried are logged at info level (since logInfo is logger.info.bind(logger)). Using warn would make these easier to surface in monitoring/alerting and better reflects the semantics — something went wrong, but it's being handled.