From c2dac8d385afc805e587f79bcb4480c82b928a2b Mon Sep 17 00:00:00 2001
From: wancat <wancat@wancat.cc>
Date: Sun, 12 Nov 2023 00:11:35 +0800
Subject: [PATCH] fix: wrap all async controller with error handler

* add validations to newCommand
---
 app/src/utils/socket.js                       |  2 +-
 app/src/views/GamePage.vue                    | 10 ++-
 packages/container/cli.js                     | 10 ++-
 server/src/api/controllers/authController.js  | 47 +++++------
 .../src/api/controllers/commandController.js  | 24 ++----
 server/src/api/controllers/questController.js |  9 ++-
 .../src/api/controllers/sessionController.js  | 80 +++++++------------
 server/src/api/controllers/userController.js  | 18 ++---
 server/src/api/routes/commandRouter.js        |  3 +-
 server/src/api/validators/commandValidator.js | 17 ++++
 server/src/api/validators/sessionValidator.js |  4 +-
 server/src/game/commandHandler.js             | 10 +--
 server/src/middleware/error.js                |  6 ++
 13 files changed, 120 insertions(+), 120 deletions(-)
 create mode 100644 server/src/api/validators/commandValidator.js

diff --git a/app/src/utils/socket.js b/app/src/utils/socket.js
index 5411ab1..b93e145 100644
--- a/app/src/utils/socket.js
+++ b/app/src/utils/socket.js
@@ -23,7 +23,7 @@ class SocketWrapper {
       this.socket.once('terminal', () => {
         resolve()
       })
-      this.socket.once('connect_error', reject)
+      this.socket.on('connect_error', reject)
       this.bindListeners()
     })
   }
diff --git a/app/src/views/GamePage.vue b/app/src/views/GamePage.vue
index 3c8bce6..d34a80c 100644
--- a/app/src/views/GamePage.vue
+++ b/app/src/views/GamePage.vue
@@ -1,5 +1,6 @@
 <script setup>
 import { computed, onMounted } from 'vue'
+import { useToast } from 'vue-toastification'
 import sessionStore, { init } from '../store/session'
 import GameHeaderPart from '../components/GameHeaderPart.vue'
 import CommandlistPart from '../components/CommandlistPart.vue'
@@ -21,8 +22,15 @@ const props = defineProps({
   },
 })
 
+const toast = useToast()
+
 onMounted(async () => {
-  await init(props.questId)
+  try {
+    await init(props.questId)
+  } catch (err) {
+    console.error(err)
+    toast.error('Failed to initialize game session')
+  }
 })
 </script>
 
diff --git a/packages/container/cli.js b/packages/container/cli.js
index 2165189..42890be 100644
--- a/packages/container/cli.js
+++ b/packages/container/cli.js
@@ -48,7 +48,7 @@ async function main() {
     },
   })
 
-  const additionalData = await handleCommand(CMD_NAME)
+  const params = await handleCommand(CMD_NAME)
 
   const output = await readOrNone(CMD_OUTPUT_FILE)
   const error = await readOrNone(CMD_ERROR_FILE)
@@ -58,7 +58,7 @@ async function main() {
     error,
     exit_code: CMD_EXIT_CODE,
     pwd: PWD,
-    ...additionalData,
+    params,
   }
   try {
     const res = await api.post('/commands', payload)
@@ -77,7 +77,11 @@ async function main() {
       console.log(colorize('Quest completed!', 'blue'))
     }
   } catch (err) {
-    console.error(err)
+    if (err.response) {
+      console.error(err.response.data)
+      return
+    }
+    console.error(err.message)
   }
 }
 
diff --git a/server/src/api/controllers/authController.js b/server/src/api/controllers/authController.js
index 5c25232..6ac6a1f 100644
--- a/server/src/api/controllers/authController.js
+++ b/server/src/api/controllers/authController.js
@@ -1,8 +1,9 @@
 import { matchedData } from 'express-validator'
 import { User } from '@linux-odyssey/models'
 import { genJWT, hashPassword } from '../../utils/auth.js'
+import { asyncHandler } from '../../middleware/error.js'
 
-export async function issueToken(req, res) {
+export const issueToken = asyncHandler(async (req, res) => {
   const { user } = req
   const token = await genJWT({
     _id: user.id,
@@ -10,34 +11,27 @@ export async function issueToken(req, res) {
     email: user.email,
   })
   res.json({ token })
-}
+})
 
-export async function register(req, res, next) {
+export const register = asyncHandler(async (req, res, next) => {
   const { username, password, email } = matchedData(req)
 
   const user = new User({
     username,
     email,
   })
-  try {
-    user.hashedPassword = await hashPassword(password)
-    await user.save()
-    req.login(user, (err) => {
-      if (err) {
-        next(err)
-        return
-      }
-      res.status(201).json({
-        message: 'user created',
-      })
+  user.hashedPassword = await hashPassword(password)
+  await user.save()
+  req.login(user, (err) => {
+    if (err) {
+      next(err)
+      return
+    }
+    res.status(201).json({
+      message: 'user created',
     })
-  } catch (err) {
-    console.error(err)
-    res.status(500).json({
-      message: 'error registering user',
-    })
-  }
-}
+  })
+})
 
 export function logout(req, res) {
   req.logout((err) => {
@@ -60,7 +54,7 @@ export function checkSession(req, res) {
   })
 }
 
-export async function socialLogin(req, res) {
+export function socialLogin(req, res) {
   const { newUser } = req.user
   if (newUser) {
     req.session.newUser = newUser
@@ -70,7 +64,7 @@ export async function socialLogin(req, res) {
   res.redirect('/')
 }
 
-export async function registerFromSession(req, res) {
+export const registerFromSession = asyncHandler(async (req, res, next) => {
   const { username } = matchedData(req)
   const { newUser } = req.session
 
@@ -88,10 +82,7 @@ export async function registerFromSession(req, res) {
 
   req.login(user, (err) => {
     if (err) {
-      console.error(err)
-      res.status(500).json({
-        message: 'error logging in',
-      })
+      next(err)
       return
     }
     delete req.session.newUser
@@ -99,4 +90,4 @@ export async function registerFromSession(req, res) {
       message: 'user created',
     })
   })
-}
+})
diff --git a/server/src/api/controllers/commandController.js b/server/src/api/controllers/commandController.js
index ca19ac3..fa8de98 100644
--- a/server/src/api/controllers/commandController.js
+++ b/server/src/api/controllers/commandController.js
@@ -1,27 +1,21 @@
 import { Command, Session } from '@linux-odyssey/models'
+import { matchedData } from 'express-validator'
 import CommandHandler from '../../game/commandHandler.js'
 import { pushToSession } from '../socket.js'
 import { finishSession } from '../../game/sessionManager.js'
+import { asyncHandler } from '../../middleware/error.js'
 
 const commandCompleteCallbacks = new Map()
 
-export async function newCommand(req, res) {
-  const { command, pwd, output, error, ...additionalData } = req.body
-
-  if (!command) {
-    res.status(400).json({ message: 'command is required' })
-    return
-  }
+export const newCommand = asyncHandler(async (req, res) => {
+  const { command, pwd, output, error, params } = matchedData(req)
 
   const { sessionId } = req.user
-
   const session = await Session.findById(sessionId)
-
   if (!session) {
     res.status(404).json({ message: 'session not found' })
     return
   }
-
   if (session.status === 'finished') {
     // Allow commands to be sent to finished sessions
     res.status(200).end()
@@ -44,10 +38,8 @@ export async function newCommand(req, res) {
 
   await c.save()
 
-  const commandHandler = new CommandHandler(session, c, additionalData)
-
+  const commandHandler = new CommandHandler(session, c, params)
   const response = await commandHandler.run()
-
   if (response) {
     c.stage = response.stage
     await c.save()
@@ -61,9 +53,9 @@ export async function newCommand(req, res) {
 
   res.status(201).json(response)
   await session.save()
-}
+})
 
-export async function completedCommand(req, res) {
+export const completedCommand = asyncHandler(async (req, res) => {
   const { sessionId } = req.user
 
   const session = await Session.findById(sessionId)
@@ -86,4 +78,4 @@ export async function completedCommand(req, res) {
   callback()
   commandCompleteCallbacks.delete(session.id)
   res.json({ message: 'ok' })
-}
+})
diff --git a/server/src/api/controllers/questController.js b/server/src/api/controllers/questController.js
index f25f42a..b27ab1e 100644
--- a/server/src/api/controllers/questController.js
+++ b/server/src/api/controllers/questController.js
@@ -1,7 +1,8 @@
 import { Quest } from '@linux-odyssey/models'
 import { matchedData } from 'express-validator'
+import { asyncHandler } from '../../middleware/error.js'
 
-export async function getQuests(req, res) {
+export const getQuests = asyncHandler(async (req, res) => {
   const quests = await Quest.find({})
   res.json(
     quests.map(({ title, _id, requirements }) => ({
@@ -10,9 +11,9 @@ export async function getQuests(req, res) {
       requirements,
     }))
   )
-}
+})
 
-export async function getQuestDetail(req, res) {
+export const getQuestDetail = asyncHandler(async (req, res) => {
   const { questId } = matchedData(req)
   const quest = await Quest.findById(questId)
   if (!quest) {
@@ -21,4 +22,4 @@ export async function getQuestDetail(req, res) {
   }
   const { _id, title, order, instruction } = quest
   res.json({ _id, title, order, instruction })
-}
+})
diff --git a/server/src/api/controllers/sessionController.js b/server/src/api/controllers/sessionController.js
index 69ce2af..9cb8d18 100644
--- a/server/src/api/controllers/sessionController.js
+++ b/server/src/api/controllers/sessionController.js
@@ -4,6 +4,7 @@ import {
   createNewSession,
   getOrCreateActiveSession,
 } from '../../game/sessionManager.js'
+import { asyncHandler } from '../../middleware/error.js'
 
 function sessionSummary(session) {
   return {
@@ -25,59 +26,40 @@ function sessionDetail(session) {
   }
 }
 
-export async function getSessionList(req, res) {
-  try {
-    const query = { user: req.user._id }
-    const { questId, status } = matchedData(req)
-    if (questId) {
-      query.quest = questId
-    }
-    query.status = status || 'active'
-    const sessions = await Session.find(query)
-    res.json(sessions.map(sessionSummary))
-  } catch (err) {
-    console.error(err)
-    res.status(500).json({ message: err.message })
+export const getSessionList = asyncHandler(async (req, res) => {
+  const query = { user: req.user._id }
+  const { questId, status } = matchedData(req)
+  if (questId) {
+    query.quest = questId
   }
-}
+  query.status = status || 'active'
+  const sessions = await Session.find(query)
+  res.json(sessions.map(sessionSummary))
+})
 
-export async function createSession(req, res) {
+export const createSession = asyncHandler(async (req, res) => {
   const { questId } = matchedData(req)
-  try {
-    const session = await createNewSession(req.user, questId)
-    res.status(201).json(sessionDetail(session))
-  } catch (err) {
-    console.error(err)
-    res.status(500).json({ message: err.message })
+  const session = await createNewSession(req.user, questId)
+  res.status(201).json(sessionDetail(session))
+})
+
+export const getSessionById = asyncHandler(async (req, res) => {
+  const { sessionId } = matchedData(req)
+  const session = await Session.findOne({
+    _id: sessionId,
+    user: req.user._id,
+  })
+
+  if (!session) {
+    res.status(404).json({ message: 'Session not found.' })
+    return
   }
-}
 
-export async function getSessionById(req, res) {
-  try {
-    const { sessionId } = matchedData(req)
-    const session = await Session.findOne({
-      _id: sessionId,
-      user: req.user._id,
-    })
+  res.json(sessionDetail(session))
+})
 
-    if (!session) {
-      res.status(404).json({ message: 'Session not found.' })
-      return
-    }
-
-    res.json(sessionDetail(session))
-  } catch (err) {
-    res.status(500).json({ message: err.message })
-  }
-}
-
-export async function getOrCreateSession(req, res) {
+export const getOrCreateSession = asyncHandler(async (req, res) => {
   const { questId } = matchedData(req)
-  try {
-    const session = await getOrCreateActiveSession(req.user, questId)
-    res.json(sessionDetail(session))
-  } catch (err) {
-    console.error(err)
-    res.status(500).json({ message: err.message })
-  }
-}
+  const session = await getOrCreateActiveSession(req.user, questId)
+  res.json(sessionDetail(session))
+})
diff --git a/server/src/api/controllers/userController.js b/server/src/api/controllers/userController.js
index 1374634..b819c96 100644
--- a/server/src/api/controllers/userController.js
+++ b/server/src/api/controllers/userController.js
@@ -1,20 +1,20 @@
 import { UserProfile } from '@linux-odyssey/models'
+import { asyncHandler } from '../../middleware/error.js'
 
-export async function me(req, res) {
+export const me = asyncHandler(async (req, res) => {
   const userProfile = await UserProfile.findOne({
     user: req.user._id,
-  }).populate('user')
+  })
   if (!userProfile) {
-    res.status(404).json({ message: 'User not found' })
-    return
+    throw new Error(
+      `UserProfile not found for user ${req.user.username} ${req.user._id}!`
+    )
   }
-  const {
-    user: { username, email },
-    progress,
-  } = userProfile
+  const { username, email } = req.user
+  const { progress } = userProfile
   res.json({
     username,
     email,
     progress,
   })
-}
+})
diff --git a/server/src/api/routes/commandRouter.js b/server/src/api/routes/commandRouter.js
index eb116c3..ef22842 100644
--- a/server/src/api/routes/commandRouter.js
+++ b/server/src/api/routes/commandRouter.js
@@ -1,9 +1,10 @@
 import { Router } from 'express'
 import * as commandController from '../controllers/commandController.js'
+import { newCommandValidator } from '../validators/commandValidator.js'
 
 const router = Router()
 
-router.post('/', commandController.newCommand)
+router.post('/', newCommandValidator, commandController.newCommand)
 router.post('/completed', commandController.completedCommand)
 
 export default router
diff --git a/server/src/api/validators/commandValidator.js b/server/src/api/validators/commandValidator.js
new file mode 100644
index 0000000..3b71529
--- /dev/null
+++ b/server/src/api/validators/commandValidator.js
@@ -0,0 +1,17 @@
+import { body } from 'express-validator'
+import { noError } from '../../middleware/validator.js'
+
+const checkCommand = () => body('command').isString().notEmpty().trim()
+const checkPwd = () => body('pwd').isString().notEmpty().trim()
+const checkOutput = () => body('output').trim().optional()
+const checkError = () => body('error').trim().optional()
+const checkParams = () => body('params').isObject().optional()
+
+export const newCommandValidator = [
+  checkCommand(),
+  checkPwd(),
+  checkOutput(),
+  checkError(),
+  checkParams(),
+  noError,
+]
diff --git a/server/src/api/validators/sessionValidator.js b/server/src/api/validators/sessionValidator.js
index ccc441a..ddb3c84 100644
--- a/server/src/api/validators/sessionValidator.js
+++ b/server/src/api/validators/sessionValidator.js
@@ -3,9 +3,7 @@ import { checkQuestId } from './questValidator.js'
 import { noError } from '../../middleware/validator.js'
 
 const checkSessionStatus = () =>
-  check('status')
-    .isString()
-    .custom((value) => ['active', 'inactive', 'finished'].includes(value))
+  check('status').isString().isIn(['active', 'inactive', 'finished'])
 
 export const checkSessionId = () =>
   check('sessionId').notEmpty().isString().isMongoId()
diff --git a/server/src/game/commandHandler.js b/server/src/game/commandHandler.js
index 54ea91f..d03f6f1 100644
--- a/server/src/game/commandHandler.js
+++ b/server/src/game/commandHandler.js
@@ -15,16 +15,16 @@ const checkMatch = (pattern, input) => {
 }
 
 export default class CommandHandler extends SessionHandler {
-  constructor(session, commandInput, additionalData) {
+  constructor(session, commandInput, params) {
     super(session)
     this.commandInput = commandInput
     this.argv = minimist(this.commandInput.command.split(' '))
 
-    this.additionalData = additionalData
+    this.params = params
   }
 
   handleEvent() {
-    if (this.additionalData.discover) this.discoverHandler()
+    if (this.params.discover) this.discoverHandler()
   }
 
   handleCommand() {
@@ -39,7 +39,7 @@ export default class CommandHandler extends SessionHandler {
       case 'ls':
         pushToSession(this.session.id, 'graph', {
           pwd: this.commandInput.pwd,
-          discover: this.additionalData.discover,
+          discover: this.params.discover,
         })
         break
 
@@ -50,7 +50,7 @@ export default class CommandHandler extends SessionHandler {
 
   async discoverHandler() {
     const graph = new FileGraph(this.session.graph)
-    graph.discover(this.additionalData.discover)
+    graph.discover(this.params.discover)
     this.session.graph = graph
   }
 
diff --git a/server/src/middleware/error.js b/server/src/middleware/error.js
index 5af9688..32a3e80 100644
--- a/server/src/middleware/error.js
+++ b/server/src/middleware/error.js
@@ -1,3 +1,9 @@
+export function asyncHandler(fn) {
+  return (req, res, next) => {
+    fn(req, res, next).catch(next)
+  }
+}
+
 // 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