fix(notifications): gracefully handle & mask invalid notifications

api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.ts

@@ -0,0 +1,228 @@
+// Unit Test File for NotificationsService: loadNotificationFile
+
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { NotificationIni } from '@app/core/types/states/notification.js';
+import {
+    Notification,
+    NotificationImportance,
+    NotificationType,
+} from '@app/unraid-api/graph/resolvers/notifications/notifications.model.js';
+import { NotificationsService } from '@app/unraid-api/graph/resolvers/notifications/notifications.service.js';
+
+// Only mock getters.dynamix and Logger
+vi.mock('@app/store/index.js', () => ({
+    getters: {
+        dynamix: vi.fn().mockReturnValue({
+            notify: { path: '/test/notifications' },
+            display: {
+                date: 'Y-m-d',
+                time: 'H:i:s',
+            },
+        }),
+    },
+}));
+
+vi.mock('@nestjs/common', async (importOriginal) => {
+    const original = await importOriginal<typeof import('@nestjs/common')>();
+    return {
+        ...original,
+        Logger: vi.fn(() => ({
+            log: vi.fn(),
+            warn: vi.fn(),
+            error: vi.fn(),
+            debug: vi.fn(),
+            verbose: vi.fn(),
+        })),
+    };
+});
+
+describe('NotificationsService - loadNotificationFile (minimal mocks)', () => {
+    let service: NotificationsService;
+
+    beforeEach(() => {
+        service = new NotificationsService();
+    });
+
+    it('should load and validate a valid notification file', async () => {
+        const mockNotificationIni: NotificationIni = {
+            timestamp: '1609459200',
+            event: 'Test Event',
+            subject: 'Test Subject',
+            description: 'Test Description',
+            importance: 'alert',
+            link: 'http://example.com',
+        };
+
+        vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
+            mockNotificationIni
+        );
+
+        const result = await (service as any).loadNotificationFile(
+            '/test/path/test.notify',
+            NotificationType.UNREAD
+        );
+        expect(result).toEqual(
+            expect.objectContaining({
+                id: 'test.notify',
+                type: NotificationType.UNREAD,
+                title: 'Test Event',
+                subject: 'Test Subject',
+                description: 'Test Description',
+                importance: NotificationImportance.ALERT,
+                link: 'http://example.com',
+                timestamp: '2021-01-01T00:00:00.000Z',
+            })
+        );
+    });
+
+    it('should return masked warning notification on validation error (missing required fields)', async () => {
+        const invalidNotificationIni: Omit<NotificationIni, 'event'> = {
+            timestamp: '1609459200',
+            // event: 'Missing Event', // missing required field
+            subject: 'Test Subject',
+            description: 'Test Description',
+            importance: 'alert',
+        };
+
+        vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
+            invalidNotificationIni
+        );
+
+        const result = await (service as any).loadNotificationFile(
+            '/test/path/invalid.notify',
+            NotificationType.UNREAD
+        );
+        expect(result.id).toBe('invalid.notify');
+        expect(result.importance).toBe(NotificationImportance.WARNING);
+        expect(result.description).toContain('invalid and cannot be displayed');
+    });
+
+    it('should handle invalid enum values', async () => {
+        const invalidNotificationIni: NotificationIni = {
+            timestamp: '1609459200',
+            event: 'Test Event',
+            subject: 'Test Subject',
+            description: 'Test Description',
+            importance: 'not-a-valid-enum' as any,
+        };
+
+        vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
+            invalidNotificationIni
+        );
+
+        const result = await (service as any).loadNotificationFile(
+            '/test/path/invalid-enum.notify',
+            NotificationType.UNREAD
+        );
+        expect(result.id).toBe('invalid-enum.notify');
+        // Implementation falls back to INFO for unknown importance
+        expect(result.importance).toBe(NotificationImportance.INFO);
+        // Should not be a masked warning notification, just fallback to INFO
+        expect(result.description).toBe('Test Description');
+    });
+
+    it('should handle missing description field (should return masked warning notification)', async () => {
+        const mockNotificationIni: Omit<NotificationIni, 'description'> = {
+            timestamp: '1609459200',
+            event: 'Test Event',
+            subject: 'Test Subject',
+            importance: 'normal',
+        };
+
+        vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
+            mockNotificationIni
+        );
+
+        const result = await (service as any).loadNotificationFile(
+            '/test/path/test.notify',
+            NotificationType.UNREAD
+        );
+        // Should be a masked warning notification
+        expect(result.description).toContain('invalid and cannot be displayed');
+        expect(result.importance).toBe(NotificationImportance.WARNING);
+    });
+
+    it('should preserve passthrough data from notification file (only known fields)', async () => {
+        const mockNotificationIni: NotificationIni & { customField: string } = {
+            timestamp: '1609459200',
+            event: 'Test Event',
+            subject: 'Test Subject',
+            description: 'Test Description',
+            importance: 'normal',
+            link: 'http://example.com',
+            customField: 'custom value',
+        };
+
+        vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
+            mockNotificationIni
+        );
+
+        const result = await (service as any).loadNotificationFile(
+            '/test/path/test.notify',
+            NotificationType.UNREAD
+        );
+        expect(result).toEqual(
+            expect.objectContaining({
+                link: 'http://example.com',
+                // customField should NOT be present
+                description: 'Test Description',
+                id: 'test.notify',
+                type: NotificationType.UNREAD,
+                title: 'Test Event',
+                subject: 'Test Subject',
+                importance: NotificationImportance.INFO,
+                timestamp: '2021-01-01T00:00:00.000Z',
+            })
+        );
+        expect((result as any).customField).toBeUndefined();
+    });
+
+    it('should handle missing timestamp field gracefully', async () => {
+        const mockNotificationIni: Omit<NotificationIni, 'timestamp'> = {
+            // timestamp is missing
+            event: 'Test Event',
+            subject: 'Test Subject',
+            description: 'Test Description',
+            importance: 'alert',
+        };
+
+        vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
+            mockNotificationIni
+        );
+
+        const result = await (service as any).loadNotificationFile(
+            '/test/path/missing-timestamp.notify',
+            NotificationType.UNREAD
+        );
+        expect(result.id).toBe('missing-timestamp.notify');
+        expect(result.importance).toBe(NotificationImportance.ALERT);
+        expect(result.description).toBe('Test Description');
+        expect(result.timestamp).toBeUndefined(); // Missing timestamp results in undefined
+        expect(result.formattedTimestamp).toBe(undefined); // Also undefined since timestamp is missing
+    });
+
+    it('should handle malformed timestamp field gracefully', async () => {
+        const mockNotificationIni: NotificationIni = {
+            timestamp: 'not-a-timestamp',
+            event: 'Test Event',
+            subject: 'Test Subject',
+            description: 'Test Description',
+            importance: 'alert',
+        };
+
+        vi.spyOn(await import('@app/core/utils/misc/parse-config.js'), 'parseConfig').mockReturnValue(
+            mockNotificationIni
+        );
+
+        const result = await (service as any).loadNotificationFile(
+            '/test/path/malformed-timestamp.notify',
+            NotificationType.UNREAD
+        );
+        expect(result.id).toBe('malformed-timestamp.notify');
+        expect(result.importance).toBe(NotificationImportance.ALERT);
+        expect(result.description).toBe('Test Description');
+        expect(result.timestamp).toBeUndefined(); // Malformed timestamp results in undefined
+        expect(result.formattedTimestamp).toBe('not-a-timestamp'); // Returns original string when parsing fails
+    });
+});

api/src/unraid-api/graph/resolvers/notifications/notifications.service.spec.ts

@@ -1,3 +1,11 @@
+// Integration Test File for NotificationsService
+// ------------------------------------------------
+// This file contains integration-style tests for the NotificationsService.
+// It uses the full NestJS TestingModule, mocks only the minimum required dependencies,
+// and interacts with the real filesystem (in /tmp/test/notifications).
+// These tests cover end-to-end service behavior, including notification creation,
+// archiving, unarchiving, deletion, and legacy CLI compatibility.
+
 import type { TestingModule } from '@nestjs/testing';
 import { Test } from '@nestjs/testing';
 import { existsSync } from 'fs';

api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts

@@ -5,6 +5,7 @@ import { basename, join } from 'path';
 
 import type { Stats } from 'fs';
 import { FSWatcher, watch } from 'chokidar';
+import { ValidationError } from 'class-validator';
 import { execa } from 'execa';
 import { emptyDir } from 'fs-extra';
 import { encode as encodeIni } from 'ini';
@@ -635,10 +636,14 @@ export class NotificationsService {
      * Loads a notification file from disk, parses it to a Notification object, and
      * validates the object against the NotificationSchema.
      *
+     * If the file contains invalid data (doesn't conform to the Notification schema),
+     * instead of throwing, returns a masked warning notification with details masked,
+     * and logs a warning. This allows the system to gracefully handle corrupt or malformed notifications.
+     *
      * @param path The path to the notification file on disk.
      * @param type The type of the notification that is being loaded.
-     * @returns A parsed Notification object, or throws an error if the object is invalid.
-     * @throws An error if the object is invalid (doesn't conform to the graphql NotificationSchema).
+     * @returns A parsed Notification object, or a masked warning notification if invalid.
+     * @throws File system errors (file not found, permission issues) or unexpected validation errors.
      */
     private async loadNotificationFile(path: string, type: NotificationType): Promise<Notification> {
         const notificationFile = parseConfig<NotificationIni>({
@@ -656,8 +661,28 @@ export class NotificationsService {
         // The contents of the file, and therefore the notification, may not always be a valid notification.
         // so we parse it through the schema to make sure it is
 
-        const validatedNotification = await validateObject(Notification, notification);
-        return validatedNotification;
+        try {
+            const validatedNotification = await validateObject(Notification, notification);
+            return validatedNotification;
+        } catch (error) {
+            if (!(error instanceof ValidationError)) {
+                throw error;
+            }
+            const errorsToLog = error.children?.length ? error.children : error;
+            this.logger.warn(errorsToLog, `notification file at ${path} is invalid. Will mask.`);
+            const nameMask = this.getIdFromPath(path);
+            const dateMask = new Date();
+            return {
+                id: nameMask,
+                type,
+                title: nameMask,
+                subject: nameMask,
+                description: `This notification is invalid and cannot be displayed! For details, see the logs and the notification file at ${path}`,
+                importance: NotificationImportance.WARNING,
+                timestamp: dateMask.toISOString(),
+                formattedTimestamp: this.formatDatetime(dateMask),
+            };
+        }
     }
 
     private getIdFromPath(path: string) {
@@ -729,19 +754,22 @@ export class NotificationsService {
     }
 
     private formatTimestamp(timestamp: string) {
-        const { display: settings } = getters.dynamix();
         const date = this.parseNotificationDateToIsoDate(timestamp);
+        if (!date) {
+            this.logger.warn(`[formatTimestamp] Could not parse date from timestamp: ${date}`);
+            return timestamp;
+        }
+        return this.formatDatetime(date);
+    }
 
+    private formatDatetime(date: Date) {
+        const { display: settings } = getters.dynamix();
         if (!settings) {
             this.logger.warn(
                 '[formatTimestamp] Dynamix display settings not found. Cannot apply user settings.'
             );
-            return timestamp;
-        } else if (!date) {
-            this.logger.warn(`[formatTimestamp] Could not parse date from timestamp: ${date}`);
-            return timestamp;
+            return date.toISOString();
         }
-        // this.logger.debug(`[formatTimestamp] ${settings.date} :: ${settings.time} :: ${date}`);
         return formatDatetime(date, {
             dateFormat: settings.date,
             timeFormat: settings.time,