Skip to content

fix(email): replace console logs with structured logger #30

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.

Sign up for GitHub

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

Merged
deekshithgowda85 merged 1 commit into from
May 28, 2026
Merged

fix(email): replace console logs with structured logger #30

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 29 additions & 13 deletions lib/email/mail-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,27 @@ const RETRY_DELAY_MS = 1500;
function sleep(ms: number): Promise<void> {
return new Promise((r) => setTimeout(r, ms));
}

/** Basic email format validation (RFC 5322 simplified). */
function isValidEmail(email: string): boolean {
return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
}

function log(level: "info" | "warn" | "error", message: string, meta?: Record<string, unknown>) {
/** Basic email format validation (RFC 5322 simplified). */
function log(
level: "info" | "warn" | "error",
message: string,
meta?: Record<string, unknown>
) {
const entry = {
ts: new Date().toISOString(),
service: "email",
level,
message,
...meta,
};
if (level === "error") console.error(JSON.stringify(entry));
else if (level === "warn") console.warn(JSON.stringify(entry));
else console.log(JSON.stringify(entry));
}

// structured logging hook (safe replacement)
process.stdout.write(JSON.stringify(entry) + "\n");
}
Comment on lines +31 to +47
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 28, 2026

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Harden log() so metadata cannot corrupt schema or break send flow.

...meta can overwrite core fields, and JSON.stringify(entry) can throw. Logging should be fail-safe and keep a stable schema.

Proposed fix 
 function log(
   level: "info" | "warn" | "error",
   message: string,
   meta?: Record<string, unknown>
 ) {
-  const entry = {
-    ts: new Date().toISOString(),
-    service: "email",
-    level,
-    message,
-    ...meta,
-  };
-
-  // structured logging hook (safe replacement)
-  process.stdout.write(JSON.stringify(entry) + "\n");
+  const entry = {
+    ts: new Date().toISOString(),
+    service: "email",
+    level,
+    message,
+    meta: meta ?? {},
+  };
+  try {
+    process.stdout.write(JSON.stringify(entry) + "\n");
+  } catch {
+    process.stdout.write(
+      JSON.stringify({
+        ts: new Date().toISOString(),
+        service: "email",
+        level: "error",
+        message: "Failed to serialize log entry",
+      }) + "\n"
+    );
+  }
 }
🤖 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 `@lib/email/mail-service.ts` around lines 31 - 47, The current log() lets
caller meta overwrite core fields and may throw from JSON.stringify(entry), so
harden it by sanitizing meta and making stringify fail-safe: inside function
log, create a sanitizedMeta from meta that strips/ignores reserved keys
("ts","service","level","message") and coerces values to primitives/strings (or
shallow clones only), build entry = { ts: new Date().toISOString(), service:
"email", level, message, ...sanitizedMeta }, then wrap JSON.stringify(entry) in
try/catch and on error write a minimal fallback JSON line (e.g., containing ts,
service, level, message and a safe "meta_error" or stringified meta) before
calling process.stdout.write to ensure logging never throws; refer to log,
entry, meta, process.stdout.write, and JSON.stringify when implementing.

/* ── rate limiter (in-memory, per-process) ─────────────────────────────── */

const rateBucket = { count: 0, resetAt: 0 };
Expand Down Expand Up @@ -82,7 +84,7 @@ export async function sendEmail(to: string, subject: string, html: string): Prom

// Rate-limit check
if (!checkRateLimit()) {
log("warn", "Email rate limit exceeded", { to, subject });
log("warn", "Email rate limit exceeded", { to });
return { ok: false, error: "Rate limit exceeded — try again shortly" };
}

Expand Down Expand Up @@ -113,29 +115,43 @@ export async function sendEmail(to: string, subject: string, html: string): Prom

if (res.ok) {
const data = (await res.json()) as { messageId?: string };
log("info", "Email sent", { to, subject, messageId: data.messageId, attempt });
log("info", "Email sent", { to, subject });
return { ok: true, messageId: data.messageId };
}

// Non-retryable client errors (4xx except 429)
if (res.status >= 400 && res.status < 500 && res.status !== 429) {
const errBody = await res.text();
log("error", "Brevo rejected email (non-retryable)", { status: res.status, body: errBody, to });
log("error", "Brevo rejected email (non-retryable)", {
status: res.status,
body: errBody,
to
});
Comment on lines +125 to +129
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid logging raw Brevo error bodies.

body: errBody can leak provider payload details and is unbounded. Prefer a sanitized/truncated field.

Proposed fix 
         log("error", "Brevo rejected email (non-retryable)", {
           status: res.status,
-          body: errBody,
+          bodySnippet: errBody.slice(0, 500),
           to
         });
🤖 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 `@lib/email/mail-service.ts` around lines 125 - 129, The current log call in
mail-service.ts exposes the full Brevo response body (errBody) which may leak
provider payloads and be arbitrarily large; replace body: errBody with a
sanitized, bounded snippet—e.g., create a helper like
sanitizeAndTruncateError(errBody) or getSafeErrorSnippet(errBody) and pass that
value (alongside res.status and to) to log("error", "Brevo rejected email
(non-retryable)", {...}); ensure the helper strips sensitive fields (tokens,
PII) and truncates to a safe max length before logging.

return { ok: false, error: `Brevo error ${res.status}` };
}

lastError = `HTTP ${res.status}`;
log("warn", `Email attempt ${attempt}/${MAX_RETRIES} failed`, { status: res.status, to });
log("warn", `Email attempt ${attempt}/${MAX_RETRIES} failed`, {
status: res.status,
to
});
} catch (err) {
lastError = err instanceof Error ? err.message : String(err);
log("warn", `Email attempt ${attempt}/${MAX_RETRIES} threw`, { error: lastError, to });
log("warn", `Email attempt ${attempt}/${MAX_RETRIES} threw`, {
error: lastError,
to
});
}

if (attempt < MAX_RETRIES) {
await sleep(RETRY_DELAY_MS * attempt);
}
}

log("error", "Email send failed after retries", { to, subject, error: lastError });
log("error", "Email send failed after retries", {
to,
subject,
error: lastError
});
Comment on lines +151 to +155
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Final failure log still includes subject, which reintroduces sensitive logging.

This conflicts with the stated goal to omit subject from final-failure logging metadata.

Proposed fix 
   log("error", "Email send failed after retries", {
     to,
-    subject,
     error: lastError
   });
📝 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.

Suggested change
log("error", "Email send failed after retries", {
to,
subject,
error: lastError
});
log("error", "Email send failed after retries", {
to,
error: lastError
});
🤖 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 `@lib/email/mail-service.ts` around lines 151 - 155, The final failure log in
mail-service.ts currently includes the sensitive `subject` field; update the log
call in the retry/final-error path (the call to log("error", "Email send failed
after retries", { to, subject, error: lastError })) to omit or redact `subject`
so only non-sensitive metadata is logged (e.g., keep `to` and `error: lastError`
or replace `subject` with a redacted placeholder). Modify the object passed to
the `log` function accordingly and ensure any related tests/assertions reference
the new sanitized metadata.

return { ok: false, error: `Failed after ${MAX_RETRIES} retries: ${lastError}` };
}
Loading