From 3cebcddc91a84d0ff2f01903acb856036ef7626a Mon Sep 17 00:00:00 2001
From: "Lin, Hung-Shin" <wancat@wancat.cc>
Date: Wed, 22 Nov 2023 11:26:03 +0800
Subject: [PATCH] fix: expiryCheck error handling

---
 packages/models/session.js             |  2 ++
 server/src/containers/docker.js        |  2 +-
 server/src/containers/expiryChecker.js | 45 ++++++++++++++++----------
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/packages/models/session.js b/packages/models/session.js
index 50c62b5..4d4e185 100644
--- a/packages/models/session.js
+++ b/packages/models/session.js
@@ -35,10 +35,12 @@ const Session = model(
       user: {
         type: Schema.ObjectId,
         ref: 'User',
+        required: true,
       },
       quest: {
         type: String,
         ref: 'Quest',
+        required: true,
       },
       containerId: String,
       status: {
diff --git a/server/src/containers/docker.js b/server/src/containers/docker.js
index 8eee2ce..939e276 100644
--- a/server/src/containers/docker.js
+++ b/server/src/containers/docker.js
@@ -76,7 +76,7 @@ export async function deleteContainer(id) {
   try {
     await container.stop()
   } catch (error) {
-    console.log(error)
+    console.warn('Failed to stop container', id)
   }
   await container.remove()
 }
diff --git a/server/src/containers/expiryChecker.js b/server/src/containers/expiryChecker.js
index b591722..85e431c 100644
--- a/server/src/containers/expiryChecker.js
+++ b/server/src/containers/expiryChecker.js
@@ -3,6 +3,28 @@ import { Session } from '@linux-odyssey/models'
 import config from '../config.js'
 import { deleteContainer } from './docker.js'
 
+async function disableSession(session) {
+  try {
+    await deleteContainer(session.containerId)
+  } catch (err) {
+    // If the container doesn't exist, we can ignore the error
+    if (err.statusCode !== 404) {
+      throw new Error(
+        `Failed to delete container ${session.containerId} for session ${session._id}`
+      )
+    }
+  }
+  if (session.status === 'active') {
+    session.status = 'inactive'
+  }
+  session.containerId = null
+  try {
+    await session.save()
+  } catch (err) {
+    throw new Error(`Failed to save session ${session._id}\n${err}`)
+  }
+}
+
 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
@@ -13,25 +35,14 @@ export async function removeExpired() {
       lastActivityAt: { $lt: new Date(Date.now() - config.containerExpiry) },
     })
     await Promise.all(
-      sessions.map(async (session) => {
-        console.log('Expiring session: ', session._id, session.containerId)
-        try {
-          await deleteContainer(session.containerId)
-        } catch (err) {
-          if (err.statusCode !== 404) {
-            throw err
-          }
-        }
-        if (session.status === 'active') {
-          session.status = 'inactive'
-        }
-        session.containerId = null
-        await session.save()
-      })
+      sessions.map((session) =>
+        disableSession(session).catch((err) => {
+          throw new Error(`Failed to expire session: ${session._id}\n${err}`)
+        })
+      )
     )
-    console.log('Done checking for expired sessions')
   } catch (err) {
-    console.error(err)
+    console.error('Remove expiring failed:', err)
   }
 }