fix: invalid configs no longer crash API

.claude/settings.local.json

@@ -9,7 +9,8 @@
       "Bash(pnpm test:*)",
       "Bash(grep:*)",
       "Bash(pnpm type-check:*)",
-      "Bash(pnpm lint:*)"
+      "Bash(pnpm lint:*)",
+      "Bash(pnpm --filter ./api lint)"
     ]
   },
   "enableAllProjectMcpServers": false

api/src/core/utils/misc/parse-config.ts

@@ -124,7 +124,15 @@ export const parseConfig = <T extends Record<string, any>>(
         throw new AppError('Invalid Parameters Passed to ParseConfig');
     }
 
-    const data: Record<string, any> = parseIni(fileContents);
+    let data: Record<string, any>;
+    try {
+        data = parseIni(fileContents);
+    } catch (error) {
+        throw new AppError(
+            `Failed to parse config file: ${error instanceof Error ? error.message : String(error)}`
+        );
+    }
+
     // Remove quotes around keys
     const dataWithoutQuoteKeys = Object.fromEntries(
         Object.entries(data).map(([key, value]) => [key.replace(/^"(.+(?="$))"$/, '$1'), value])

api/src/unraid-api/config/api-config.module.ts

@@ -12,6 +12,8 @@ import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.help
 
 export { type ApiConfig };
 
+const logger = new Logger('ApiConfig');
+
 const createDefaultConfig = (): ApiConfig => ({
     version: API_VERSION,
     extraOrigins: [],
@@ -33,21 +35,54 @@ export const persistApiConfig = async (config: ApiConfig) => {
 };
 
 export const loadApiConfig = async () => {
-    const defaultConfig = createDefaultConfig();
-    const apiConfig = new ApiStateConfig<ApiConfig>(
-        {
-            name: 'api',
-            defaultConfig,
-            parse: (data) => data as ApiConfig,
-        },
-        new ConfigPersistenceHelper()
-    );
-    const diskConfig = await apiConfig.parseConfig();
-    return {
-        ...defaultConfig,
-        ...diskConfig,
-        version: API_VERSION,
-    };
+    try {
+        const defaultConfig = createDefaultConfig();
+        const apiConfig = new ApiStateConfig<ApiConfig>(
+            {
+                name: 'api',
+                defaultConfig,
+                parse: (data) => data as ApiConfig,
+            },
+            new ConfigPersistenceHelper()
+        );
+
+        let diskConfig: ApiConfig | undefined;
+        try {
+            diskConfig = await apiConfig.parseConfig();
+        } catch (error) {
+            logger.error('Failed to load API config from disk, using defaults:', error);
+            diskConfig = undefined;
+
+            // Try to overwrite the invalid config with defaults to fix the issue
+            try {
+                const configToWrite = {
+                    ...defaultConfig,
+                    version: API_VERSION,
+                };
+
+                const writeSuccess = await apiConfig.persist(configToWrite);
+                if (writeSuccess) {
+                    logger.log('Successfully overwrote invalid config file with defaults.');
+                } else {
+                    logger.error(
+                        'Failed to overwrite invalid config file. Continuing with defaults in memory only.'
+                    );
+                }
+            } catch (persistError) {
+                logger.error('Error during config file repair:', persistError);
+            }
+        }
+
+        return {
+            ...defaultConfig,
+            ...diskConfig,
+            version: API_VERSION,
+        };
+    } catch (outerError) {
+        // This should never happen, but ensures the config factory never throws
+        logger.error('Critical error in loadApiConfig, using minimal defaults:', outerError);
+        return createDefaultConfig();
+    }
 };
 
 /**
@@ -81,21 +116,29 @@ export class ApiConfigPersistence {
     }
 
     async onModuleInit() {
-        if (!(await fileExists(this.filePath))) {
-            this.migrateFromMyServersConfig();
+        try {
+            if (!(await fileExists(this.filePath))) {
+                this.migrateFromMyServersConfig();
+            }
+            await this.persistenceHelper.persistIfChanged(this.filePath, this.config);
+            this.configService.changes$.pipe(bufferTime(25)).subscribe({
+                next: async (changes) => {
+                    if (changes.some((change) => change.path.startsWith('api'))) {
+                        this.logger.verbose(`API Config changed ${JSON.stringify(changes)}`);
+                        try {
+                            await this.persistenceHelper.persistIfChanged(this.filePath, this.config);
+                        } catch (persistError) {
+                            this.logger.error('Error persisting config changes:', persistError);
+                        }
+                    }
+                },
+                error: (err) => {
+                    this.logger.error('Error receiving config changes:', err);
+                },
+            });
+        } catch (error) {
+            this.logger.error('Error during API config module initialization:', error);
         }
-        await this.persistenceHelper.persistIfChanged(this.filePath, this.config);
-        this.configService.changes$.pipe(bufferTime(25)).subscribe({
-            next: async (changes) => {
-                if (changes.some((change) => change.path.startsWith('api'))) {
-                    this.logger.verbose(`API Config changed ${JSON.stringify(changes)}`);
-                    await this.persistenceHelper.persistIfChanged(this.filePath, this.config);
-                }
-            },
-            error: (err) => {
-                this.logger.error('Error receiving config changes:', err);
-            },
-        });
     }
 
     convertLegacyConfig(

api/src/unraid-api/config/api-config.test.ts

@@ -2,9 +2,26 @@ import { ConfigService } from '@nestjs/config';
 
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
-import { ApiConfigPersistence } from '@app/unraid-api/config/api-config.module.js';
+import { fileExists } from '@app/core/utils/files/file-exists.js';
+import { ApiConfigPersistence, loadApiConfig } from '@app/unraid-api/config/api-config.module.js';
 import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js';
 
+// Mock the core file-exists utility used by ApiStateConfig
+vi.mock('@app/core/utils/files/file-exists.js', () => ({
+    fileExists: vi.fn(),
+}));
+
+// Mock the shared file-exists utility used by ConfigPersistenceHelper
+vi.mock('@unraid/shared/util/file.js', () => ({
+    fileExists: vi.fn(),
+}));
+
+// Mock fs/promises for file I/O operations
+vi.mock('fs/promises', () => ({
+    readFile: vi.fn(),
+    writeFile: vi.fn(),
+}));
+
 describe('ApiConfigPersistence', () => {
     let service: ApiConfigPersistence;
     let configService: ConfigService;
@@ -135,3 +152,127 @@ describe('ApiConfigPersistence', () => {
         });
     });
 });
+
+describe('loadApiConfig', () => {
+    let readFile: any;
+    let writeFile: any;
+
+    beforeEach(async () => {
+        vi.clearAllMocks();
+        // Reset modules to ensure fresh imports
+        vi.resetModules();
+
+        // Get mocked functions
+        const fsMocks = await import('fs/promises');
+        readFile = fsMocks.readFile;
+        writeFile = fsMocks.writeFile;
+    });
+
+    it('should return default config when file does not exist', async () => {
+        vi.mocked(fileExists).mockResolvedValue(false);
+
+        const result = await loadApiConfig();
+
+        expect(result).toEqual({
+            version: expect.any(String),
+            extraOrigins: [],
+            sandbox: false,
+            ssoSubIds: [],
+            plugins: [],
+        });
+    });
+
+    it('should merge disk config with defaults when file exists', async () => {
+        const diskConfig = {
+            extraOrigins: ['https://example.com'],
+            sandbox: true,
+            ssoSubIds: ['sub1', 'sub2'],
+        };
+
+        vi.mocked(fileExists).mockResolvedValue(true);
+        vi.mocked(readFile).mockResolvedValue(JSON.stringify(diskConfig));
+
+        const result = await loadApiConfig();
+
+        expect(result).toEqual({
+            version: expect.any(String),
+            extraOrigins: ['https://example.com'],
+            sandbox: true,
+            ssoSubIds: ['sub1', 'sub2'],
+            plugins: [],
+        });
+    });
+
+    it('should use default config and overwrite file when JSON parsing fails', async () => {
+        const { fileExists: sharedFileExists } = await import('@unraid/shared/util/file.js');
+
+        vi.mocked(fileExists).mockResolvedValue(true);
+        vi.mocked(readFile).mockResolvedValue('{ invalid json }');
+        vi.mocked(sharedFileExists).mockResolvedValue(false); // For persist operation
+        vi.mocked(writeFile).mockResolvedValue(undefined);
+
+        const result = await loadApiConfig();
+
+        // Error logging is handled by NestJS Logger, just verify the config is returned
+        expect(writeFile).toHaveBeenCalled();
+        expect(result).toEqual({
+            version: expect.any(String),
+            extraOrigins: [],
+            sandbox: false,
+            ssoSubIds: [],
+            plugins: [],
+        });
+    });
+
+    it('should handle write failure gracefully when JSON parsing fails', async () => {
+        const { fileExists: sharedFileExists } = await import('@unraid/shared/util/file.js');
+
+        vi.mocked(fileExists).mockResolvedValue(true);
+        vi.mocked(readFile).mockResolvedValue('{ invalid json }');
+        vi.mocked(sharedFileExists).mockResolvedValue(false); // For persist operation
+        vi.mocked(writeFile).mockRejectedValue(new Error('Permission denied'));
+
+        const result = await loadApiConfig();
+
+        // Error logging is handled by NestJS Logger, just verify the config is returned
+        expect(writeFile).toHaveBeenCalled();
+        expect(result).toEqual({
+            version: expect.any(String),
+            extraOrigins: [],
+            sandbox: false,
+            ssoSubIds: [],
+            plugins: [],
+        });
+    });
+
+    it('should use default config when file is empty', async () => {
+        vi.mocked(fileExists).mockResolvedValue(true);
+        vi.mocked(readFile).mockResolvedValue('');
+
+        const result = await loadApiConfig();
+
+        // No error logging expected for empty files
+        expect(result).toEqual({
+            version: expect.any(String),
+            extraOrigins: [],
+            sandbox: false,
+            ssoSubIds: [],
+            plugins: [],
+        });
+    });
+
+    it('should always override version with current API_VERSION', async () => {
+        const diskConfig = {
+            version: 'old-version',
+            extraOrigins: ['https://example.com'],
+        };
+
+        vi.mocked(fileExists).mockResolvedValue(true);
+        vi.mocked(readFile).mockResolvedValue(JSON.stringify(diskConfig));
+
+        const result = await loadApiConfig();
+
+        expect(result.version).not.toBe('old-version');
+        expect(result.version).toBeTruthy();
+    });
+});