fix(logging): use logger instead of console in stack modules#3442
Merged
PierreBrisorgueil merged 5 commits intomasterfrom Apr 9, 2026
Merged
fix(logging): use logger instead of console in stack modules#3442PierreBrisorgueil merged 5 commits intomasterfrom
PierreBrisorgueil merged 5 commits intomasterfrom
Conversation
Auth integration tests used hardcoded emails without cleaning the database before each run. On shared MongoDB instances (e.g. ARC self-hosted runners), stale data from previous CI runs caused cascading test failures. Add pre-signup cleanup in all beforeAll/beforeEach hooks across the four auth integration test files to purge any leftover users by email before attempting signup. Closes #3437
Replace console.error/log/warn/info with the Winston-based logger from lib/services/logger.js across all production stack modules. This ensures structured logging, Sentry capture, and consistent log formatting. Files excluded from this change: - config/index.js and lib/helpers/config.js (loaded before logger init) - lib/services/logger.js (is the logger itself) - scripts/seed.js (CLI script) - Test files (except where assertions needed updating) Closes #3434
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 29 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe changes systematically replace all Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (45.45%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #3442 +/- ##
==========================================
+ Coverage 85.51% 85.54% +0.02%
==========================================
Files 113 113
Lines 2879 2878 -1
Branches 795 795
==========================================
Hits 2462 2462
+ Misses 331 330 -1
Partials 86 86 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
Replaces direct console.* usage with the centralized Winston logger across core runtime/services to standardize logging behavior, and updates affected tests to assert against logger calls.
Changes:
- Swap
console.log/info/warn/errorforlogger.*in server bootstrap and stack services (app init, mongoose, migrations, seed, express error paths, mailer, sentry, audit, billing webhook). - Update unit/integration tests to mock/spy on
loggerinstead ofconsole. - Add stale-test-data cleanup in multiple auth integration suites to reduce flakiness on shared databases.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| server.js | Uses logger for startup/shutdown logging. |
| lib/app.js | Uses logger for startup configuration/readiness logging and shutdown logs. |
| lib/services/mongoose.js | Logs connect/disconnect and connection failures via logger. |
| lib/services/migrations.js | Logs migration progress/errors via logger. |
| lib/services/seed.js | Logs seed progress/errors via logger. |
| lib/services/express.js | Logs route-loading and error middleware output via logger. |
| lib/services/sentry.js | Logs Sentry init failures via logger. |
| lib/helpers/mailer/index.js | Logs mail send failures via logger. |
| modules/billing/controllers/billing.webhook.controller.js | Logs webhook handler failures via logger. |
| modules/audit/audit.init.js | Logs module enablement via logger. |
| lib/services/tests/sentry.unit.tests.js | Mocks logger for Sentry service tests. |
| lib/helpers/mailer/tests/mailer.unit.tests.js | Mocks logger for mailer tests. |
| modules/billing/tests/billing.controller.unit.tests.js | Mocks logger and asserts logger.error calls. |
| modules/core/tests/core.integration.tests.js | Spies on logger.info to verify seed logging. |
| modules/auth/tests/auth.signup.organization.integration.tests.js | Adds helper to purge stale users before signup org tests. |
| modules/auth/tests/auth.integration.tests.js | Adds stale-user cleanup in multiple auth flows. |
| modules/auth/tests/auth.authorization.integration.tests.js | Adds stale-user cleanup before authz tests. |
| modules/auth/tests/auth.abilities.integration.tests.js | Adds stale-user cleanup before abilities tests. |
… secrets - Log full Error objects instead of just .message (server.js, mailer, sentry) - Remove JSON.stringify wrapping on error messages (server.js) - Fix 'develoment' typo → 'development' (lib/app.js) - Redact DB URI credentials before logging (lib/app.js) - Redact seed passwords from log output (lib/services/seed.js)
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/app.js`:
- Around line 174-177: The success log is emitted unconditionally before error
handling; modify the server close callback so that
logger.info(chalk.yellow('Server closed')) is only called when there is no
error: keep the existing error branch (logger.error(...), process.exitCode = 1)
and move the success log into the else path (or after a conditional check of err
=== null) to avoid logging success when an error occurred; update the close
callback where logger and process.exitCode are used.
- Line 116: The readiness-check warning currently logs only err.message via
logger.warn(chalk.yellow(...)) which loses stack and structured fields; update
the readiness-check logging in lib/app.js to pass the full error object (or
err.stack) to logger.warn alongside the formatted message so the stack and
metadata are preserved — locate the logger.warn(...) call in the
readiness-check/initialization code and change it to include the err as an
additional argument (or include err.stack in the message) so diagnostics retain
full error details.
In `@lib/services/migrations.js`:
- Around line 151-152: The migration failure log currently only emits
err.message which omits stack/context; update the error logging in
lib/services/migrations.js (the logger calls around Migration failed and
err.message) to log the full Error object in the same entry — e.g., pass the
Error object as an additional argument or include it in the metadata so the
logger records stack and context (replace the separate
logger.error(chalk.red(err.message)) with a single logger.error invocation that
includes the Error object).
In `@lib/services/mongoose.js`:
- Around line 47-48: Combine the two separate logger.error calls into a single
call so the context message and error object are logged together; specifically
update the error handling in the MongoDB connection code (where logger and chalk
are used, e.g., the connection failure block in lib/services/mongoose.js) to
call logger.error with the combined message and the err object (preserve
chalk.red formatting on the message) so the log entry contains both the
descriptive text and the full error/stack trace.
In `@server.js`:
- Around line 12-13: The logger.error call is currently passing the error
message and the error object separately (logger.error(e.message, e)), causing
redundant output; update the call to pass a single contextualized argument such
as a prefixed error object or a single string with the error (e.g.,
logger.error("Server failed:", e) or logger.error(e) ), so replace the current
logger.error(e.message, e) usage in server.js with a single, non-redundant
logger.error invocation that includes context and the error object (refer to
logger.error and the caught error variable e).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9039c906-c8df-4082-9208-7db37f8ba83b
📒 Files selected for processing (18)
lib/app.jslib/helpers/mailer/index.jslib/helpers/mailer/tests/mailer.unit.tests.jslib/services/express.jslib/services/migrations.jslib/services/mongoose.jslib/services/seed.jslib/services/sentry.jslib/services/tests/sentry.unit.tests.jsmodules/audit/audit.init.jsmodules/auth/tests/auth.abilities.integration.tests.jsmodules/auth/tests/auth.authorization.integration.tests.jsmodules/auth/tests/auth.integration.tests.jsmodules/auth/tests/auth.signup.organization.integration.tests.jsmodules/billing/controllers/billing.webhook.controller.jsmodules/billing/tests/billing.controller.unit.tests.jsmodules/core/tests/core.integration.tests.jsserver.js
… close log - Include error object in readiness-check warn (lib/app.js) - Move success log after error check in server close callback (lib/app.js) - Log full error object for migration failures (lib/services/migrations.js)
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
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.
Summary
console.error/console.log/console.warn/console.infowith the Winston-basedloggerfromlib/services/logger.jsacross all production stack modulesScope
Changed (10 production files):
server.js,lib/app.js,lib/services/mongoose.js,lib/services/migrations.js,lib/services/seed.js,lib/services/express.js,lib/services/sentry.js,lib/helpers/mailer/index.js,modules/billing/controllers/billing.webhook.controller.js,modules/audit/audit.init.jsUpdated tests (4 files):
lib/services/tests/sentry.unit.tests.js,lib/helpers/mailer/tests/mailer.unit.tests.js,modules/billing/tests/billing.controller.unit.tests.js,modules/core/tests/core.integration.tests.jsIntentionally excluded (bootstrap/config layer):
config/index.js,lib/helpers/config.js— loaded before logger is initialized (circular dependency)lib/services/logger.js— is the logger itself, must use consolescripts/seed.js— CLI script, console is appropriateconsole.log(err)in catch blocks is acceptable for test debuggingTest plan
npm run lintpassesnpm run test:coverage— 64 suites, 756 tests, all greenCloses #3434
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores