feat: introduce pino structured logger#48
Conversation
- Add pino and pino-pretty as dependencies - Create server/lib/logger.ts with pino (pretty printing in dev, JSON in production, configurable LOG_LEVEL) - Replace console.error in server/deploy.ts with logger.error (audit log write failures) - Replace console.log/error in server/lib/send-magic-link-email.ts with logger.info/error (dev magic link fallback + Resend errors)
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughAdds ChangesStructured Logging via Pino
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 docstrings
🧪 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 |
|
React Doctor found no issues. 🎉 Reviewed by React Doctor for commit |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces structured logging by adding pino and pino-pretty dependencies and replacing standard console logging calls with a centralized logger utility in server/deploy.ts and server/lib/send-magic-link-email.ts. The reviewer noted that the logger should be configured to be silent during tests to avoid flooding the test output with verbose logs, and provided a code suggestion to handle the "test" environment.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const isProduction = process.env.NODE_ENV === "production"; | ||
|
|
||
| export const logger = pino({ | ||
| level: process.env.LOG_LEVEL ?? (isProduction ? "info" : "debug"), | ||
| ...(isProduction | ||
| ? {} | ||
| : { | ||
| transport: { | ||
| target: "pino-pretty", | ||
| options: { colorize: true }, | ||
| }, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
The pull request description states that the logger should be "silent in tests". However, the current implementation only checks isProduction and defaults to "debug" with pino-pretty enabled when NODE_ENV is not "production" (which includes "test"). This will flood the test output with verbose logs.
We should explicitly check if the environment is "test" and default the log level to "silent" while disabling the pretty transport.
| const isProduction = process.env.NODE_ENV === "production"; | |
| export const logger = pino({ | |
| level: process.env.LOG_LEVEL ?? (isProduction ? "info" : "debug"), | |
| ...(isProduction | |
| ? {} | |
| : { | |
| transport: { | |
| target: "pino-pretty", | |
| options: { colorize: true }, | |
| }, | |
| }), | |
| }); | |
| const isProduction = process.env.NODE_ENV === "production"; | |
| const isTest = process.env.NODE_ENV === "test"; | |
| export const logger = pino({ | |
| level: process.env.LOG_LEVEL ?? (isTest ? "silent" : isProduction ? "info" : "debug"), | |
| ...(isProduction || isTest | |
| ? {} | |
| : { | |
| transport: { | |
| target: "pino-pretty", | |
| options: { colorize: true }, | |
| }, | |
| }), | |
| }); |
|
cc @jellydn — requesting a quick sign-off on the logger API surface before this lands. PR #47 stacks on top of this and imports API contract (
|
1 participant
Introduces structured server-side logging with
pino, replacing ad-hocconsole.log/console.errorcalls.What
server/lib/logger.ts— single configurable logger (env-drivenLOG_LEVEL,pino-prettyin dev, silent in tests).server/deploy.tsand the magic-link sender, the earliest sites where unstructured logs hurt debugging.pino+pino-prettytopackage.json+bun.lock.Why split
Logger is the foundation for the rest of #47 (the proxy 502 observability fix and the
logHandlerFailurehelper both depend onserver/lib/logger.ts). Splitting it out as the bottom of the stack keeps the diff for #47 focused on file splits, DB cleanup, and the helper — not "introduce logger + first consumer + first helper" all at once.Test plan
bun run typecheck→ 0 errorsbun run test→ all tests passSummary by CodeRabbit