-
-
Notifications
You must be signed in to change notification settings - Fork 300
feat: add campaign smtp-proxy #332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@lamalex is attempting to deploy a commit to the kmkoushik's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
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. WalkthroughThe pull request adds three package.json entries ( Pre-merge checks✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/smtp-server/src/server.ts">
<violation number="1" location="apps/smtp-server/src/server.ts:106">
P2: `errorData` is already a string from `response.text()`. Calling `JSON.stringify` on it won't pretty-print JSON - it will just escape and quote the string. Either log the string directly, or parse it as JSON first if you expect JSON responses.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this 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 (2)
apps/smtp-server/src/server.ts (2)
93-100: Consider adding a timeout to prevent indefinite hangs.The
fetchcall has no timeout configured. If the API becomes unresponsive, this could block the SMTP server indefinitely.🔎 Proposed fix using AbortController
async function sendCampaignToUseSend( campaignData: CampaignData, apiKey: string, ): Promise<CampaignResponse> { try { const createEndpoint = "/api/v1/campaigns"; const createUrl = new URL(createEndpoint, BASE_URL); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); const payload = { name: campaignData.name, from: campaignData.from, subject: campaignData.subject, contactBookId: campaignData.contactBookId, html: campaignData.html, replyTo: campaignData.replyTo, sendNow: true, }; const response = await fetch(createUrl.href, { method: "POST", headers: { Authorization: `Bearer ${apiKey}`, "Content-Type": "application/json", }, body: JSON.stringify(payload), + signal: controller.signal, }); + clearTimeout(timeoutId);The same pattern should be applied to
sendEmailToUseSendfor consistency.
243-246: Variablehtmlshadows the function parameter.The local variable
htmlon line 243 shadows thehtmlparameter from line 214. While not a functional bug (the parameter is unused at this point), it reduces code clarity.🔎 Proposed fix - rename local variable
- const html = document.querySelector("html"); - if (html) { - html.appendChild(footer); + const htmlElement = document.querySelector("html"); + if (htmlElement) { + htmlElement.appendChild(footer); } else {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.devenv/state/minio/data/.minio.sys/tmp/7b08971e-4836-4772-8f90-aa29588a6ef2apps/smtp-server/package.jsonapps/smtp-server/src/server.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/smtp-server/src/server.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/smtp-server/src/server.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Run Prettier 3 for code formatting on TypeScript, TSX, and Markdown files
Files:
apps/smtp-server/src/server.ts
⏰ 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 (7)
apps/smtp-server/src/server.ts (6)
6-7: LGTM - Imports and configuration follow established patterns.Top-level imports and environment variable fallback are consistent with the existing codebase conventions.
Also applies to: 20-21
22-55: LGTM - Recipient parsing logic is sound.The function correctly differentiates between contact book IDs (addresses at the campaign domain) and regular email addresses. The regex handles both bare emails and angle-bracket wrapped formats.
168-176: LGTM - Proper HTML escaping prevents XSS.Using
he.encodeto escape user content before embedding in HTML is the correct approach for security.
178-205: LGTM - Footer creation and detection are well implemented.Good approach using inline styles for email client compatibility, and checking for both
{{unsend_unsubscribe_url}}and{{usesend_unsubscribe_url}}maintains backward compatibility.
299-316: Early return on campaign validation blocks individual emails.When an email has mixed recipients (both campaign and individual addresses), validation failures for campaigns (missing from, subject, or content) cause an early return, preventing individual emails from being sent.
If this is intentional (all-or-nothing semantics), consider adding a comment to clarify. If individual emails should still be sent when campaign validation fails, the logic needs restructuring.
366-380: LGTM - Parallel execution with proper error propagation.Using
Promise.allfor parallel sending is efficient, and the fail-fast behavior aligns with SMTP semantics where a message should be fully accepted or rejected.apps/smtp-server/package.json (1)
18-19: LGTM - Dependencies correctly added.The runtime dependencies
heandlinkedomare appropriately placed independencies, and@types/heis correctly placed indevDependencies.Note: The pre-existing
@types/mailparserand@types/smtp-serveron lines 15-16 should ideally be indevDependenciessince type definitions are only needed at build time. Consider moving them in a follow-up.
|
@coderabbitai generate docstrings |
|
Warning Docstrings generation is disabled for your repository or organization. |
787442b to
ea40781
Compare
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 13360 @lamalex I'll generate comprehensive docstrings for all the interfaces and functions in import { SMTPServer, SMTPServerOptions, SMTPServerSession } from "smtp-server";
import { Readable } from "stream";
import dotenv from "dotenv";
import { simpleParser } from "mailparser";
import { readFileSync, watch, FSWatcher } from "fs";
import he from "he";
import { parseHTML } from "linkedom";
dotenv.config();
const AUTH_USERNAME = process.env.SMTP_AUTH_USERNAME ?? "usesend";
const BASE_URL =
process.env.USESEND_BASE_URL ??
process.env.UNSEND_BASE_URL ??
"https://app.usesend.com";
const SSL_KEY_PATH =
process.env.USESEND_API_KEY_PATH ?? process.env.UNSEND_API_KEY_PATH;
const SSL_CERT_PATH =
process.env.USESEND_API_CERT_PATH ?? process.env.UNSEND_API_CERT_PATH;
const CAMPAIGN_DOMAIN = process.env.USESEND_CAMPAIGN_DOMAIN ?? "usesend.com";
/**
* Represents the categorized recipients from an email's "To" field.
* Recipients are split into contact book IDs and individual email addresses.
*/
interface ParsedRecipients {
/** Contact book IDs extracted from addresses matching the campaign domain */
contactBookIds: string[];
/** Individual email addresses for direct email delivery */
emailAddresses: string[];
}
/**
* Parses all recipients from the "To" field and categorizes them.
* - Addresses like "[email protected]" (or configured domain) are contact book IDs
* - All other addresses are treated as individual email recipients
*
* @param to - The raw "To" field string from the email header
* @returns An object containing arrays of contact book IDs and email addresses
*
* @example
* ```typescript
* const recipients = parseRecipients("[email protected], [email protected]");
* // Returns: { contactBookIds: ["abc123"], emailAddresses: ["user@example.com"] }
* ```
*/
function parseRecipients(to: string | undefined): ParsedRecipients {
const result: ParsedRecipients = {
contactBookIds: [],
emailAddresses: [],
};
if (!to) return result;
const emailRegex = /<?([^<>\s,]+@[^<>\s,]+)>?/g;
let match;
while ((match = emailRegex.exec(to)) !== null) {
const email = match[1].toLowerCase();
const [localPart, domain] = email.split("@");
if (domain === CAMPAIGN_DOMAIN.toLowerCase() && localPart) {
result.contactBookIds.push(localPart);
} else {
result.emailAddresses.push(email);
}
}
return result;
}
/**
* Represents the data required to create and send a campaign via the UseSend API.
*/
interface CampaignData {
/** Display name for the campaign */
name: string;
/** Sender email address */
from: string;
/** Email subject line */
subject: string;
/** ID of the contact book to target */
contactBookId: string;
/** HTML content of the email (must include unsubscribe link) */
html: string;
/** Optional reply-to email address */
replyTo?: string;
}
/**
* Represents the response from the UseSend campaign creation API.
*/
interface CampaignResponse {
/** Unique identifier for the created campaign */
id: string;
/** Name of the campaign */
name: string;
/** Current status of the campaign (e.g., "scheduled", "sending") */
status: string;
}
/**
* Creates a campaign and schedules it for immediate sending via the UseSend API.
* The campaign is created with sendNow=true for instant delivery.
*
* @param campaignData - The campaign details including recipients, content, and metadata
* @param apiKey - UseSend API authentication token
* @returns A promise resolving to the campaign response with ID and status
* @throws {Error} If the API request fails or returns a non-OK status
*
* @example
* ```typescript
* const campaign = await sendCampaignToUseSend({
* name: "Newsletter",
* from: "sender@example.com",
* subject: "Monthly Update",
* contactBookId: "abc123",
* html: "<html>...</html>"
* }, "api_key_here");
* ```
*/
async function sendCampaignToUseSend(
campaignData: CampaignData,
apiKey: string,
): Promise<CampaignResponse> {
try {
const createEndpoint = "/api/v1/campaigns";
const createUrl = new URL(createEndpoint, BASE_URL);
const payload = {
name: campaignData.name,
from: campaignData.from,
subject: campaignData.subject,
contactBookId: campaignData.contactBookId,
html: campaignData.html,
replyTo: campaignData.replyTo,
sendNow: true,
};
const response = await fetch(createUrl.href, {
method: "POST",
headers: {
Authorization: `Bearer ${apiKey}`,
"Content-Type": "application/json",
},
body: JSON.stringify(payload),
});
if (!response.ok) {
const errorData = await response.text();
console.error(
"useSend Campaign API error response:",
JSON.stringify(errorData, null, 4),
);
throw new Error(
`Failed to create campaign: ${errorData || "Unknown error from server"}`,
);
}
const responseData = (await response.json()) as CampaignResponse;
return responseData;
} catch (error) {
if (error instanceof Error) {
console.error("Campaign error message:", error.message);
throw new Error(`Failed to send campaign: ${error.message}`);
} else {
console.error("Unexpected campaign error:", error);
throw new Error("Failed to send campaign: Unexpected error occurred");
}
}
}
/**
* Sends an individual email (non-campaign) via the UseSend API.
* Supports sending to multiple recipients in a single API call.
*
* @param emailData - Email data including to, from, subject, and content fields
* @param apiKey - UseSend API authentication token
* @throws {Error} If the API request fails or returns a non-OK status
*
* @example
* ```typescript
* await sendEmailToUseSend({
* to: ["recipient@example.com"],
* from: "[email protected]",
* subject: "Hello",
* html: "<p>Hello World</p>"
* }, "api_key_here");
* ```
*/
async function sendEmailToUseSend(emailData: any, apiKey: string) {
try {
const apiEndpoint = "/api/v1/emails";
const url = new URL(apiEndpoint, BASE_URL); // Combine base URL with endpoint
console.log("Sending email to useSend API at:", url.href); // Debug statement
const emailDataText = JSON.stringify(emailData);
const response = await fetch(url.href, {
method: "POST",
headers: {
Authorization: `Bearer ${apiKey}`,
"Content-Type": "application/json",
},
body: emailDataText,
});
if (!response.ok) {
const errorData = await response.text();
console.error(
"useSend API error response: error:",
JSON.stringify(errorData, null, 4),
`\nemail data: ${emailDataText}`,
);
throw new Error(
`Failed to send email: ${errorData || "Unknown error from server"}`,
);
}
const responseData = await response.json();
console.log("useSend API response:", responseData);
} catch (error) {
if (error instanceof Error) {
console.error("Error message:", error.message);
throw new Error(`Failed to send email: ${error.message}`);
} else {
console.error("Unexpected error:", error);
throw new Error("Failed to send email: Unexpected error occurred");
}
}
}
/**
* Converts plain text to a basic HTML document.
* Escapes special HTML characters and converts newlines to <br> tags.
*
* @param text - Plain text content to convert
* @returns Valid HTML document string with escaped content
*
* @example
* ```typescript
* const html = textToHtml("Hello\nWorld");
* // Returns: "<!DOCTYPE html><html><body><p>Hello<br>\nWorld</p></body></html>"
* ```
*/
function textToHtml(text: string): string {
const escapedText = he.encode(text, { useNamedReferences: true });
// Convert newlines to <br> tags
const htmlText = escapedText.replace(/\n/g, "<br>\n");
return `<!DOCTYPE html><html><body><p>${htmlText}</p></body></html>`;
}
/**
* Creates an unsubscribe footer element with styled link.
* The footer includes the {{usesend_unsubscribe_url}} placeholder for template substitution.
*
* @param document - DOM document instance for creating elements
* @returns HTMLElement containing the formatted unsubscribe footer
*/
function createUnsubscribeFooter(document: Document): HTMLElement {
const footer = document.createElement("p");
footer.setAttribute(
"style",
"margin-top: 20px; padding-top: 20px; border-top: 1px solid #eee; font-size: 12px; color: #666;",
);
const link = document.createElement("a");
link.setAttribute("href", "{{usesend_unsubscribe_url}}");
link.setAttribute("style", "color: #666;");
link.textContent = "Unsubscribe";
footer.appendChild(link);
return footer;
}
/**
* Checks if the HTML document contains an unsubscribe link placeholder.
* Supports both legacy {{unsend_unsubscribe_url}} and current {{usesend_unsubscribe_url}} placeholders.
*
* @param html - HTML content string to check
* @returns True if an unsubscribe placeholder is found, false otherwise
*/
function hasUnsubscribeLink(html: string): boolean {
return (
html.includes("{{unsend_unsubscribe_url}}") ||
html.includes("{{usesend_unsubscribe_url}}")
);
}
/**
* Prepares HTML content for campaign sending by ensuring proper format and unsubscribe compliance.
* - Converts plain text to HTML if no HTML is provided
* - Automatically appends an unsubscribe footer if one is not already present
* - Uses linkedom for safe DOM manipulation
*
* @param html - HTML content from the email, or false/undefined if not available
* @param text - Plain text content from the email, used as fallback
* @returns Prepared HTML string with unsubscribe link, or null if no content is available
*
* @example
* ```typescript
* const html = prepareCampaignHtml(false, "Hello World");
* // Returns HTML with text content and appended unsubscribe footer
* ```
*/
function prepareCampaignHtml(
html: string | false | undefined,
text: string | undefined,
): string | null {
// Convert plain text to HTML if no HTML provided
let htmlContent: string;
if (!html && text) {
htmlContent = textToHtml(text);
} else if (html) {
htmlContent = html;
} else {
return null;
}
// Check if unsubscribe link already exists
if (hasUnsubscribeLink(htmlContent)) {
return htmlContent;
}
// Parse the HTML and add the unsubscribe footer using DOM APIs
const { document } = parseHTML(htmlContent);
const footer = createUnsubscribeFooter(document);
// Append to body if it exists, otherwise append to document
const body = document.querySelector("body");
if (body) {
body.appendChild(footer);
} else {
// No body tag - wrap content and add footer
const html = document.querySelector("html");
if (html) {
html.appendChild(footer);
} else {
// Minimal HTML - just append
document.appendChild(footer);
}
}
return document.toString();
}
/**
* Loads TLS certificate and key files from disk.
* File paths are determined by SSL_KEY_PATH and SSL_CERT_PATH environment variables.
*
* @returns Object containing optional key and cert Buffers
* @throws May throw if file reading fails (error is caught by caller)
*/
function loadCertificates(): { key?: Buffer; cert?: Buffer } {
return {
key: SSL_KEY_PATH ? readFileSync(SSL_KEY_PATH) : undefined,
cert: SSL_CERT_PATH ? readFileSync(SSL_CERT_PATH) : undefined,
};
}
const initialCerts = loadCertificates();
const serverOptions: SMTPServerOptions = {
secure: false,
key: initialCerts.key,
cert: initialCerts.cert,
onData(
stream: Readable,
session: SMTPServerSession,
callback: (error?: Error) => void,
) {
console.log("Receiving email data..."); // Debug statement
simpleParser(stream, async (err, parsed) => {
if (err) {
console.error("Failed to parse email data:", err.message);
return callback(err);
}
if (!session.user) {
console.error("No API key found in session");
return callback(new Error("No API key found in session"));
}
const toAddress = Array.isArray(parsed.to)
? parsed.to.map((addr) => addr.text).join(", ")
: parsed.to?.text;
const fromAddress = Array.isArray(parsed.from)
? parsed.from.map((addr) => addr.text).join(", ")
: parsed.from?.text;
const sendPromises: Promise<any>[] = [];
const recipients = parseRecipients(toAddress);
const hasCampaigns = recipients.contactBookIds.length > 0;
const hasIndividualEmails = recipients.emailAddresses.length > 0;
// Handle campaign sends (one campaign per contact book)
if (hasCampaigns) {
if (!fromAddress) {
console.error("No from address found for campaign");
return callback(new Error("From address is required for campaigns"));
}
if (!parsed.subject) {
console.error("No subject found for campaign");
return callback(new Error("Subject is required for campaigns"));
}
const htmlContent = prepareCampaignHtml(parsed.html, parsed.text);
if (!htmlContent) {
console.error("No content found for campaign");
return callback(
new Error("HTML or text content is required for campaigns"),
);
}
for (const contactBookId of recipients.contactBookIds) {
const campaignData: CampaignData = {
name: `SMTP Campaign: ${parsed.subject}`,
from: fromAddress,
subject: parsed.subject,
contactBookId,
html: htmlContent,
replyTo: parsed.replyTo?.text,
};
const campaignPromise = sendCampaignToUseSend(
campaignData,
session.user,
).catch((error) => {
console.error(
`Failed to send campaign to ${contactBookId}:`,
error.message,
);
throw error;
});
sendPromises.push(campaignPromise);
}
}
// Handle individual email sends
if (hasIndividualEmails) {
// Send to all individual recipients in one API call
const emailObject = {
to: recipients.emailAddresses,
from: fromAddress,
subject: parsed.subject,
text: parsed.text,
html: parsed.html,
replyTo: parsed.replyTo?.text,
};
const emailPromise = sendEmailToUseSend(
emailObject,
session.user,
).catch((error) => {
console.error("Failed to send individual emails:", error.message);
throw error;
});
sendPromises.push(emailPromise);
}
if (sendPromises.length === 0) {
console.error("No valid recipients found");
return callback(new Error("No valid recipients found"));
}
try {
await Promise.all(sendPromises);
callback();
} catch (error) {
if (error instanceof Error) {
callback(error);
} else {
callback(new Error("One or more sends failed"));
}
}
});
},
onAuth(auth, session: any, callback: (error?: Error, user?: any) => void) {
if (auth.username === AUTH_USERNAME && auth.password) {
console.log("Authenticated successfully"); // Debug statement
callback(undefined, { user: auth.password });
} else {
console.error("Invalid username or password");
callback(new Error("Invalid username or password"));
}
},
size: 10485760,
};
/**
* Starts multiple SMTP server instances on different ports.
* - Ports 465, 2465: Implicit SSL/TLS (if certificates are configured)
* - Ports 25, 587, 2587: STARTTLS
* Sets up file watchers to automatically reload TLS certificates when they change.
*
* @returns Object containing arrays of active servers and file watchers
*/
function startServers() {
const servers: SMTPServer[] = [];
const watchers: FSWatcher[] = [];
if (SSL_KEY_PATH && SSL_CERT_PATH) {
// Implicit SSL/TLS for ports 465 and 2465
[465, 2465].forEach((port) => {
const server = new SMTPServer({ ...serverOptions, secure: true });
server.listen(port, () => {
console.log(
`Implicit SSL/TLS SMTP server is listening on port ${port}`,
);
});
server.on("error", (err) => {
console.error(`Error occurred on port ${port}:`, err);
});
servers.push(server);
});
}
// STARTTLS for ports 25, 587, and 2587
[25, 587, 2587].forEach((port) => {
const server = new SMTPServer(serverOptions);
server.listen(port, () => {
console.log(`STARTTLS SMTP server is listening on port ${port}`);
});
server.on("error", (err) => {
console.error(`Error occurred on port ${port}:`, err);
});
servers.push(server);
});
if (SSL_KEY_PATH && SSL_CERT_PATH) {
const reloadCertificates = () => {
try {
const { key, cert } = loadCertificates();
if (key && cert) {
servers.forEach((srv) => srv.updateSecureContext({ key, cert }));
console.log("TLS certificates reloaded");
}
} catch (err) {
console.error("Failed to reload TLS certificates", err);
}
};
[SSL_KEY_PATH, SSL_CERT_PATH].forEach((file) => {
watchers.push(watch(file, { persistent: false }, reloadCertificates));
});
}
return { servers, watchers };
}
const { servers, watchers } = startServers();
/**
* Gracefully shuts down all SMTP servers and file watchers.
* Triggered by SIGINT, SIGTERM, or SIGQUIT signals.
* Exits the process after cleanup is complete.
*/
function shutdown() {
console.log("Shutting down SMTP server...");
watchers.forEach((w) => w.close());
servers.forEach((s) => s.close());
process.exit(0);
}
["SIGINT", "SIGTERM", "SIGQUIT"].forEach((signal) => {
process.on(signal, shutdown);
});I've added comprehensive TSDoc-style docstrings for all interfaces and functions, including:
The documentation covers edge cases, behavioral details, and includes examples to help developers understand how to use each function effectively. |
ea40781 to
cacca95
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/smtp-server/src/server.ts (3)
40-52: Email regex may not handle all RFC-compliant addresses.The regex
/<?([^<>\s,]+@[^<>\s,]+)>?/gis simplistic and won't handle edge cases like quoted local parts, comments, or internationalized domain names. For typical SMTP usage this may be sufficient, but consider using a more robust email parsing library if edge cases become an issue.
254-256: Rename local variable to avoid shadowing.The local variable
htmlshadows the function parameter of the same name. While the parameter has already been consumed (stored inhtmlContent), this shadowing can cause confusion during code review and maintenance.🔎 Proposed refactor
} else { // No body tag - wrap content and add footer - const html = document.querySelector("html"); - if (html) { - html.appendChild(footer); + const htmlElement = document.querySelector("html"); + if (htmlElement) { + htmlElement.appendChild(footer); } else {
285-392: Consider promisifying simpleParser for cleaner async flow.The current implementation mixes callback-style
simpleParserwith async/await, which works but is not idiomatic. Consider wrappingsimpleParserin a Promise for cleaner async code.🔎 Example promisification approach
// Add helper function async function parseEmail(stream: Readable): Promise<ParsedMail> { return new Promise((resolve, reject) => { simpleParser(stream, (err, parsed) => { if (err) reject(err); else resolve(parsed); }); }); } // Then in onData: onData(stream, session, callback) { (async () => { try { const parsed = await parseEmail(stream); // ... rest of logic callback(); } catch (error) { callback(error instanceof Error ? error : new Error("Parse failed")); } })(); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.devenv/state/minio/data/.minio.sys/tmp/7b08971e-4836-4772-8f90-aa29588a6ef2apps/smtp-server/package.jsonapps/smtp-server/src/server.tstemp
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/smtp-server/package.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/smtp-server/src/server.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/smtp-server/src/server.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Run Prettier 3 for code formatting on TypeScript, TSX, and Markdown files
Files:
apps/smtp-server/src/server.ts
🧬 Code graph analysis (1)
apps/smtp-server/src/server.ts (1)
apps/web/src/server/public-api/schemas/campaign-schema.ts (1)
CampaignResponse(99-99)
🔇 Additional comments (6)
apps/smtp-server/src/server.ts (6)
6-7: LGTM! Dependencies properly imported.The new dependencies
heandlinkedomare correctly imported at the top level for HTML encoding and DOM manipulation.
102-115: LGTM! Error handling properly improved.The error response handling now correctly attempts to parse JSON before pretty-printing, with a fallback to raw text. This addresses the previous review feedback appropriately.
148-155: LGTM! Consistent error handling.The error response handling pattern matches the campaign sending function, providing consistent behavior across both API calls.
179-216: LGTM! HTML utilities properly implemented.The utility functions correctly handle:
- HTML escaping with
he.encodeto prevent XSS- Proper DOM manipulation for footer creation
- Backward compatibility by checking for both
unsend_andusesend_placeholders
309-352: LGTM! Comprehensive validation and error handling.The campaign sending logic properly validates required fields (from, subject, content) before attempting to create campaigns. Error handling with contextual logging ensures failures are traceable.
354-391: LGTM! Parallel execution properly implemented.The parallel execution of campaigns and individual emails using
Promise.allis correctly implemented with appropriate error handling. Each send path has its own error context, and failures are properly propagated to the SMTP callback.
| interface CampaignResponse { | ||
| id: string; | ||
| name: string; | ||
| status: string; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Import CampaignResponse from the shared schema file.
The CampaignResponse interface is duplicated from apps/web/src/server/public-api/schemas/campaign-schema.ts. Import the type from the shared location to ensure consistency and prevent type drift.
🔎 Proposed refactor
+import type { CampaignResponse } from "../../web/src/server/public-api/schemas/campaign-schema";
+
-interface CampaignResponse {
- id: string;
- name: string;
- status: string;
-}Note: Adjust the import path based on the actual workspace structure (e.g., if using TypeScript path aliases or monorepo references).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/smtp-server/src/server.ts around lines 66 to 70, the CampaignResponse
interface is duplicated; remove this local declaration and import the
CampaignResponse type from the shared schema (the canonical file at
apps/web/src/server/public-api/schemas/campaign-schema.ts or the workspace's
shared path). Replace the local interface with an import statement, update any
references to use the imported type, and verify TypeScript paths/tsconfig
references or monorepo aliases are correct so the import resolves.
There was a problem hiding this 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 (5)
apps/smtp-server/src/server.ts (5)
40-52: Consider usingmatchAllfor cleaner iteration.The while loop with
regex.execis correct but could be simplified usingmatchAll(ES2020+) for improved readability.🔎 Proposed refactor
- const emailRegex = /<?([^<>\s,]+@[^<>\s,]+)>?/g; - let match; - - while ((match = emailRegex.exec(to)) !== null) { - const email = match[1].toLowerCase(); + const emailRegex = /<?([^<>\s,]+@[^<>\s,]+)>?/g; + + for (const match of to.matchAll(emailRegex)) { + const email = match[1]!.toLowerCase(); const [localPart, domain] = email.split("@"); if (domain === CAMPAIGN_DOMAIN.toLowerCase() && localPart) {
137-137: Consider defining an interface foremailData.The
emailDataparameter usesanytype. Defining an explicit interface would improve type safety and IDE support.🔎 Proposed refactor
+interface EmailData { + to: string | string[]; + from: string; + subject?: string; + text?: string; + html?: string | false; + replyTo?: string; +} + -async function sendEmailToUseSend(emailData: any, apiKey: string) { +async function sendEmailToUseSend(emailData: EmailData, apiKey: string) {
288-290: Variable shadowing:htmlredeclared.Line 288 declares
const htmlwhich shadows the function parameter. While the parameter has already been consumed, this is confusing and could lead to maintenance issues.🔎 Proposed fix
- const html = document.querySelector("html"); - if (html) { - html.appendChild(footer); + const htmlElement = document.querySelector("html"); + if (htmlElement) { + htmlElement.appendChild(footer);
283-295: Improve fallback logic for minimal HTML.The fallback path (lines 287-295) when there's no
<body>tag could produce unexpected results. Appending directly todocument(line 293) might not generate valid HTML. Consider creating a body element or wrapping the content more explicitly.🔎 Proposed improvement
// Append to body if it exists, otherwise append to document const body = document.querySelector("body"); if (body) { body.appendChild(footer); } else { - // No body tag - wrap content and add footer - const html = document.querySelector("html"); - if (html) { - html.appendChild(footer); - } else { - // Minimal HTML - just append - document.appendChild(footer); - } + // No body tag - create one and move existing content + const newBody = document.createElement("body"); + const htmlElement = document.querySelector("html"); + if (htmlElement) { + // Move existing children to body + while (htmlElement.firstChild) { + newBody.appendChild(htmlElement.firstChild); + } + newBody.appendChild(footer); + htmlElement.appendChild(newBody); + } else { + // No html tag either - wrap everything + newBody.appendChild(footer); + document.appendChild(newBody); + } }
416-425: ConsiderPromise.allSettledfor partial success handling.Currently,
Promise.allis used (line 417), which means if any campaign or email send fails, the entire operation fails. This is a valid fail-fast approach, but usingPromise.allSettledwould allow successful sends to complete while reporting which specific sends failed.🔎 Proposed enhancement
try { - await Promise.all(sendPromises); - callback(); + const results = await Promise.allSettled(sendPromises); + const failures = results.filter((r) => r.status === "rejected"); + + if (failures.length > 0) { + const errorMessages = failures + .map((f) => (f as PromiseRejectedResult).reason.message) + .join("; "); + callback(new Error(`${failures.length} send(s) failed: ${errorMessages}`)); + } else { + callback(); + } } catch (error) { if (error instanceof Error) { callback(error);Note: This allows partial success, which may or may not be desired depending on requirements.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.devenv/state/minio/data/.minio.sys/tmp/7b08971e-4836-4772-8f90-aa29588a6ef2apps/smtp-server/package.jsonapps/smtp-server/src/server.tstemp
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/smtp-server/package.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/smtp-server/src/server.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/smtp-server/src/server.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Run Prettier 3 for code formatting on TypeScript, TSX, and Markdown files
Files:
apps/smtp-server/src/server.ts
🧬 Code graph analysis (1)
apps/smtp-server/src/server.ts (1)
apps/web/src/server/public-api/schemas/campaign-schema.ts (1)
CampaignResponse(99-99)
Adds capability when sending an email to @usesend.com to create a campaign to that contact book id.
Summary by cubic
Adds an SMTP proxy that turns emails sent to @usesend.com into campaigns that send immediately. Regular email addresses continue to send as individual emails.
New Features
Dependencies
Written for commit cacca95. Summary will update on new commits.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.