Conversation
amazon ses integration
Development
Development
Development
template for mass mailing
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds concurrency-limited parallel email sending using p-limit (limit 12), replacing sequential processing; preserves per-row validation, result schema and summary computation. Adds new HTML email templates: Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Route as SendEmails Route
participant Parser as CSV/HTML Parser
participant Limiter as p-limit (max 12)
participant SES as AWS SES
participant Agg as Result Aggregator
Client->>Route: POST /SendEmails (CSV + template)
Route->>Parser: parse CSV, validate required fields
Parser-->>Route: rows + template
Note over Route,Limiter: Serial -> concurrent scheduling (limit 12)
loop per row (max 12 concurrent)
Route->>Limiter: schedule task
Limiter->>Route: run task
Route->>Route: validate row, replace placeholders
Route->>SES: SendEmailCommand
alt success
SES-->>Route: messageId
Route->>Agg: push {status: success, messageId, ...}
else error
SES-->>Route: error
Route->>Route: compute mapped errorMessage (not used)
Route->>Agg: push {status: error, message: raw error.message}
end
end
Route->>Route: compute counts, statusCode, sandbox note
Route-->>Client: JSON {results[], summary}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
beacon/app/api/SendEmails/route.js (1)
92-108: Fix required-column check (you’re validating first row values, not header presence).If the first row has empty Name/Email, you 400 even when columns exist. Check header keys instead.
- if (!firstRow.Name || !firstRow.Email) { - return NextResponse.json( - { - success: false, - message: 'Excel file must contain "Name" and "Email" columns', - availableColumns: availableColumns, - hasName: !!firstRow.Name, - hasEmail: !!firstRow.Email - }, - { status: 400 } - ); - } + const hasNameCol = availableColumns.some(c => c.trim().toLowerCase() === 'name'); + const hasEmailCol = availableColumns.some(c => c.trim().toLowerCase() === 'email'); + if (!hasNameCol || !hasEmailCol) { + return NextResponse.json( + { + success: false, + message: 'Excel file must contain "Name" and "Email" columns', + availableColumns, + hasName: hasNameCol, + hasEmail: hasEmailCol + }, + { status: 400 } + ); + }
🧹 Nitpick comments (3)
beacon/app/api/SendEmails/route.js (3)
113-115: Preserve input order in results; avoid push from concurrent tasks.Concurrent pushes make order nondeterministic. Index into a pre-sized array.
- const results = []; + const results = new Array(data.length);- results.push({ + results[idx] = { email: row.Email || 'Unknown', name: row.Name || 'Unknown', status: 'skipped', message: 'Missing email or name' - }); + };- results.push({ + results[idx] = { email: row.Email, name: row.Name, status: 'success', message: 'Email sent successfully', messageId: response.MessageId - }); + };- results.push({ + results[idx] = { email: row.Email, name: row.Name, status: 'error', - message: error.message || 'Unknown error occurred' - }); + message: errorMessage + };Also applies to: 216-223, 244-251, 262-267
311-314: Make awsNote detection robust (case‑insensitive and based on error results).Brittle substring match may miss variants; also constrain to error entries.
- awsNote: errorCount > 0 && results.some(r => r.message && r.message.includes('not verified')) + awsNote: errorCount > 0 && results.some( + r => r?.status === 'error' && typeof r?.message === 'string' && /not verified/i.test(r.message) + ) ? 'AWS SES is in sandbox mode. Only verified email addresses can receive emails. Please verify recipient emails in AWS SES Console or request production access.' : null
208-271: Add basic retry/backoff for transient SES errors (Throttling/5xx).Reduce flakiness under load with bounded retries + jitter.
Apply minimal call-site change:
- const response = await sesClient.send(command); + const response = await withRetry(() => sesClient.send(command));Add this helper near the top of the file (outside the handler):
// Simple bounded retry with exponential backoff + jitter for transient errors const withRetry = async (fn, { retries = 3, baseMs = 250 } = {}) => { let err; for (let attempt = 0; attempt <= retries; attempt++) { try { return await fn(); } catch (e) { const code = String(e?.name || e?.Code || '').toLowerCase(); const msg = String(e?.message || '').toLowerCase(); const retriable = code.includes('throttl') || code.includes('timeout') || code.includes('temporar') || (e?.$metadata?.httpStatusCode >= 500); if (!retriable || attempt === retries) { err = e; break; } const sleep = baseMs * 2 ** attempt + Math.floor(Math.random() * 100); await new Promise(r => setTimeout(r, sleep)); } } throw err; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
beacon/app/api/SendEmails/route.js(2 hunks)
🔇 Additional comments (1)
beacon/app/api/SendEmails/route.js (1)
117-117: Make concurrency configurable; don't hardcode "12".SES send rates vary by account/region — use an env var (validate >0) with a sane default and confirm your account's MaxSendRate via
aws ses get-send-quota(MaxSendRate) or the SES console.- const limit = pLimit(12); // 12 concurrent sends (SES default rate) + const configured = Number(process.env.EMAIL_SEND_CONCURRENCY ?? 5); + const concurrency = Number.isFinite(configured) && configured > 0 ? configured : 5; + const limit = pLimit(concurrency);
| import * as XLSX from 'xlsx'; | ||
| import { promises as fs } from 'fs'; | ||
| import path from 'path'; | ||
| import pLimit from 'p-limit'; |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm p-limit is declared and compatible with ESM/Next runtime.
Ensure dependency is present (v4+ is ESM) and not duplicated via subdeps.
🏁 Script executed:
#!/bin/bash
# Verify dependency and version
rg -n '"p-limit"' package.json
jq -r '.dependencies["p-limit"] // .devDependencies["p-limit"]' package.jsonLength of output: 123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "---- find package.json files ----"
find . -name package.json -print || true
echo
echo "---- search for occurrences of 'p-limit' ----"
rg -n --hidden -S "p-limit" || true
echo
echo "---- show beacon/app/api/SendEmails/route.js (if present) ----"
if [ -f "beacon/app/api/SendEmails/route.js" ]; then
sed -n '1,240p' beacon/app/api/SendEmails/route.js
else
echo "beacon/app/api/SendEmails/route.js not found"
fi
echo
echo "---- inspect any package.json found for p-limit ----"
for f in $(find . -name package.json -print); do
echo "---- $f ----"
if command -v jq >/dev/null 2>&1; then
jq -r '.name // "<no name>" as $n | ($n + " -> p-limit: " + (.dependencies["p-limit"] // .devDependencies["p-limit"] // "<not declared>"))' "$f" || cat "$f"
else
grep -n '"p-limit"' "$f" || echo "<jq missing; grep ran>"
fi
done
echo
echo "---- search common lockfiles for p-limit versions ----"
rg -n --hidden -S "p-limit" package-lock.json yarn.lock pnpm-lock.yaml pnpm-lock.json || trueLength of output: 10533
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "---- beacon/package.json (full) ----"
if [ -f "beacon/package.json" ]; then
sed -n '1,240p' beacon/package.json
else
echo "beacon/package.json not found"
fi
echo
echo "---- beacon/package-lock.json: p-limit matches (show surrounding lines) ----"
if [ -f "beacon/package-lock.json" ]; then
rg -n --hidden -S "p-limit" beacon/package-lock.json -n -C3 || true
echo
echo "---- show node_modules/p-limit block if present ----"
rg -n --hidden -S '"node_modules/p-limit":' -n -C6 beacon/package-lock.json || true
else
echo "beacon/package-lock.json not found"
fi
echo
echo "---- beacon/app/api/SendEmails/route.js (search for runtime/export) ----"
if [ -f "beacon/app/api/SendEmails/route.js" ]; then
rg -n "export const runtime|export const config|export const dynamic" beacon/app/api/SendEmails/route.js || echo "no explicit runtime/config exports found in route.js"
echo
sed -n '1,400p' beacon/app/api/SendEmails/route.js
else
echo "beacon/app/api/SendEmails/route.js not found"
fiLength of output: 16037
Declare p-limit in package.json and choose an ESM-compatible version
- beacon/app/api/SendEmails/route.js imports p-limit (line 6) but beacon/package.json does not declare it; package-lock shows node_modules/p-limit@3.1.0 as a transitive dev-only dependency (risk: omitted in production installs).
- p-limit v3 is CommonJS; v4+ is ESM-only. Action: add p-limit to beacon/package.json dependencies and pin the major you need — use ^4.x if this route may run in Edge/ESM, or ^3.x if you must remain CJS — then run install to update the lockfile.
🤖 Prompt for AI Agents
In beacon/app/api/SendEmails/route.js around line 6, p-limit is imported but not
declared in beacon/package.json and the transitive node_modules/p-limit@3.1.0 is
a dev-only dep; add p-limit to beacon/package.json "dependencies" (not
devDependencies) and pin the major you need: use "^4.x" if this route must run
in ESM/Edge environments or stay with "^3.x" if you must remain CommonJS, then
run npm/yarn install to update the lockfile and ensure it’s installed in
production installs.
| // for (const row of data) { | ||
| // try { | ||
| // processedCount++; | ||
| // console.log(`Processing email ${processedCount}/${data.length} for ${row.Email}`); | ||
|
|
||
| // // Skip rows with missing email or name | ||
| // if (!row.Email || !row.Name) { | ||
| // results.push({ | ||
| // email: row.Email || 'Unknown', | ||
| // name: row.Name || 'Unknown', | ||
| // status: 'skipped', | ||
| // message: 'Missing email or name' | ||
| // }); | ||
| // continue; | ||
| // } | ||
|
|
||
| // // Replace placeholders in template | ||
| // let personalizedTemplate = template; | ||
|
|
||
| // // Replace {{Recipient_name}} with the actual name | ||
| // personalizedTemplate = personalizedTemplate.replace(/{{Recipient_name}}/g, row.Name); | ||
|
|
||
| // // Replace any other placeholders that might exist | ||
| // Object.keys(row).forEach(key => { | ||
| // const placeholder = new RegExp(`{{${key}}}`, 'g'); | ||
| // personalizedTemplate = personalizedTemplate.replace(placeholder, row[key]); | ||
| // }); | ||
|
|
||
| // const isPlainTextOnly = false; // We always send as HTML now since rich text editor produces HTML | ||
|
|
||
| // // Create SES email parameters | ||
| // const emailParams = { | ||
| // Source: senderEmail, | ||
| // Destination: { | ||
| // ToAddresses: [row.Email], | ||
| // }, | ||
| // Message: { | ||
| // Subject: { | ||
| // Data: subject, | ||
| // Charset: 'UTF-8', | ||
| // }, | ||
| // Body: { | ||
| // Html: { | ||
| // Data: personalizedTemplate, | ||
| // Charset: 'UTF-8', | ||
| // }, | ||
| // }, | ||
| // }, | ||
| // }; | ||
|
|
||
| // // Send email | ||
| // const command = new SendEmailCommand(emailParams); | ||
| // const response = await sesClient.send(command); | ||
|
|
||
| // results.push({ | ||
| // email: row.Email, | ||
| // name: row.Name, | ||
| // status: 'success', | ||
| // message: 'Email sent successfully', | ||
| // messageId: response.MessageId | ||
| // }); | ||
|
|
||
| // // Add a small delay between emails to avoid rate limiting | ||
| // if (processedCount < data.length) { | ||
| // await new Promise(resolve => setTimeout(resolve, 100)); | ||
| // } | ||
|
|
||
| // } catch (error) { | ||
| // console.error(`Error sending email to ${row.Email}:`, error); | ||
|
|
||
| // // Provide specific error messages | ||
| // let errorMessage = error.message || 'Unknown error occurred'; | ||
| // if (error.message.includes('Email address is not verified')) { | ||
| // errorMessage = 'Email address not verified in AWS SES'; | ||
| // } else if (error.message.includes('AccessDenied')) { | ||
| // errorMessage = 'AWS SES permission denied - check IAM permissions'; | ||
| // } else if (error.message.includes('InvalidParameterValue')) { | ||
| // errorMessage = 'Invalid email format or parameters'; | ||
| // } | ||
|
|
||
| // results.push({ | ||
| // email: row.Email, | ||
| // name: row.Name, | ||
| // status: 'error', | ||
| // message: errorMessage | ||
| // }); | ||
| // } | ||
| // } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented‑out legacy loop.
Large commented blocks add noise and risk divergence.
Delete this block entirely from the PR.
🤖 Prompt for AI Agents
In beacon/app/api/SendEmails/route.js around lines 119 to 206, remove the large
commented-out legacy for-loop block (the entire commented section that processes
rows, sends SES emails, and handles errors) to eliminate dead code and noise;
ensure no dependent references remain elsewhere in the file and run
tests/linting to verify nothing breaks after deletion.
| limit(async () => { | ||
| try { | ||
| processedCount++; | ||
| console.log(`Processing email ${processedCount}/${data.length} for ${row.Email}`); |
There was a problem hiding this comment.
Don’t log full recipient emails (PII). Mask before logging.
Avoid emitting raw addresses to logs.
- console.log(`Processing email ${processedCount}/${data.length} for ${row.Email}`);
+ const maskedEmail = typeof row.Email === 'string'
+ ? row.Email.replace(/(.{2}).+(@.+)/, '$1***$2')
+ : 'Unknown';
+ console.log(`Processing email ${processedCount}/${data.length} for ${maskedEmail}`);📝 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.
| console.log(`Processing email ${processedCount}/${data.length} for ${row.Email}`); | |
| const maskedEmail = typeof row.Email === 'string' | |
| ? row.Email.replace(/(.{2}).+(@.+)/, '$1***$2') | |
| : 'Unknown'; | |
| console.log(`Processing email ${processedCount}/${data.length} for ${maskedEmail}`); |
🤖 Prompt for AI Agents
In beacon/app/api/SendEmails/route.js around line 213 the code logs full
recipient emails (PII) with console.log(`Processing email
${processedCount}/${data.length} for ${row.Email}`); — replace this with a
masked form of the email before logging (e.g., keep first character and domain,
replace intervening characters with asterisks or show only the domain and a
short hashed/tokenized prefix), and ensure no other logs emit the raw row.Email;
update the logging statement to use the masked value and add a small helper or
inline logic to perform the masking consistently.
| // Replace placeholders | ||
| let personalizedTemplate = template.replace(/{{Recipient_name}}/g, row.Name); | ||
| Object.keys(row).forEach(key => { | ||
| const placeholder = new RegExp(`{{${key}}}`, 'g'); | ||
| personalizedTemplate = personalizedTemplate.replace(placeholder, row[key]); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Escape placeholder keys and avoid inserting "undefined" into the template.
Unescaped regex keys can mis‑match; undefined values render as the string "undefined".
- let personalizedTemplate = template.replace(/{{Recipient_name}}/g, row.Name);
- Object.keys(row).forEach(key => {
- const placeholder = new RegExp(`{{${key}}}`, 'g');
- personalizedTemplate = personalizedTemplate.replace(placeholder, row[key]);
- });
+ const safe = v => (v == null ? '' : String(v));
+ let personalizedTemplate = template.replace(/{{Recipient_name}}/g, safe(row.Name));
+ for (const [key, value] of Object.entries(row)) {
+ const escaped = key.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+ personalizedTemplate = personalizedTemplate
+ .replace(new RegExp(`{{${escaped}}}`, 'g'), safe(value));
+ }📝 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.
| // Replace placeholders | |
| let personalizedTemplate = template.replace(/{{Recipient_name}}/g, row.Name); | |
| Object.keys(row).forEach(key => { | |
| const placeholder = new RegExp(`{{${key}}}`, 'g'); | |
| personalizedTemplate = personalizedTemplate.replace(placeholder, row[key]); | |
| }); | |
| // Replace placeholders | |
| const safe = v => (v == null ? '' : String(v)); | |
| let personalizedTemplate = template.replace(/{{Recipient_name}}/g, safe(row.Name)); | |
| for (const [key, value] of Object.entries(row)) { | |
| const escaped = key.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | |
| personalizedTemplate = personalizedTemplate | |
| .replace(new RegExp(`{{${escaped}}}`, 'g'), safe(value)); | |
| } |
🤖 Prompt for AI Agents
In beacon/app/api/SendEmails/route.js around lines 225 to 231, the loop builds a
RegExp from object keys without escaping special characters and blindly inserts
row[key] which can become the string "undefined"; update it to escape regex
metacharacters in the key before constructing the RegExp (so keys like "a.b" or
"name?" match literally) and only replace when row[key] is not null/undefined
(use an empty string or a safe default for falsy values), converting non-string
values to string explicitly before replacement.
| } catch (error) { | ||
| console.error(`Error sending email to ${row.Email}:`, error); | ||
| let errorMessage = error.message || 'Unknown error occurred'; | ||
| if (error.message.includes('Email address is not verified')) { | ||
| errorMessage = 'Email address not verified in AWS SES'; | ||
| } else if (error.message.includes('AccessDenied')) { | ||
| errorMessage = 'AWS SES permission denied - check IAM permissions'; | ||
| } else if (error.message.includes('InvalidParameterValue')) { | ||
| errorMessage = 'Invalid email format or parameters'; | ||
| } | ||
| results.push({ | ||
| email: row.Email, | ||
| name: row.Name, | ||
| status: 'error', | ||
| message: error.message || 'Unknown error occurred' | ||
| }); |
There was a problem hiding this comment.
Use the mapped errorMessage and guard when error.message is undefined; also normalize matching.
Current code computes errorMessage but returns raw error.message and can throw on .includes when message is absent.
- console.error(`Error sending email to ${row.Email}:`, error);
- let errorMessage = error.message || 'Unknown error occurred';
- if (error.message.includes('Email address is not verified')) {
+ console.error(`Error sending email to ${row.Email}:`, error);
+ const rawMsg = String(error?.message ?? '');
+ const msg = rawMsg.toLowerCase();
+ let errorMessage = rawMsg || 'Unknown error occurred';
+ if (msg.includes('not verified')) {
errorMessage = 'Email address not verified in AWS SES';
- } else if (error.message.includes('AccessDenied')) {
+ } else if (msg.includes('accessdenied')) {
errorMessage = 'AWS SES permission denied - check IAM permissions';
- } else if (error.message.includes('InvalidParameterValue')) {
+ } else if (msg.includes('invalidparametervalue')) {
errorMessage = 'Invalid email format or parameters';
+ } else if (msg.includes('throttl')) {
+ errorMessage = 'AWS SES rate limit exceeded - reduce concurrency or request a higher limit';
}
- results.push({
- email: row.Email,
- name: row.Name,
- status: 'error',
- message: error.message || 'Unknown error occurred'
- });
+ results[idx] = {
+ email: row.Email,
+ name: row.Name,
+ status: 'error',
+ message: errorMessage
+ };Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In beacon/app/api/SendEmails/route.js around lines 252-267, the code computes a
mapped errorMessage but still uses error.message and calls .includes on it which
can throw if undefined; fix by first extracting a safe string like const rawMsg
= (error && error.message) ? String(error.message) : '' , normalize it to
lowerCase for matching, run includes checks against the normalized string to set
errorMessage cases, then use the computed errorMessage when pushing results and
in the console.error (falling back to a generic message if empty).
There was a problem hiding this comment.
Pull Request Overview
This PR implements concurrent emailing functionality to improve bulk email sending performance by replacing sequential processing with controlled concurrency using the p-limit library.
Key changes:
- Introduces concurrent email processing with a limit of 12 simultaneous sends
- Replaces sequential for-loop with Promise.all and p-limit for controlled concurrency
- Removes artificial delays between email sends that were previously used for rate limiting
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| email: row.Email, | ||
| name: row.Name, | ||
| status: 'error', | ||
| message: error.message || 'Unknown error occurred' |
There was a problem hiding this comment.
The errorMessage variable that was constructed with specific error handling logic is being ignored. Line 266 should use errorMessage instead of error.message || 'Unknown error occurred' to preserve the enhanced error messages for AWS SES issues.
| message: error.message || 'Unknown error occurred' | |
| message: errorMessage |
| // } catch (error) { | ||
| // console.error(`Error sending email to ${row.Email}:`, error); | ||
|
|
||
| // // Provide specific error messages | ||
| // let errorMessage = error.message || 'Unknown error occurred'; | ||
| // if (error.message.includes('Email address is not verified')) { | ||
| // errorMessage = 'Email address not verified in AWS SES'; | ||
| // } else if (error.message.includes('AccessDenied')) { | ||
| // errorMessage = 'AWS SES permission denied - check IAM permissions'; | ||
| // } else if (error.message.includes('InvalidParameterValue')) { | ||
| // errorMessage = 'Invalid email format or parameters'; | ||
| // } | ||
|
|
||
| // results.push({ | ||
| // email: row.Email, | ||
| // name: row.Name, | ||
| // status: 'error', | ||
| // message: errorMessage | ||
| // }); | ||
| // } | ||
| // } | ||
|
|
There was a problem hiding this comment.
Large blocks of commented-out code should be removed instead of left in the codebase. This creates clutter and makes the code harder to maintain. The old implementation can be retrieved from version control if needed.
| // } catch (error) { | |
| // console.error(`Error sending email to ${row.Email}:`, error); | |
| // // Provide specific error messages | |
| // let errorMessage = error.message || 'Unknown error occurred'; | |
| // if (error.message.includes('Email address is not verified')) { | |
| // errorMessage = 'Email address not verified in AWS SES'; | |
| // } else if (error.message.includes('AccessDenied')) { | |
| // errorMessage = 'AWS SES permission denied - check IAM permissions'; | |
| // } else if (error.message.includes('InvalidParameterValue')) { | |
| // errorMessage = 'Invalid email format or parameters'; | |
| // } | |
| // results.push({ | |
| // email: row.Email, | |
| // name: row.Name, | |
| // status: 'error', | |
| // message: errorMessage | |
| // }); | |
| // } | |
| // } |
| processedCount++; | ||
| console.log(`Processing email ${processedCount}/${data.length} for ${row.Email}`); |
There was a problem hiding this comment.
The processedCount variable is being incremented concurrently without synchronization, which can lead to race conditions and inaccurate counts. Consider using an atomic counter or removing this logging since the concurrent processing makes the count unreliable for progress tracking.
| processedCount++; | |
| console.log(`Processing email ${processedCount}/${data.length} for ${row.Email}`); |
🤖 PR Review Agent ReportReview completed at: 2025-09-21 23:34:34 UTC 📊 Summary
Issue Severity Breakdown
AI Feedback Categories
🚨 High Priority Issues
💡 Recommendations
AI Review Confidence: 86% This review was generated automatically. Please use your judgment for final decisions. Tools used: Static analysis + AI-powered review |
🤖 PR Review Agent ReportReview completed at: 2025-09-21 23:35:56 UTC 📊 Summary
Issue Severity Breakdown
AI Feedback Categories
🚨 High Priority Issues
💡 Recommendations
AI Review Confidence: 86% This review was generated automatically. Please use your judgment for final decisions. Tools used: Static analysis + AI-powered review |
PR Review Agent ReportReview completed at: 2025-09-21 23:51:47 UTC Code Quality ScoreOverall Score: 75/100 Detailed Breakdown:
Executive SummaryThe code introduces concurrency for faster email sending, which is a positive improvement. However, there are significant security concerns related to input validation and error handling. The code also lacks comprehensive testing and could benefit from refactoring for improved maintainability. Addressing the security vulnerabilities and adding thorough testing should be prioritized. Summary
Issue Severity Breakdown
AI Feedback Categories
High Priority Issues
Recommendations
AI Review Confidence: 86% This review was generated automatically. Please use your judgment for final decisions. Tools used: Static analysis + AI-powered review |
PR Review Agent ReportReview completed at: 2025-09-21 23:52:44 UTC Code Quality ScoreOverall Score: 75/100 Detailed Breakdown:
Executive SummaryThe code introduces concurrency for faster email sending, which is a positive improvement. However, there are some critical security and maintainability concerns that need to be addressed. Input validation is missing, error handling is insufficient, and test coverage is low. Addressing these issues is crucial before deploying to production. Improving placeholder replacement efficiency and refining concurrency control will further enhance performance and maintainability. Summary
Issue Severity Breakdown
AI Feedback Categories
High Priority Issues
Recommendations
AI Review Confidence: 82% This review was generated automatically. Please use your judgment for final decisions. Tools used: Static analysis + AI-powered review |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
beacon/templates/onboarding.html (1)
137-145: Add rel="noopener noreferrer" to external links; consider templating the WhatsApp URL.Mitigates reverse‑tabnabbing and makes the link configurable across environments.
- <a href="https://chat.whatsapp.com/ITYlpxzhcM374c6jo1SAGU?mode=ems_copy_t" target="_blank" + <a href="{{whatsapp_link}}" target="_blank" rel="noopener noreferrer" style="background:linear-gradient(90deg, #16a34a 0%, #22c55e 100%); border-radius:10px; display:inline-block; line-height:48px; padding:0 24px; font-family: 'Inter', -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif; font-size:16px; color:#ffffff; font-weight:600; text-align:center; box-shadow:0 2px 4px rgba(0,0,0,0.1);"> Join Our WhatsApp Group </a> @@ - <a href="https://chat.whatsapp.com/ITYlpxzhcM374c6jo1SAGU?mode=ems_copy_t" - style="color:#1f2a44; word-break:break-all;">https://chat.whatsapp.com/ITYlpxzhcM374c6jo1SAGU?mode=ems_copy_t</a> + <a href="{{whatsapp_link}}" rel="noopener noreferrer" + style="color:#1f2a44; word-break:break-all;">{{whatsapp_link}}</a>beacon/templates/RSVP.html (2)
146-149: Parameterize event details and RSVP URL for reuse.Improves maintainability and reduces future edits.
- <strong>Date:</strong> 14th October<br> - <strong>Time:</strong> 8:30 AM - 5:00 PM<br> - <strong>Venue:</strong> iMac Lab C, Tech Park 1, SRM Institute of Science and Technology + <strong>Date:</strong> {{Event_date}}<br> + <strong>Time:</strong> {{Event_time}}<br> + <strong>Venue:</strong> {{Event_venue}} @@ - <a class="btn" href="https://luma.com/9y9irgqf" target="_blank">RSVP Now</a> + <a class="btn" href="{{RSVP_url}}" target="_blank" rel="noopener noreferrer">RSVP Now</a>Also applies to: 161-161
129-135: Add rel="noopener noreferrer" to external poster link.Prevents reverse‑tabnabbing and is a general hardening.
- <a href="https://res.cloudinary.com/doslhy0tq/image/upload/v1759901984/posterfiesta_hdxi6y.png" - target="_blank" style="display:block;"> + <a href="https://res.cloudinary.com/doslhy0tq/image/upload/v1759901984/posterfiesta_hdxi6y.png" + target="_blank" rel="noopener noreferrer" style="display:block;">beacon/templates/postpone.html (2)
176-188: Parameterize dates, venue, and timing for reuse and easier updates.Reduces hard‑coded content and future edit risk.
- <strong>Monday & Tuesday</strong><br> - September 15th & 16th, 2025 + <strong>{{New_days_label}}</strong><br> + {{New_dates}} @@ - <strong style="color: #dc2626;">today and tomorrow (11th and 12th September)</strong> + <strong style="color: #dc2626;">{{Original_days_label}} ({{Original_dates}})</strong> @@ - TP 301, TP 302 + {{Interview_venue}} @@ - From 5:20 PM + {{Interview_time}}Also applies to: 204-205, 231-233, 240-241
255-275: Confirm consent and consider centralizing contact numbers.Phone numbers are PII; ensure consent and store in config/env to avoid hard‑coding.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
beacon/public/sample-data.xlsxis excluded by!**/*.xlsx
📒 Files selected for processing (4)
beacon/templates/RSVP.html(1 hunks)beacon/templates/onboarding.html(1 hunks)beacon/templates/postpone.html(1 hunks)beacon/templates/update.html(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- beacon/templates/update.html
| <table role="presentation" aria-hidden="true" cellpadding="0" cellspacing="0" border="0" width="100%" | ||
| style=" min-height: 100vh;"> | ||
| <tr> | ||
| <td align="center" style="padding:32px 16px;"> | ||
| <table role="presentation" cellpadding="0" cellspacing="0" border="0" width="600" class="container" | ||
| style="width:600px; max-width:600px; background:#ffffff; border-radius:20px; box-shadow:0 20px 40px rgba(0,0,0,0.15); overflow: hidden;"> | ||
|
|
||
| <!-- Header Section --> | ||
| <tr> |
There was a problem hiding this comment.
Remove aria-hidden from main content table; it hides the email from screen readers.
This is an accessibility blocker.
- <table role="presentation" aria-hidden="true" cellpadding="0" cellspacing="0" border="0" width="100%"
+ <table role="presentation" cellpadding="0" cellspacing="0" border="0" width="100%"
style=" min-height: 100vh;">📝 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.
| <table role="presentation" aria-hidden="true" cellpadding="0" cellspacing="0" border="0" width="100%" | |
| style=" min-height: 100vh;"> | |
| <tr> | |
| <td align="center" style="padding:32px 16px;"> | |
| <table role="presentation" cellpadding="0" cellspacing="0" border="0" width="600" class="container" | |
| style="width:600px; max-width:600px; background:#ffffff; border-radius:20px; box-shadow:0 20px 40px rgba(0,0,0,0.15); overflow: hidden;"> | |
| <!-- Header Section --> | |
| <tr> | |
| <table role="presentation" cellpadding="0" cellspacing="0" border="0" width="100%" | |
| style=" min-height: 100vh;"> | |
| <tr> | |
| <td align="center" style="padding:32px 16px;"> | |
| <table role="presentation" cellpadding="0" cellspacing="0" border="0" width="600" class="container" | |
| style="width:600px; max-width:600px; background:#ffffff; border-radius:20px; box-shadow:0 20px 40px rgba(0,0,0,0.15); overflow: hidden;"> | |
| <!-- Header Section --> | |
| <tr> |
🤖 Prompt for AI Agents
In beacon/templates/postpone.html around lines 138 to 146 the main content table
has aria-hidden="true", which hides the email content from screen readers;
remove the aria-hidden attribute (or set it to false/not present) on this
primary table so assistive technologies can access the content, ensuring the
attribute is only used on purely decorative elements if needed.
| style="margin:0 0 24px 0; font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial; font-size:18px; line-height:1.6; color:#374151;"> | ||
| Dear {{Recipient_name}}, | ||
| </p> |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure Recipient_name is HTML‑escaped at injection time.
Avoid raw replacement; sanitize/escape before inserting.
Use the same verification script provided in RSVP.html to confirm escaping in the SendEmails route.
#!/bin/bash
echo "Locating RSVP.html template"
find . -type f -iname "RSVP.html"
echo
echo "Showing first 200 lines of RSVP.html"
find . -type f -iname "RSVP.html" -exec sed -n '1,200p' {} \;
echo
echo "Searching for 'escape' or filter usage in RSVP.html"
find . -type f -iname "RSVP.html" -exec rg -n -C3 "escape" {} \;
echo
echo "Finding SendEmails route in code"
rg -n -i "SendEmails" -C3 .#!/bin/bash
# Locate RSVP.html template
RSVP=$(find . -type f -path '*/templates/RSVP.html' | head -n1)
echo "RSVP.html path: $RSVP"
# Print first 200 lines of RSVP.html
[ -n "$RSVP" ] && sed -n '1,200p' "$RSVP"
# Search for Recipient_name usage and escaping in RSVP.html
[ -n "$RSVP" ] && rg -n -C2 "{{\s*Recipient_name" "$RSVP"
[ -n "$RSVP" ] && rg -n "|e\>" "$RSVP"
# Locate SendEmails route definition and rendering calls
rg -n "SendEmails" -C3 .
rg -n "render_template" -C3 beacon/🏁 Script executed:
#!/usr/bin/env bash
# 1. Locate RSVP.html template
RSVP=$(find . -type f -path "*beacon/templates/RSVP.html" | head -n1)
echo "RSVP.html path: $RSVP"
# 2. Show lines around Recipient_name usage
if [ -f "$RSVP" ]; then
echo "Occurrences of Recipient_name in RSVP.html:"
rg -n -C2 "{{\s*Recipient_name" "$RSVP"
echo "Escaping/filter patterns in RSVP.html:"
rg -n -C2 "\|e\>" "$RSVP"
else
echo "RSVP.html not found"
fi
# 3. Locate SendEmails route and rendering calls
echo "Searching for SendEmails handler:"
rg -n "SendEmails" -C3 .
echo "Searching for render_template usage in beacon/:"
rg -n "render_template" -C3 beacon/Length of output: 3012
#!/bin/bash
# Show context around Recipient_name in RSVP.html
echo "RSVP.html snippet:"
rg -n "Recipient_name" -C3 beacon/templates/RSVP.html || echo "No matches"
echo
# Show beginning of SendEmails route.js
echo "SendEmails route.js header:"
sed -n '1,200p' beacon/app/api/SendEmails/route.js || echo "Cannot open route.js"
echo
# Search for template placeholder replacements in route.js
echo "Searching for .replace calls in route.js:"
rg -n "replace" -C3 beacon/app/api/SendEmails/route.js || echo "No replace calls found"🏁 Script executed:
#!/usr/bin/env bash
echo "Locating RSVP.html template..."
RSVP=$(find beacon/templates -type f -iname "RSVP.html" | head -n1)
echo "RSVP.html path: $RSVP"
echo
if [ -f "$RSVP" ]; then
echo "Lines 150-200 around Recipient_name in RSVP.html:"
sed -n '150,200p' "$RSVP"
echo
echo "Occurrences of Recipient_name:"
grep -n '{{Recipient_name' "$RSVP"
else
echo "RSVP.html not found"
fi
echo
echo "Inspecting SendEmails route handler..."
ROUTE="beacon/app/api/SendEmails/route.js"
echo "Route file: $ROUTE"
echo
if [ -f "$ROUTE" ]; then
echo "First 200 lines of route.js:"
sed -n '1,200p' "$ROUTE"
echo
echo "Searching for placeholder replacement calls:"
grep -n '.replace' "$ROUTE"
else
echo "route.js not found"
fiLength of output: 9952
Escape Recipient_name before template substitution
In beacon/app/api/SendEmails/route.js (around line 226), wrap row.Name with an HTML-escape utility (e.g. escapeHtml(row.Name)) before calling .replace(/{{Recipient_name}}/g, …) to prevent HTML injection.
🤖 Prompt for AI Agents
In beacon/templates/postpone.html around lines 167 to 169, the template inserts
{{Recipient_name}} without escaping which allows HTML injection; before calling
.replace(/{{Recipient_name}}/g, …) in beacon/app/api/SendEmails/route.js (around
line 226) ensure you HTML-escape the source value (e.g. use existing escapeHtml
utility or add one) and pass escapeHtml(row.Name) into the replace call,
importing/require-ing the utility if needed and using the escaped string for all
substitutions.
| <p class="greet">Hello {{Recipient_name}},</p> | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Ensure Recipient_name is HTML‑escaped at injection time.
If server does raw string replace, this is XSS‑prone. Escape/sanitize before substitution.
Run to verify escaping/sanitization in SendEmails route:
🏁 Script executed:
#!/bin/bash
# Look for placeholder replacement and any escaping/sanitization usage.
rg -n -C2 -g '!**/dist/**' -g '!**/build/**' -P 'Recipient_name|sanitize|escape|dompurify|xss|he\(|escapeHtml|validator\.escape'
rg -n -C3 -P 'replace\(\s*["\']\{\{Recipient_name\}\}["\']'
rg -n -C3 -P 'SendEmails/route\.js|send.*SES|p-limit'Length of output: 11296
Escape Recipient_name during template substitution
In beacon/app/api/SendEmails/route.js:226, you’re replacing {{Recipient_name}} with raw row.Name, which permits XSS. Apply HTML escaping (e.g., he.encode(row.Name)) or sanitize the value before insertion.
🤖 Prompt for AI Agents
In beacon/templates/RSVP.html around lines 138-139, the template inserts
{{Recipient_name}} without escaping which allows XSS when route.js replaces it
with raw row.Name (see route.js line 226); fix by HTML-escaping or sanitizing
the name before substitution (e.g., use a library like he.encode or a
sanitizer), import/require that encoder in beacon/app/api/SendEmails/route.js,
replace the raw insertion with the escaped value (and provide a safe fallback if
Name is missing), and add/update tests to assert that HTML in names is rendered
escaped.
Summary by CodeRabbit
New Features
Refactor