From f504a00ce6ff1ff9e1728a02c7fcc3dc5e1f221c Mon Sep 17 00:00:00 2001 From: SuperComboGamer Date: Sat, 20 Dec 2025 23:35:03 -0500 Subject: [PATCH] feat: improve error handling in terminal settings retrieval and enhance path normalization - Wrapped the terminal settings retrieval in a try-catch block to handle potential errors and respond with a 500 status and error details. - Updated path normalization logic to skip resolution for WSL UNC paths, preventing potential issues with path handling in Windows Subsystem for Linux. - Enhanced unit tests for session termination to include timer-based assertions for graceful session killing. --- .../src/routes/terminal/routes/settings.ts | 25 +++++++++++++------ apps/server/src/services/terminal-service.ts | 6 +++-- .../unit/services/terminal-service.test.ts | 10 +++++++- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/apps/server/src/routes/terminal/routes/settings.ts b/apps/server/src/routes/terminal/routes/settings.ts index 08e670ae..3af142a8 100644 --- a/apps/server/src/routes/terminal/routes/settings.ts +++ b/apps/server/src/routes/terminal/routes/settings.ts @@ -8,14 +8,23 @@ import { getErrorMessage, logError } from "../common.js"; export function createSettingsGetHandler() { return (_req: Request, res: Response): void => { - const terminalService = getTerminalService(); - res.json({ - success: true, - data: { - maxSessions: terminalService.getMaxSessions(), - currentSessions: terminalService.getSessionCount(), - }, - }); + try { + const terminalService = getTerminalService(); + res.json({ + success: true, + data: { + maxSessions: terminalService.getMaxSessions(), + currentSessions: terminalService.getSessionCount(), + }, + }); + } catch (error) { + logError(error, "Get terminal settings failed"); + res.status(500).json({ + success: false, + error: "Failed to get terminal settings", + details: getErrorMessage(error), + }); + } }; } diff --git a/apps/server/src/services/terminal-service.ts b/apps/server/src/services/terminal-service.ts index c11af057..8eb0d76b 100644 --- a/apps/server/src/services/terminal-service.ts +++ b/apps/server/src/services/terminal-service.ts @@ -181,8 +181,10 @@ export class TerminalService extends EventEmitter { } // Normalize the path to resolve . and .. segments - // This converts relative paths to absolute and cleans up the path - cwd = path.resolve(cwd); + // Skip normalization for WSL UNC paths as path.resolve would break them + if (!cwd.startsWith("//wsl")) { + cwd = path.resolve(cwd); + } // Check if path exists and is a directory try { diff --git a/apps/server/tests/unit/services/terminal-service.test.ts b/apps/server/tests/unit/services/terminal-service.test.ts index d273061a..b20e8047 100644 --- a/apps/server/tests/unit/services/terminal-service.test.ts +++ b/apps/server/tests/unit/services/terminal-service.test.ts @@ -408,6 +408,7 @@ describe("terminal-service.ts", () => { describe("killSession", () => { it("should kill existing session", () => { + vi.useFakeTimers(); vi.mocked(fs.existsSync).mockReturnValue(true); vi.mocked(fs.statSync).mockReturnValue({ isDirectory: () => true } as any); vi.spyOn(process, "env", "get").mockReturnValue({ SHELL: "/bin/bash" }); @@ -416,8 +417,15 @@ describe("terminal-service.ts", () => { const result = service.killSession(session.id); expect(result).toBe(true); - expect(mockPtyProcess.kill).toHaveBeenCalled(); + expect(mockPtyProcess.kill).toHaveBeenCalledWith("SIGTERM"); + + // Session is removed after SIGKILL timeout (1 second) + vi.advanceTimersByTime(1000); + + expect(mockPtyProcess.kill).toHaveBeenCalledWith("SIGKILL"); expect(service.getSession(session.id)).toBeUndefined(); + + vi.useRealTimers(); }); it("should return false for non-existent session", () => {