Refactor for Improved Maintainability and Stability

lib/config.ts

@@ -0,0 +1,10 @@
+// lib/config.ts
+import { env } from './env.js';
+
+const config = {
+  isProduction: env.NODE_ENV === 'production',
+  isDevelopment: env.NODE_ENV === 'development',
+  isTest: env.NODE_ENV === 'test',
+};
+
+export default config;

lib/env.ts

@@ -0,0 +1,14 @@
+// lib/env.ts
+import { z } from 'zod';
+
+const envSchema = z.object({
+  PORT: z.coerce.number().default(3000),
+  HOST: z.string().default('127.0.0.1'),
+  NODE_ENV: z.enum(['development', 'production', 'test']).default('development'),
+  NEXTAUTH_URL: z.string().url().optional(),
+  NEXTAUTH_SECRET: z.string().min(1),
+  SPOTIFY_CLIENT_ID: z.string().optional(),
+  SPOTIFY_CLIENT_SECRET: z.string().optional(),
+});
+
+export const env = envSchema.parse(process.env);

server.ts

@@ -24,53 +24,43 @@ import { StateSnapshot } from './types/websocket.js'
 import logger from './utils/logger.js'
 import swaggerUi from 'swagger-ui-express'
 import swaggerSpec from './lib/swagger.js'
+import { env } from './lib/env.js'
+import config from './lib/config.js'
 
-const port: number = process.env.PORT ? +process.env.PORT : 3000 // Explicitly handle undefined and convert to number
-// Allow overriding bind address via the HOST env var for flexibility in CI/containers
-const hostname =
-  process.env.NODE_ENV === 'production'
-    ? '0.0.0.0'
-    : process.env.HOST || '127.0.0.1' // Bind to all interfaces in production
-
-const dev = process.env.NODE_ENV !== 'production'
-
-// === QUICK WIN 1: CRITICAL SECURITY CHECK ===
-if (!dev && !process.env.NEXTAUTH_SECRET) {
-  console.error('FATAL: NEXTAUTH_SECRET environment variable is missing.')
-  console.error('This is mandatory for production security. Shutting down.')
-  process.exit(1)
-}
-// ===========================================
+const port = env.PORT
+const hostname = config.isProduction ? '0.0.0.0' : env.HOST
+const dev = !config.isProduction
 
 const app = next({ dev, hostname, port })
 
-logger.info(`Starting server in ${dev ? 'development' : 'production'} mode`)
-logger.info(`Environment: NODE_ENV=${process.env.NODE_ENV}`)
+logger.info(
+  `Starting server in ${config.isDevelopment ? 'development' : 'production'} mode`
+)
+logger.info(`Environment: NODE_ENV=${env.NODE_ENV}`)
 logger.info(`NEXTAUTH_URL: ${getBaseURL()}`)
 logger.info(`Hostname: ${hostname}, Port: ${port}`)
 const nextRequestHandler = app.getRequestHandler()
 
 // Create Express app for routing and middleware
 const expressApp = express()
 
-// --- Main Application Setup ---
-
-app
-  .prepare()
-  .then(async () => {
+/**
+ * Main application setup and server start.
+ * This function initializes the Next.js app, creates an HTTP server,
+ * attaches middleware, sets up the WebSocket server, and starts listening for requests.
+ */
+const startServer = async () => {
+  try {
+    await app.prepare()
     const server = createServer(expressApp)
 
     // --- Static Asset Serving (Production Only) ---
-    // In production, serve the Next.js static assets directly from the .next/static folder.
-    // This is more efficient than letting the Next.js handler do it.
-    if (!dev) {
+    if (config.isProduction) {
       const staticPath = path.join(process.cwd(), '.next/static')
       logger.info(`Serving static files from: ${staticPath}`)
-
       expressApp.use(
         '/_next/static',
         express.static(staticPath, {
-          // All files in _next/static have content hashes, so they can be cached indefinitely.
           immutable: true,
           maxAge: '365d',
         })
@@ -90,7 +80,6 @@ app
         type: 'SPOTIFY_SERVICE_INIT_UPDATE',
         payload: false,
       })
-      // Fallback stub to avoid crashing entire server if Spotify setup fails
       spotifyService = {
         handleCommand: () => {},
         stopPolling: () => {},
@@ -107,34 +96,43 @@ app
       spotifyServiceInitialized: spotifyService.isReady(),
     })
 
-    // 4. Initialize WebSocket Manager (to handle commands and connections)
-    initSocketManager(wss, { tabataService, spotifyService }, getUnifiedStateSnapshot)
+    // 4. Initialize WebSocket Manager
+    initSocketManager(
+      wss,
+      { tabataService, spotifyService },
+      getUnifiedStateSnapshot
+    )
 
     // --- Express Routing ---
-
-    // Swagger UI
     expressApp.use(
       '/api-docs',
       swaggerUi.serve,
       swaggerUi.setup(swaggerSpec)
     )
 
-    // Handle all Next.js routing (pages, API routes, etc.)
-    // Token delivery is handled by Next.js API route at /api/internal/token-delivery
+    // --- Health Check Endpoints ---
+    expressApp.get('/health/live', (req, res) => {
+      res.status(200).send('OK')
+    })
+
+    expressApp.get('/health/ready', (req, res) => {
+      const spotifyReady = spotifyService?.isReady() ?? false
+      if (spotifyReady) {
+        res.status(200).send('OK')
+      } else {
+        res.status(503).send('Service Unavailable')
+      }
+    })
+
     expressApp.use(async (req: Request, res: Response) => {
-      // Intercept token delivery POST and force Spotify poll
       if (
         req.method === 'POST' &&
         req.url &&
         req.url.includes('/api/internal/token-delivery')
       ) {
-        // Wait a moment for token to be written
         setTimeout(async () => {
           if (spotifyService) {
-            // Signal the service to reload tokens from disk
             spotifyService.setRefreshToken('signal')
-
-            // Wait a bit for reload, then force poll
             setTimeout(async () => {
               if (typeof spotifyService.forcePollAndBroadcast === 'function') {
                 await spotifyService.forcePollAndBroadcast()
@@ -144,41 +142,35 @@ app
         }, 1000)
       }
       return nextRequestHandler(req, res)
-    }) // --- HTTP/WS Upgrade Handling ---
+    })
 
-    // Attach the WebSocket server to the HTTP server instance using the 'upgrade' event
+    // --- HTTP/WS Upgrade Handling ---
     server.on(
       'upgrade',
       (req: IncomingMessage, socket: Socket, head: Buffer) => {
         const { pathname } = parse(req.url || '')
-
-        // Only upgrade connections to the specific WebSocket path
         if (pathname === '/ws') {
           wss.handleUpgrade(req, socket, head, (ws: WebSocket) => {
             wss.emit('connection', ws, req)
           })
         }
-        // If not our WebSocket path, simply return and let other upgrade handlers (e.g., Next.js's) take over.
-        // DO NOT re-emit "upgrade" as it can lead to infinite recursion.
       }
     )
 
     // --- Start Server ---
-
-    // Handle server errors (e.g., port already in use)
     server.on('error', (err: Error) => {
       logger.error({ err }, 'Server error')
       process.exit(1)
     })
 
-    // Begin listening
     server.listen(port, hostname, () => {
-      // This callback only runs on successful listening
       logger.info(`> Ready on http://${hostname}:${port}`)
       logger.info(`> WebSocket Server listening on ws://${hostname}:${port}/ws`)
     })
-  })
-  .catch((err: Error) => {
+  } catch (err) {
     logger.error({ err }, 'Next.js preparation failed')
     process.exit(1)
-  })
+  }
+}
+
+startServer()

utils/socketManager.ts

@@ -14,10 +14,11 @@ import {
   StateSnapshot,
 } from '../types/websocket.js'
 import { broadcast, initBroadcaster } from './broadcast.js'
+import logger from './logger.js'
 
 // Define service instances to be managed
-let tabataServiceInstance: TabataTimer
-let spotifyServiceInstance: SpotifyPolling
+let tabataService: TabataTimer
+let spotifyService: SpotifyPolling
 // New: Define a function to get the state snapshot
 let getUnifiedStateSnapshot: () => StateSnapshot
 
@@ -29,21 +30,24 @@ interface Services {
 }
 
 /**
- * Initializes the WebSocket Server manager and registers the core services.
+ * Initializes the WebSocket manager, sets up connection listeners, and registers services.
+ * @param wss The WebSocket server instance.
+ * @param services An object containing the core application services (TabataTimer, SpotifyPolling).
+ * @param getSnapshot A function that returns a complete snapshot of the current application state.
  */
 const initSocketManager = (
   wss: WebSocketServer,
   services: Services,
   getSnapshot: () => StateSnapshot
 ) => {
   initBroadcaster(wss)
-  tabataServiceInstance = services.tabataService
-  spotifyServiceInstance = services.spotifyService
+  tabataService = services.tabataService
+  spotifyService = services.spotifyService
   getUnifiedStateSnapshot = getSnapshot
 
   wss.on('connection', (ws: WebSocket) => {
     const clientId = `user-${Math.random().toString(36).substring(2, 9)}`
-    console.log(`WebSocket Client connected: ${clientId}`)
+    logger.info({ clientId }, 'WebSocket client connected')
 
     // Initialize with minimal placeholder; omit name so UI can suppress until real data arrives
     const defaultClientData: HrmData = {
@@ -60,7 +64,7 @@ const initSocketManager = (
     })
 
     ws.on('close', () => {
-      console.log(`WebSocket Client disconnected: ${clientId}`)
+      logger.info({ clientId }, 'WebSocket client disconnected')
       hrmClients.delete(clientId)
       broadcast({
         type: 'HRM_UPDATE',
@@ -71,28 +75,23 @@ const initSocketManager = (
 }
 
 /**
- * Handles incoming JSON messages from client applications.
+ * Parses, validates, and routes incoming messages from a WebSocket client.
+ * @param ws The WebSocket instance for the client that sent the message.
+ * @param jsonMessage The raw JSON message string received from the client.
+ * @param clientId The unique identifier for the connected client.
  */
 const handleIncomingMessage = (
   ws: WebSocket,
   jsonMessage: string,
   clientId: string
 ) => {
-  console.log(
-    `[socketManager] INCOMING MESSAGE from ${clientId}:`,
-    jsonMessage
-  )
+  logger.info({ clientId, jsonMessage }, 'Incoming WebSocket message')
   try {
     // Parse and validate message type for type-safe routing
     const parsedMessage = JSON.parse(jsonMessage)
-    console.log(`[socketManager] PARSED JSON:`, parsedMessage)
-
     const message = ClientCommandMessageSchema.parse(parsedMessage) // Use Zod for parsing and validation
 
-    console.log(
-      `[socketManager] Received message from ${clientId}:`,
-      message.type
-    )
+    logger.info({ clientId, type: message.type }, 'Received message')
 
     switch (message.type) {
       case 'GET_STATE': {
@@ -115,11 +114,13 @@ const handleIncomingMessage = (
 
       case 'HRM_INPUT': {
         const existingClientData = hrmClients.get(clientId)
-        console.log(
-          `[socketManager] HRM_INPUT - clientId: ${clientId}, existingData:`,
-          existingClientData,
-          'newValue:',
-          message.data.value
+        logger.info(
+          {
+            clientId,
+            existingData: existingClientData,
+            newValue: message.data.value,
+          },
+          'HRM_INPUT received'
         )
         if (existingClientData) {
           // Filter out null values to avoid overwriting valid data
@@ -130,9 +131,9 @@ const handleIncomingMessage = (
             ...existingClientData,
             ...updatedClientProperties,
           })
-          console.log(
-            `[socketManager] HRM_INPUT - Updated clientData for ${clientId}:`,
-            hrmClients.get(clientId)
+          logger.info(
+            { clientId, updatedData: hrmClients.get(clientId) },
+            'HRM_INPUT - Updated clientData'
           )
         }
         broadcast({
@@ -143,22 +144,22 @@ const handleIncomingMessage = (
       }
 
       case 'TIMER_COMMAND': {
-        if (tabataServiceInstance) {
-          tabataServiceInstance.handleCommand(message.command)
+        if (tabataService) {
+          tabataService.handleCommand(message.command)
         }
         break
       }
 
       case 'SET_MODE': {
-        if (tabataServiceInstance) {
-          tabataServiceInstance.setMode(message.mode)
+        if (tabataService) {
+          tabataService.setMode(message.mode)
         }
         break
       }
 
       case 'TIMER_CONFIG': {
-        if (tabataServiceInstance) {
-          tabataServiceInstance.setConfig({
+        if (tabataService) {
+          tabataService.setConfig({
             workDuration: message.workDuration,
             restDuration: message.restDuration,
           })
@@ -167,9 +168,9 @@ const handleIncomingMessage = (
       }
 
       case 'SPOTIFY_COMMAND': {
-        if (spotifyServiceInstance) {
+        if (spotifyService) {
           // message.command is already typed as Spotify_COMMAND, which now includes deviceId, volume, and playlistUri
-          spotifyServiceInstance.handleCommand(
+          spotifyService.handleCommand(
             message.command,
             message.deviceId,
             message.volume,
@@ -181,16 +182,16 @@ const handleIncomingMessage = (
 
       default:
         // This case should ideally not be reached if ClientCommandMessageSchema is exhaustive
-        console.warn(
-          'Unknown message type received:',
-          (message as { type: unknown }).type
+        logger.warn(
+          { type: (message as { type: unknown }).type },
+          'Unknown message type received'
         )
     }
   } catch (e) {
-    console.error('Error processing incoming message:', e)
+    logger.error({ err: e }, 'Error processing incoming message')
     // Add more specific error handling for Zod validation errors
     if (e instanceof z.ZodError) {
-      console.error('WebSocket message validation failed:', e.issues)
+      logger.error({ errors: e.issues }, 'WebSocket message validation failed')
     }
   }
 }