Skip to content

Commit b7388c3

Browse files
fix(logging): use logger instead of console in stack modules (#3442)
* fix(auth): clean up stale test users before signup in integration tests 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 * fix(auth): add @returns JSDoc to purgeUser helper * fix(logging): replace console.* with Winston logger in stack modules 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 * fix(logging): address review feedback — preserve error stacks, redact 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) * fix(logging): address CodeRabbit review — full error objects, reorder 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)
1 parent 77b6994 commit b7388c3

14 files changed

Lines changed: 84 additions & 49 deletions

File tree

lib/app.js

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import nodeHttp from 'http';
66
import nodeHttps from 'https';
77

88
import config from '../config/index.js';
9+
import logger from './services/logger.js';
910
import express from './services/express.js';
1011
import mongooseService from './services/mongoose.js';
1112
import migrations from './services/migrations.js';
@@ -94,26 +95,25 @@ const logConfiguration = async () => {
9495
// Create server URL
9596
const server = `${(config.secure && config.secure.credentials ? 'https://' : 'http://') + config.api.host}:${config.api.port}`;
9697
// Logging initialization
97-
console.log(chalk.green(config.app.title));
98-
console.log();
99-
console.log(chalk.green(`Environment: ${process.env.NODE_ENV ? process.env.NODE_ENV : 'develoment'}`));
100-
console.log(chalk.green(`Server: ${server}`));
101-
console.log(chalk.green(`Database: ${config.db.uri}`));
102-
if (config.cors.origin.length > 0) console.log(chalk.green(`Cors: ${config.cors.origin}`));
98+
logger.info(chalk.green(config.app.title));
99+
logger.info(chalk.green(`Environment: ${process.env.NODE_ENV ? process.env.NODE_ENV : 'development'}`));
100+
logger.info(chalk.green(`Server: ${server}`));
101+
const safeUri = config.db.uri.replace(/\/\/[^@]+@/, '//***:***@');
102+
logger.info(chalk.green(`Database: ${safeUri}`));
103+
if (config.cors.origin.length > 0) logger.info(chalk.green(`Cors: ${config.cors.origin}`));
103104

104105
// SaaS readiness summary (skip in test to keep output clean)
105106
if (process.env.NODE_ENV !== 'test') {
106107
try {
107108
const { default: HomeService } = await import('../modules/home/services/home.service.js');
108109
const checks = HomeService.getReadinessStatus();
109-
console.log();
110-
console.log(chalk.green('SaaS Readiness:'));
110+
logger.info(chalk.green('SaaS Readiness:'));
111111
checks.forEach((c) => {
112112
const icon = c.status === 'ok' ? chalk.green('OK') : chalk.yellow('WARN');
113-
console.log(` ${icon} ${c.category.padEnd(12)} ${c.message}`);
113+
logger.info(` ${icon} ${c.category.padEnd(12)} ${c.message}`);
114114
});
115115
} catch (err) {
116-
console.log(chalk.yellow(` SaaS readiness check failed: ${err.message}`));
116+
logger.warn(chalk.yellow(` SaaS readiness check failed: ${err.message}`), err);
117117
}
118118
}
119119
};
@@ -160,7 +160,7 @@ const FORCE_SHUTDOWN_TIMEOUT_MS = 5000;
160160
const shutdown = async (server) => {
161161
// Force exit if graceful shutdown hangs
162162
const forceTimeout = setTimeout(() => {
163-
console.error(chalk.red('Forced shutdown (timeout)'));
163+
logger.error(chalk.red('Forced shutdown (timeout)'));
164164
process.exit(1);
165165
}, FORCE_SHUTDOWN_TIMEOUT_MS);
166166
forceTimeout.unref();
@@ -171,15 +171,16 @@ const shutdown = async (server) => {
171171
await SentryService.shutdown();
172172
await mongooseService.disconnect();
173173
value.http.close((err) => {
174-
console.info(chalk.yellow('Server closed'));
175174
if (err) {
176-
console.info(chalk.red('Error on server close.', err));
175+
logger.error(chalk.red('Error on server close.'), err);
177176
process.exitCode = 1;
177+
} else {
178+
logger.info(chalk.yellow('Server closed'));
178179
}
179180
process.exit();
180181
});
181182
} catch (err) {
182-
console.error(chalk.red('Shutdown error: server never started or shutdown failed'), err);
183+
logger.error(chalk.red('Shutdown error: server never started or shutdown failed'), err);
183184
process.exit(1);
184185
}
185186
};

lib/helpers/mailer/index.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import path from 'path';
22
import handlebars from 'handlebars';
33

44
import config from '../../../config/index.js';
5+
import logger from '../../services/logger.js';
56
import files from '../files.js';
67
import NodemailerProvider from './provider.nodemailer.js';
78
import ResendProvider from './provider.resend.js';
@@ -93,7 +94,7 @@ const sendMail = async (mail) => {
9394
if (!Array.isArray(result?.accepted)) return { ...result, accepted: [mail.to], rejected: [] };
9495
return result;
9596
} catch (err) {
96-
console.error(`Mail send error: ${err.message}`);
97+
logger.error('Mail send error', err);
9798
return null;
9899
}
99100
};

lib/helpers/mailer/tests/mailer.unit.tests.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@ jest.unstable_mockModule('../provider.nodemailer.js', () => ({
2929
default: jest.fn().mockImplementation(() => ({ send: jest.fn() })),
3030
}));
3131

32+
jest.unstable_mockModule('../../../services/logger.js', () => ({
33+
default: {
34+
error: jest.fn(),
35+
warn: jest.fn(),
36+
info: jest.fn(),
37+
debug: jest.fn(),
38+
},
39+
}));
40+
3241
const { default: mailer } = await import('../index.js');
3342

3443
describe('mailer index with resend provider unit tests:', () => {

lib/services/express.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,12 @@ const initPreParserRoutes = async (app) => {
117117
try {
118118
const route = await import(path.resolve(routePath));
119119
if (typeof route.default !== 'function') {
120-
console.warn(`Pre-parser route ${routePath} does not export a default function`);
120+
logger.warn(`Pre-parser route ${routePath} does not export a default function`);
121121
continue;
122122
}
123123
route.default(app);
124124
} catch (err) {
125-
console.error(`Failed to load pre-parser route: ${routePath}`, err);
125+
logger.error(`Failed to load pre-parser route: ${routePath}`, err);
126126
throw err;
127127
}
128128
}
@@ -237,7 +237,7 @@ const initModulesServerRoutes = async (app) => {
237237
const initErrorRoutes = (app) => {
238238
app.use((err, req, res, next) => {
239239
if (!err) return next();
240-
console.error(err.stack);
240+
logger.error(err.stack);
241241
res.status(err.status || 500).send({
242242
message: err.message,
243243
code: err.code,

lib/services/migrations.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import chalk from 'chalk';
55
import mongoose from 'mongoose';
66
import path from 'path';
77
import { glob } from 'glob';
8+
import logger from './logger.js';
89

910
/**
1011
* Scan all modules for migration files matching `modules/*/migrations/*.js`.
@@ -89,7 +90,7 @@ const runMigration = async (filePath, executed) => {
8990
// Atomically claim the migration to prevent concurrent execution
9091
const claimed = await claimMigration(name);
9192
if (!claimed) {
92-
console.log(chalk.yellow(` Migration already claimed by another runner: ${name}`));
93+
logger.warn(chalk.yellow(` Migration already claimed by another runner: ${name}`));
9394
return false;
9495
}
9596

@@ -115,7 +116,7 @@ const runMigration = async (filePath, executed) => {
115116
throw err;
116117
}
117118

118-
console.log(chalk.green(` Migration executed: ${name}`));
119+
logger.info(chalk.green(` Migration executed: ${name}`));
119120
return true;
120121
};
121122

@@ -133,30 +134,29 @@ const run = async () => {
133134
const files = await discoverMigrationFiles();
134135

135136
if (files.length === 0) {
136-
console.log(chalk.yellow('No migration files found.'));
137+
logger.warn(chalk.yellow('No migration files found.'));
137138
return { total: 0, executed: 0 };
138139
}
139140

140141
const executed = await getExecutedMigrations();
141142
let executedCount = 0;
142143

143-
console.log(chalk.yellow(`Running migrations (${files.length} found, ${executed.size} already executed)...`));
144+
logger.info(chalk.yellow(`Running migrations (${files.length} found, ${executed.size} already executed)...`));
144145

145146
for (const filePath of files) {
146147
try {
147148
const wasRun = await runMigration(filePath, executed);
148149
if (wasRun) executedCount++;
149150
} catch (err) {
150-
console.error(chalk.red(`Migration failed: ${path.basename(filePath)}`));
151-
console.error(chalk.red(err.message));
151+
logger.error(chalk.red(`Migration failed: ${path.basename(filePath)}`), err);
152152
throw err;
153153
}
154154
}
155155

156156
if (executedCount > 0) {
157-
console.log(chalk.green(`Migrations complete: ${executedCount} executed.`));
157+
logger.info(chalk.green(`Migrations complete: ${executedCount} executed.`));
158158
} else {
159-
console.log(chalk.yellow('All migrations already up to date.'));
159+
logger.info(chalk.yellow('All migrations already up to date.'));
160160
}
161161

162162
return { total: files.length, executed: executedCount };

lib/services/mongoose.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import chalk from 'chalk';
66
import mongoose from 'mongoose';
77
import path from 'path';
88
import config from '../../config/index.js';
9+
import logger from './logger.js';
910

1011
/**
1112
* Load all mongoose related models
@@ -38,13 +39,13 @@ const connect = async () => {
3839

3940
await mongoose.connect(config.db.uri, mongoOptions);
4041
mongoose.set('debug', config.db.debug);
41-
console.info(chalk.yellow('Connected to MongoDB.'));
42+
logger.info(chalk.yellow('Connected to MongoDB.'));
4243

4344
return mongoose;
4445
} catch (err) {
4546
// Log Error
46-
console.error(chalk.red('Could not connect to MongoDB!'));
47-
console.log(err);
47+
logger.error(chalk.red('Could not connect to MongoDB!'));
48+
logger.error(err);
4849
throw err;
4950
}
5051
};
@@ -54,7 +55,7 @@ const connect = async () => {
5455
*/
5556
const disconnect = async () => {
5657
await mongoose.disconnect();
57-
console.info(chalk.yellow('Disconnected from MongoDB.'));
58+
logger.info(chalk.yellow('Disconnected from MongoDB.'));
5859
};
5960

6061
export default {

lib/services/seed.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import _ from 'lodash';
55
import chalk from 'chalk';
66

77
import config from '../../config/index.js';
8+
import logger from './logger.js';
89

910
import AppError from '../helpers/AppError.js';
1011

@@ -19,10 +20,10 @@ const seedTheUser = (UserService, user) => async (password) => {
1920
if (process.env.NODE_ENV === 'test' && (await UserService.get(user))) UserService.remove(user);
2021
try {
2122
const result = await UserService.create(user);
22-
if (seedOptions.logResults) console.log(chalk.bold.blue(`Database Seeding: Local ${user.email} added with password set to ${password}`));
23+
if (seedOptions.logResults) logger.info(chalk.bold.blue(`Database Seeding: Local ${user.email} added with password set to ***`));
2324
return result;
2425
} catch (err) {
25-
console.log(err);
26+
logger.error(err);
2627
throw new AppError('Failed to seedTheUser.', { code: 'LIB_ERROR' });
2728
}
2829
};
@@ -31,10 +32,10 @@ const seedTheUser = (UserService, user) => async (password) => {
3132
const seedTasks = async (TaskService, task, user) => {
3233
try {
3334
const result = await TaskService.create(task, user);
34-
if (seedOptions.logResults) console.log(chalk.bold.blue(`Database Seeding: Local ${task.title} added`));
35+
if (seedOptions.logResults) logger.info(chalk.bold.blue(`Database Seeding: Local ${task.title} added`));
3536
return result;
3637
} catch (err) {
37-
console.log(err);
38+
logger.error(err);
3839
throw new AppError('Failed to seedTasks.', { code: 'LIB_ERROR' });
3940
}
4041
};
@@ -66,7 +67,7 @@ const start = async (options, UserService, AuthService, TaskService) => {
6667
}
6768
}
6869
} catch (err) {
69-
console.log(err);
70+
logger.error(err);
7071
return new AppError('Error on seed start.');
7172
}
7273

@@ -84,7 +85,7 @@ const user = async (user, UserService, AuthService) => {
8485
pwd = await AuthService.generateRandomPassphrase();
8586
result.push(await seedTheUser(UserService, user)(pwd));
8687
} catch (err) {
87-
console.log(err);
88+
logger.error(err);
8889
return new AppError('Error on seed start.');
8990
}
9091

lib/services/sentry.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Module dependencies
33
*/
44
import config from '../../config/index.js';
5+
import logger from './logger.js';
56

67
/**
78
* Sentry client reference (null when not configured).
@@ -32,7 +33,7 @@ const init = async () => {
3233
tracesSampleRate: process.env.NODE_ENV === 'production' ? 0.1 : 1.0,
3334
});
3435
} catch (err) {
35-
console.error('Sentry init failed:', err.message);
36+
logger.error('Sentry init failed:', err);
3637
Sentry = null;
3738
}
3839
};

lib/services/tests/sentry.unit.tests.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@ jest.unstable_mockModule('../../../config/index.js', () => ({
1010
},
1111
}));
1212

13+
jest.unstable_mockModule('../logger.js', () => ({
14+
default: {
15+
error: jest.fn(),
16+
warn: jest.fn(),
17+
info: jest.fn(),
18+
debug: jest.fn(),
19+
},
20+
}));
21+
1322
describe('SentryService unit tests:', () => {
1423
let SentryService;
1524

modules/audit/audit.init.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Module dependencies
33
*/
44
import config from '../../config/index.js';
5+
import logger from '../../lib/services/logger.js';
56
import auditMiddleware from './middlewares/audit.middleware.js';
67

78
/**
@@ -22,6 +23,6 @@ export default async (app) => {
2223

2324
if (process.env.NODE_ENV !== 'test') {
2425
const enabled = config.audit?.enabled ?? false;
25-
console.log(`Audit module: ${enabled ? 'enabled' : 'disabled'}`);
26+
logger.info(`Audit module: ${enabled ? 'enabled' : 'disabled'}`);
2627
}
2728
};

0 commit comments

Comments
 (0)