fix(email): replace console logs with structured logger#30
Merged
deekshithgowda85 merged 1 commit intoMay 28, 2026
Merged
Conversation
|
@amna-sehgal is attempting to deploy a commit to the Deekshith Gowda HS's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe email service logging is refactored to replace direct console calls with a dedicated structured JSON logger. The new logger writes timestamped entries to stdout and accepts ChangesEmail logging infrastructure and integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🤖 Prompt for all review comments with 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.
Inline comments:
In `@lib/email/mail-service.ts`:
- Around line 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.
- Around line 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.
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+31
to
+47
| /** 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"); | ||
| } |
There was a problem hiding this comment.
🛠️ 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.
Comment on lines
+125
to
+129
| log("error", "Brevo rejected email (non-retryable)", { | ||
| status: res.status, | ||
| body: errBody, | ||
| to | ||
| }); |
There was a problem hiding this comment.
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.
Comment on lines
+151
to
+155
| log("error", "Email send failed after retries", { | ||
| to, | ||
| subject, | ||
| error: lastError | ||
| }); |
There was a problem hiding this comment.
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.
deekshithgowda85
added
gssoc:approved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
This PR replaces all direct
console.log/console.errorusage in the email service with a structured logging helper.It ensures consistent log formatting across the email module and prevents potential sensitive data leakage by standardizing logging behavior.
Related Issue
Closes: #17
What Changed
console.logstatements inlib/email/mail-service.tslog()helper for consistent logginginfo,warn,error)Screenshots or Demo
Not applicable (backend/service-level change)
Verification
pnpm lintconsole.*usage remains inlib/email/mail-service.tsChecklist
Summary by CodeRabbit