feat: Refactor nodemon configuration and improve error handling in NodemonService

elibosley · elibosley · commit c6cb6d2c88ca · 2025-11-21T16:00:31.000-05:00
- Updated nodemon.json to remove unnecessary watch entry.
- Adjusted NODEMON_CONFIG_PATH and UNRAID_API_CWD paths for better structure.
- Enhanced error handling in isUnraidApiRunning and start methods of NodemonService to ensure proper logging and resource management.
- Added tests for error scenarios in NodemonService to ensure robustness.
diff --git a/api/nodemon.json b/api/nodemon.json
@@ -1,7 +1,6 @@
 {
     "watch": [
-        "dist/main.js",
-        "myservers.cfg"
+        "dist/main.js"
     ],
     "ignore": [
         "node_modules",
diff --git a/api/src/core/utils/process/unraid-api-running.ts b/api/src/core/utils/process/unraid-api-running.ts
@@ -4,17 +4,17 @@ import { fileExists } from '@app/core/utils/files/file-exists.js';
 import { NODEMON_PID_PATH } from '@app/environment.js';
 
 export const isUnraidApiRunning = async (): Promise<boolean> => {
-    if (!(await fileExists(NODEMON_PID_PATH))) {
-        return false;
-    }
+    try {
+        if (!(await fileExists(NODEMON_PID_PATH))) {
+            return false;
+        }
 
-    const pidText = (await readFile(NODEMON_PID_PATH, 'utf-8')).trim();
-    const pid = Number.parseInt(pidText, 10);
-    if (Number.isNaN(pid)) {
-        return false;
-    }
+        const pidText = (await readFile(NODEMON_PID_PATH, 'utf-8')).trim();
+        const pid = Number.parseInt(pidText, 10);
+        if (Number.isNaN(pid)) {
+            return false;
+        }
 
-    try {
         process.kill(pid, 0);
         return true;
     } catch {
diff --git a/api/src/environment.ts b/api/src/environment.ts
@@ -110,9 +110,9 @@ export const NODEMON_PATH = join(
     'bin',
     'nodemon.js'
 );
-export const NODEMON_CONFIG_PATH = join(import.meta.dirname, '../../', 'nodemon.json');
+export const NODEMON_CONFIG_PATH = join(import.meta.dirname, '../', 'nodemon.json');
 export const NODEMON_PID_PATH = process.env.NODEMON_PID_PATH ?? '/var/run/unraid-api/nodemon.pid';
-export const UNRAID_API_CWD = process.env.UNRAID_API_CWD ?? join(import.meta.dirname, '../../');
+export const UNRAID_API_CWD = process.env.UNRAID_API_CWD ?? join(import.meta.dirname, '../');
 
 export const PATHS_CONFIG_MODULES =
     process.env.PATHS_CONFIG_MODULES ?? '/boot/config/plugins/dynamix.my.servers/configs';
diff --git a/api/src/unraid-api/cli/nodemon.service.spec.ts b/api/src/unraid-api/cli/nodemon.service.spec.ts
@@ -8,7 +8,7 @@ import { fileExists } from '@app/core/utils/files/file-exists.js';
 import { NodemonService } from '@app/unraid-api/cli/nodemon.service.js';
 
 vi.mock('node:fs', () => ({
-    createWriteStream: vi.fn(() => ({ pipe: vi.fn() })),
+    createWriteStream: vi.fn(() => ({ pipe: vi.fn(), close: vi.fn() })),
 }));
 vi.mock('node:fs/promises');
 vi.mock('execa', () => ({ execa: vi.fn() }));
@@ -57,8 +57,21 @@ describe('NodemonService', () => {
         expect(mockMkdir).toHaveBeenCalledWith('/var/run/unraid-api', { recursive: true });
     });
 
+    it('throws error when directory creation fails', async () => {
+        const service = new NodemonService(logger);
+        const error = new Error('Permission denied');
+        mockMkdir.mockRejectedValue(error);
+
+        await expect(service.ensureNodemonDependencies()).rejects.toThrow('Permission denied');
+        expect(mockMkdir).toHaveBeenCalledWith('/var/log/unraid-api', { recursive: true });
+    });
+
     it('starts nodemon and writes pid file', async () => {
         const service = new NodemonService(logger);
+        const logStream = { pipe: vi.fn(), close: vi.fn() };
+        vi.mocked(createWriteStream).mockReturnValue(
+            logStream as unknown as ReturnType<typeof createWriteStream>
+        );
         const stdout = { pipe: vi.fn() };
         const stderr = { pipe: vi.fn() };
         const unref = vi.fn();
@@ -87,6 +100,60 @@ describe('NodemonService', () => {
         expect(unref).toHaveBeenCalled();
         expect(mockWriteFile).toHaveBeenCalledWith('/var/run/unraid-api/nodemon.pid', '123');
         expect(logger.info).toHaveBeenCalledWith('Started nodemon (pid 123)');
+        expect(logStream.close).not.toHaveBeenCalled();
+    });
+
+    it('throws error and aborts start when directory creation fails', async () => {
+        const service = new NodemonService(logger);
+        const error = new Error('Permission denied');
+        mockMkdir.mockRejectedValue(error);
+
+        await expect(service.start()).rejects.toThrow('Permission denied');
+        expect(logger.error).toHaveBeenCalledWith(
+            'Failed to ensure nodemon dependencies: Permission denied'
+        );
+        expect(execa).not.toHaveBeenCalled();
+    });
+
+    it('throws error and closes logStream when execa fails', async () => {
+        const service = new NodemonService(logger);
+        const logStream = { pipe: vi.fn(), close: vi.fn() };
+        vi.mocked(createWriteStream).mockReturnValue(
+            logStream as unknown as ReturnType<typeof createWriteStream>
+        );
+        const error = new Error('Command not found');
+        vi.mocked(execa).mockImplementation(() => {
+            throw error;
+        });
+
+        await expect(service.start()).rejects.toThrow('Failed to start nodemon: Command not found');
+        expect(logStream.close).toHaveBeenCalled();
+        expect(mockWriteFile).not.toHaveBeenCalled();
+        expect(logger.info).not.toHaveBeenCalled();
+    });
+
+    it('throws error and closes logStream when pid is missing', async () => {
+        const service = new NodemonService(logger);
+        const logStream = { pipe: vi.fn(), close: vi.fn() };
+        vi.mocked(createWriteStream).mockReturnValue(
+            logStream as unknown as ReturnType<typeof createWriteStream>
+        );
+        const stdout = { pipe: vi.fn() };
+        const stderr = { pipe: vi.fn() };
+        const unref = vi.fn();
+        vi.mocked(execa).mockReturnValue({
+            pid: undefined,
+            stdout,
+            stderr,
+            unref,
+        } as unknown as ReturnType<typeof execa>);
+
+        await expect(service.start()).rejects.toThrow(
+            'Failed to start nodemon: process spawned but no PID was assigned'
+        );
+        expect(logStream.close).toHaveBeenCalled();
+        expect(mockWriteFile).not.toHaveBeenCalled();
+        expect(logger.info).not.toHaveBeenCalled();
     });
 
     it('returns not running when pid file is missing', async () => {
@@ -98,4 +165,44 @@ describe('NodemonService', () => {
         expect(result).toBe(false);
         expect(logger.info).toHaveBeenCalledWith('unraid-api is not running (no pid file).');
     });
+
+    it('logs stdout when tail succeeds', async () => {
+        const service = new NodemonService(logger);
+        vi.mocked(execa).mockResolvedValue({
+            stdout: 'log line 1\nlog line 2',
+        } as unknown as Awaited<ReturnType<typeof execa>>);
+
+        const result = await service.logs(50);
+
+        expect(execa).toHaveBeenCalledWith('tail', ['-n', '50', '/var/log/graphql-api.log']);
+        expect(logger.log).toHaveBeenCalledWith('log line 1\nlog line 2');
+        expect(result).toBe('log line 1\nlog line 2');
+    });
+
+    it('handles ENOENT error when log file is missing', async () => {
+        const service = new NodemonService(logger);
+        const error = new Error('ENOENT: no such file or directory');
+        (error as Error & { code?: string }).code = 'ENOENT';
+        vi.mocked(execa).mockRejectedValue(error);
+
+        const result = await service.logs();
+
+        expect(logger.error).toHaveBeenCalledWith(
+            'Log file not found: /var/log/graphql-api.log (ENOENT: no such file or directory)'
+        );
+        expect(result).toBe('');
+    });
+
+    it('handles non-zero exit error from tail', async () => {
+        const service = new NodemonService(logger);
+        const error = new Error('Command failed with exit code 1');
+        vi.mocked(execa).mockRejectedValue(error);
+
+        const result = await service.logs(100);
+
+        expect(logger.error).toHaveBeenCalledWith(
+            'Failed to read logs from /var/log/graphql-api.log: Command failed with exit code 1'
+        );
+        expect(result).toBe('');
+    });
 });
diff --git a/api/src/unraid-api/cli/nodemon.service.ts b/api/src/unraid-api/cli/nodemon.service.ts
@@ -32,14 +32,8 @@ export class NodemonService {
     constructor(private readonly logger: LogService) {}
 
     async ensureNodemonDependencies() {
-        try {
-            await mkdir(PATHS_LOGS_DIR, { recursive: true });
-            await mkdir(dirname(NODEMON_PID_PATH), { recursive: true });
-        } catch (error) {
-            this.logger.error(
-                `Failed to fully ensure nodemon dependencies: ${error instanceof Error ? error.message : error}`
-            );
-        }
+        await mkdir(PATHS_LOGS_DIR, { recursive: true });
+        await mkdir(dirname(NODEMON_PID_PATH), { recursive: true });
     }
 
     private async getStoredPid(): Promise<number | null> {
@@ -59,28 +53,47 @@ export class NodemonService {
     }
 
     async start(options: StartOptions = {}) {
-        await this.ensureNodemonDependencies();
+        try {
+            await this.ensureNodemonDependencies();
+        } catch (error) {
+            this.logger.error(
+                `Failed to ensure nodemon dependencies: ${error instanceof Error ? error.message : error}`
+            );
+            throw error;
+        }
+
         await this.stop({ quiet: true });
 
-        const env = { ...process.env, ...options.env } as Record<string, string>;
+        const overrides = Object.fromEntries(
+            Object.entries(options.env ?? {}).filter(([, value]) => value !== undefined)
+        );
+        const env = { ...process.env, ...overrides } as Record<string, string>;
         const logStream = createWriteStream(PATHS_LOGS_FILE, { flags: 'a' });
 
-        const nodemonProcess = execa(NODEMON_PATH, ['--config', NODEMON_CONFIG_PATH, '--quiet'], {
-            cwd: UNRAID_API_CWD,
-            env,
-            detached: true,
-            stdio: ['ignore', 'pipe', 'pipe'],
-        });
-
-        nodemonProcess.stdout?.pipe(logStream);
-        nodemonProcess.stderr?.pipe(logStream);
-        nodemonProcess.unref();
+        let nodemonProcess;
+        try {
+            nodemonProcess = execa(NODEMON_PATH, ['--config', NODEMON_CONFIG_PATH, '--quiet'], {
+                cwd: UNRAID_API_CWD,
+                env,
+                detached: true,
+                stdio: ['ignore', 'pipe', 'pipe'],
+            });
+
+            nodemonProcess.stdout?.pipe(logStream);
+            nodemonProcess.stderr?.pipe(logStream);
+            nodemonProcess.unref();
+
+            if (!nodemonProcess.pid) {
+                logStream.close();
+                throw new Error('Failed to start nodemon: process spawned but no PID was assigned');
+            }
 
-        if (nodemonProcess.pid) {
             await writeFile(NODEMON_PID_PATH, `${nodemonProcess.pid}`);
             this.logger.info(`Started nodemon (pid ${nodemonProcess.pid})`);
-        } else {
-            this.logger.error('Failed to determine nodemon pid.');
+        } catch (error) {
+            logStream.close();
+            const errorMessage = error instanceof Error ? error.message : String(error);
+            throw new Error(`Failed to start nodemon: ${errorMessage}`);
         }
     }
 
@@ -126,8 +139,23 @@ export class NodemonService {
         return running;
     }
 
-    async logs(lines = 100) {
-        const { stdout } = await execa('tail', ['-n', `${lines}`, PATHS_LOGS_FILE]);
-        this.logger.log(stdout);
+    async logs(lines = 100): Promise<string> {
+        try {
+            const { stdout } = await execa('tail', ['-n', `${lines}`, PATHS_LOGS_FILE]);
+            this.logger.log(stdout);
+            return stdout;
+        } catch (error) {
+            const errorMessage = error instanceof Error ? error.message : String(error);
+            const isFileNotFound =
+                errorMessage.includes('ENOENT') ||
+                (error instanceof Error && 'code' in error && error.code === 'ENOENT');
+
+            if (isFileNotFound) {
+                this.logger.error(`Log file not found: ${PATHS_LOGS_FILE} (${errorMessage})`);
+            } else {
+                this.logger.error(`Failed to read logs from ${PATHS_LOGS_FILE}: ${errorMessage}`);
+            }
+            return '';
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"watch": [`
`3`		`- "dist/main.js",`
`4`		`- "myservers.cfg"`
	`3`	`+ "dist/main.js"`
`5`	`4`	`],`
`6`	`5`	`"ignore": [`
`7`	`6`	`"node_modules",`