From 82d1004ecfc94e1217facf5096ace17f782674d6 Mon Sep 17 00:00:00 2001
From: "Lin, Hung-Shin" <wancat@wancat.cc>
Date: Mon, 20 Nov 2023 14:59:14 +0800
Subject: [PATCH] wip: removing console log

---
 .eslintrc.js                                 |  2 +-
 docker-compose.yml                           |  1 +
 server/src/api/controllers/authController.js |  3 ++-
 server/src/api/socket.js                     |  7 ++++---
 server/src/auth/passport.js                  |  3 ++-
 server/src/cli/build.js                      | 11 ++++++-----
 server/src/cli/clean.js                      |  3 ++-
 server/src/cli/setup.js                      |  1 +
 server/src/config.js                         |  4 ++++
 server/src/containers/docker.js              | 13 +++++++------
 server/src/containers/expiryChecker.js       |  7 +++----
 server/src/game/commandHandler.js            |  3 ++-
 server/src/index.js                          | 10 +++++-----
 server/src/middleware/error.js               |  4 +++-
 server/src/models/sessionManager.js          |  5 +++--
 server/src/utils/logger.js                   | 14 +++++++++++---
 16 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/.eslintrc.js b/.eslintrc.js
index 1d3cb3d..85b46cb 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -17,7 +17,7 @@ module.exports = {
     sourceType: 'module',
   },
   rules: {
-    'no-console': process.env.NODE_ENV === 'production' ? 'warn' : 'off',
+    'no-console': 'error',
     'no-debugger': process.env.NODE_ENV === 'production' ? 'warn' : 'off',
     'import/extensions': 'off',
     camelcase: ['error', { properties: 'never', ignoreDestructuring: true }],
diff --git a/docker-compose.yml b/docker-compose.yml
index 3aeaf0b..e64b394 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -33,6 +33,7 @@ services:
       - ./server/swagger.yaml:/app/server/swagger.yaml
       - ./packages:/app/packages
       - ./quests:/app/quests
+      - ./logs:/app/logs
     environment:
       - BASE_URL=http://localhost:5173
       - MONGO_URL=mongodb://db:27017/odyssey-test
diff --git a/server/src/api/controllers/authController.js b/server/src/api/controllers/authController.js
index 2ddcd50..abfb053 100644
--- a/server/src/api/controllers/authController.js
+++ b/server/src/api/controllers/authController.js
@@ -2,6 +2,7 @@ import { matchedData } from 'express-validator'
 import { genJWT } from '../../utils/auth.js'
 import { createUser } from '../../models/userManager.js'
 import { asyncHandler } from '../../middleware/error.js'
+import logger from '../../utils/logger.js'
 
 export const issueToken = asyncHandler(async (req, res) => {
   const { user } = req
@@ -31,7 +32,7 @@ export const register = asyncHandler(async (req, res, next) => {
 export function logout(req, res) {
   req.logout((err) => {
     if (err) {
-      console.error(err)
+      logger.error(err)
       res.status(500).json({
         message: 'error logging out',
       })
diff --git a/server/src/api/socket.js b/server/src/api/socket.js
index 7a0c542..20a06f7 100644
--- a/server/src/api/socket.js
+++ b/server/src/api/socket.js
@@ -4,6 +4,7 @@ import { Session } from '@linux-odyssey/models'
 import { getAndStartContainer, attachContainer } from '../containers/docker.js'
 import SessionMiddleware from '../middleware/session.js'
 import { genJWT } from '../utils/auth.js'
+import logger from '../utils/logger.js'
 
 const sessions = new Map()
 
@@ -37,7 +38,7 @@ async function connectContainer(socket, next) {
   // console.log(socket.handshake)
   const { sessionId } = socket.handshake.query
   if (!(sessionId && validator.isMongoId(sessionId))) {
-    console.warn('Invalid Session ID.', sessionId)
+    logger.warn('Invalid Session ID.', user.username, sessionId)
     next(new Error('Invalid Session ID.'))
     return
   }
@@ -71,7 +72,7 @@ function onConnect(socket) {
     socket.emit('terminal', chunk.toString())
   })
 
-  socket.on('message', console.log)
+  socket.on('message', logger.info)
 
   socket.on('terminal', function incoming(message) {
     stream.socket.write(message)
@@ -98,6 +99,6 @@ export default (server) => {
 
   io.on('connection', onConnect)
   io.on('connect_error', (err) => {
-    console.error(err)
+    logger.error('Socket connect error: ', err)
   })
 }
diff --git a/server/src/auth/passport.js b/server/src/auth/passport.js
index 16b26f9..c884d5a 100644
--- a/server/src/auth/passport.js
+++ b/server/src/auth/passport.js
@@ -4,6 +4,7 @@ import passwordStrategy from './passwordStrategy.js'
 import jwtStrategy from './jwtStrategy.js'
 import googleStrategy from './googleStrategy.js'
 import githubStrategy from './githubStrategy.js'
+import logger from '../utils/logger.js'
 
 passport.use(passwordStrategy)
 passport.use(jwtStrategy)
@@ -14,7 +15,7 @@ const enabledMethods = {
   github: githubStrategy !== null,
 }
 
-console.log('Enabled login methods', enabledMethods)
+logger.info('Enabled login methods', enabledMethods)
 
 if (enabledMethods.google) {
   passport.use(googleStrategy)
diff --git a/server/src/cli/build.js b/server/src/cli/build.js
index 41c95d4..418e947 100644
--- a/server/src/cli/build.js
+++ b/server/src/cli/build.js
@@ -1,12 +1,13 @@
 import { getQuestDockerfiles } from '../utils/quest.js'
 import { buildQuestImage } from '../containers/docker.js'
+import logger from '../utils/logger.js'
 
 async function buildImages() {
   const quests = await getQuestDockerfiles()
-  console.log(`Building ${quests.length} images`)
+  logger.info(`Building ${quests.length} images`)
   return Promise.all(
     quests.map(({ id, questPath }) => {
-      console.log(`Building image for ${id}`)
+      logger.info(`Building image for ${id}`)
       return buildQuestImage(questPath, id)
     })
   )
@@ -14,11 +15,11 @@ async function buildImages() {
 
 buildImages()
   .then((result) => {
-    console.log(result)
-    console.log('Done!')
+    logger.info(result)
+    logger.info('Done!')
     process.exit(0)
   })
   .catch((err) => {
-    console.error(err)
+    logger.error(err)
     process.exit(1)
   })
diff --git a/server/src/cli/clean.js b/server/src/cli/clean.js
index b73029f..fc11da2 100644
--- a/server/src/cli/clean.js
+++ b/server/src/cli/clean.js
@@ -1,6 +1,7 @@
 import connectDB from '@linux-odyssey/models'
 import { removeExpired } from '../containers/expiryChecker.js'
 import config from '../config.js'
+import logger from '../utils/logger.js'
 
 connectDB(config.db)
   .then(removeExpired)
@@ -8,6 +9,6 @@ connectDB(config.db)
     process.exit(0)
   })
   .catch((err) => {
-    console.error(err)
+    logger.error(err)
     process.exit(1)
   })
diff --git a/server/src/cli/setup.js b/server/src/cli/setup.js
index b146e3a..2a98552 100644
--- a/server/src/cli/setup.js
+++ b/server/src/cli/setup.js
@@ -1,3 +1,4 @@
+/* eslint-disable no-console */
 import { genSecret } from '../utils/env.js'
 
 function setup() {
diff --git a/server/src/config.js b/server/src/config.js
index 65008f8..3250b3b 100644
--- a/server/src/config.js
+++ b/server/src/config.js
@@ -46,6 +46,10 @@ const config = {
     imagePrefix: get('DOCKER_PREFIX', 'linuxodyssey/quest-'),
     mountQuest: get('MOUNT_QUEST', ''),
   },
+
+  log: {
+    path: get('LOG_PATH', '../logs'),
+  },
 }
 
 config.baseUrl = get('BASE_URL', `http://${config.host}:${config.port}`)
diff --git a/server/src/containers/docker.js b/server/src/containers/docker.js
index e71337f..20a6041 100644
--- a/server/src/containers/docker.js
+++ b/server/src/containers/docker.js
@@ -1,5 +1,6 @@
 import Docker from 'dockerode'
 import config, { getQuestImage } from '../config.js'
+import logger from '../utils/logger.js'
 
 const engine = new Docker()
 
@@ -33,7 +34,7 @@ export function createContainer(name, questId) {
 }
 
 export async function getAndStartContainer(id) {
-  console.log(`Getting container: ${id}`)
+  logger.debug(`Getting container: ${id}`)
   const container = engine.getContainer(id)
   if (!container) {
     throw new Error(`Container ${id} not found`)
@@ -74,7 +75,7 @@ export async function deleteContainer(id) {
   try {
     await container.stop()
   } catch (error) {
-    console.log(error)
+    logger.warn(error)
   }
   await container.remove()
 }
@@ -99,21 +100,21 @@ export function buildQuestImage(questPath, questId) {
           // Process the data (this could be Docker build output)
           const { stream, error } = JSON.parse(data.toString())
           if (stream) {
-            console.log(stream)
+            logger.info(stream)
           }
           if (error) {
-            console.error(error)
+            logger.error(error)
             reject(error)
           }
         })
 
         response.on('end', () => {
-          console.log(`Build completed for ${questId}`)
+          logger.info(`Build completed for ${questId}`)
           resolve(questId)
         })
 
         response.on('error', (error) => {
-          console.error(`Build failed for ${questId}:`, error)
+          logger.error(`Build failed for ${questId}:`, error)
           reject(error)
         })
       }
diff --git a/server/src/containers/expiryChecker.js b/server/src/containers/expiryChecker.js
index b591722..a61363b 100644
--- a/server/src/containers/expiryChecker.js
+++ b/server/src/containers/expiryChecker.js
@@ -2,19 +2,19 @@ import schedule from 'node-schedule'
 import { Session } from '@linux-odyssey/models'
 import config from '../config.js'
 import { deleteContainer } from './docker.js'
+import logger from '../utils/logger.js'
 
 export async function removeExpired() {
   // Find all sessions that are containerId is not null and lastActivityAt is
   // less than the current time minus the expiry time
   try {
-    console.log('Checking for expired sessions')
     const sessions = await Session.find({
       containerId: { $ne: null },
       lastActivityAt: { $lt: new Date(Date.now() - config.containerExpiry) },
     })
     await Promise.all(
       sessions.map(async (session) => {
-        console.log('Expiring session: ', session._id, session.containerId)
+        logger.info('Expiring session: ', session._id, session.containerId)
         try {
           await deleteContainer(session.containerId)
         } catch (err) {
@@ -29,9 +29,8 @@ export async function removeExpired() {
         await session.save()
       })
     )
-    console.log('Done checking for expired sessions')
   } catch (err) {
-    console.error(err)
+    logger.error(err)
   }
 }
 
diff --git a/server/src/game/commandHandler.js b/server/src/game/commandHandler.js
index d03f6f1..95fc34f 100644
--- a/server/src/game/commandHandler.js
+++ b/server/src/game/commandHandler.js
@@ -4,6 +4,7 @@ import { Quest } from '@linux-odyssey/models'
 import { pushToSession } from '../api/socket.js'
 import SessionHandler from './sessionHandler.js'
 import { checkFile } from '../containers/cli.js'
+import logger from '../utils/logger.js'
 
 // 檢查 pattern 是否符合 input
 const checkMatch = (pattern, input) => {
@@ -101,7 +102,7 @@ export default class CommandHandler extends SessionHandler {
     this.quest = await Quest.findById(this.session.quest)
     const stages = this.getStages()
     if (stages.length === 0) {
-      console.error('stage not found', this.session.tasks)
+      logger.warn('stage not found', this.session.tasks)
       return {}
     }
 
diff --git a/server/src/index.js b/server/src/index.js
index 58ff44c..0004af7 100644
--- a/server/src/index.js
+++ b/server/src/index.js
@@ -28,9 +28,9 @@ async function main() {
 
   try {
     await connectDB(config.db)
-    console.log('Connected to MongoDB')
+    logger.info('Connected to MongoDB')
   } catch (err) {
-    console.error(err)
+    logger.error(err)
     process.exit(1)
   }
 
@@ -53,7 +53,7 @@ async function main() {
     'uniquelocal',
     ...config.trustedProxies,
   ])
-  console.log('Trusted proxies:', config.trustedProxies)
+  logger.info('Trusted proxies:', config.trustedProxies)
   app.use(globalRateLimit)
   app.use(sessionMiddleware)
   app.use(passport.session())
@@ -74,8 +74,8 @@ async function main() {
   app.use(errorHandler)
 
   server.listen(config.port, config.host, () => {
-    console.log(`Server listening at ${config.baseUrl}`)
+    logger.info(`Server listening at ${config.baseUrl}`)
   })
 }
 
-main().catch((err) => console.log(err))
+main().catch((err) => logger.error(err))
diff --git a/server/src/middleware/error.js b/server/src/middleware/error.js
index 32a3e80..51847e1 100644
--- a/server/src/middleware/error.js
+++ b/server/src/middleware/error.js
@@ -1,3 +1,5 @@
+import logger from '../utils/logger.js'
+
 export function asyncHandler(fn) {
   return (req, res, next) => {
     fn(req, res, next).catch(next)
@@ -6,6 +8,6 @@ export function asyncHandler(fn) {
 
 // eslint-disable-next-line no-unused-vars
 export default function errorHandler(err, req, res, next) {
-  console.error(err.stack) // Log error stack trace to the console
+  logger.error(err) // Log error stack trace to the console
   res.status(500).send({ message: 'Something broke!' }) // Send error response
 }
diff --git a/server/src/models/sessionManager.js b/server/src/models/sessionManager.js
index 8a9bbb9..a8be9dd 100644
--- a/server/src/models/sessionManager.js
+++ b/server/src/models/sessionManager.js
@@ -1,6 +1,7 @@
 import { Session, Quest, UserProfile } from '@linux-odyssey/models'
 import { createContainer, deleteContainer } from '../containers/docker.js'
 import SessionHandler from '../game/sessionHandler.js'
+import logger from '../utils/logger.js'
 
 async function deactivateSessions(user, quest) {
   const sessions = await Session.find({
@@ -11,7 +12,7 @@ async function deactivateSessions(user, quest) {
   return Promise.all(
     sessions.map((session) => {
       return deleteContainer(session.containerId)
-        .catch(console.error)
+        .catch(logger.error)
         .finally(() => {
           session.status = 'inactive'
           session.containerId = null
@@ -29,7 +30,7 @@ export async function createNewSession(user, questId) {
 
   // deactivate all active sessions
   deactivateSessions(user._id, quest._id).catch((err) => {
-    console.error(err)
+    logger.error(err)
   })
 
   const container = await createContainer(
diff --git a/server/src/utils/logger.js b/server/src/utils/logger.js
index cd2f2af..a540c27 100644
--- a/server/src/utils/logger.js
+++ b/server/src/utils/logger.js
@@ -1,16 +1,24 @@
 import winston from 'winston'
+import config from '../config.js'
 
 const logger = winston.createLogger({
   level: 'info',
   format: winston.format.json(),
-  defaultMeta: { service: 'user-service' },
+  defaultMeta: { service: 'backend', timestamp: new Date().toISOString() },
   transports: [
     //
     // - Write all logs with importance level of `error` or less to `error.log`
     // - Write all logs with importance level of `info` or less to `combined.log`
     //
-    new winston.transports.File({ filename: 'error.log', level: 'error' }),
-    new winston.transports.File({ filename: 'combined.log' }),
+    new winston.transports.File({
+      dirname: config.log.path,
+      filename: 'error.log',
+      level: 'error',
+    }),
+    new winston.transports.File({
+      dirname: config.log.path,
+      filename: 'combined.log',
+    }),
   ],
 })