Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/store/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const loadDynamixConfig = () => {
};

export const getters = {
dynamix: () => loadDynamixConfig(),
dynamix: () => store.getState().dynamix,
emhttp: () => store.getState().emhttp,
paths: () => store.getState().paths,
registration: () => store.getState().registration,
Expand Down
88 changes: 88 additions & 0 deletions api/src/unraid-api/config/dynamix-config-refresh.service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { FileLoadStatus } from '@app/store/types.js';
import { DynamixConfigRefreshService } from '@app/unraid-api/config/dynamix-config-refresh.service.js';

const { dispatch, getState, loadDynamixConfigFromDiskSync, updateDynamixConfig } = vi.hoisted(() => ({
dispatch: vi.fn(),
getState: vi.fn(),
loadDynamixConfigFromDiskSync: vi.fn(),
updateDynamixConfig: vi.fn((payload: unknown) => ({ type: 'dynamix/update', payload })),
}));

vi.mock('@app/store/index.js', () => ({
store: {
dispatch,
getState,
},
}));

vi.mock('@app/store/actions/load-dynamix-config-file.js', () => ({
loadDynamixConfigFromDiskSync,
}));

vi.mock('@app/store/modules/dynamix.js', () => ({
updateDynamixConfig,
}));

describe('DynamixConfigRefreshService', () => {
let service: DynamixConfigRefreshService;

beforeEach(() => {
vi.useFakeTimers();
dispatch.mockReset();
getState.mockReset();
loadDynamixConfigFromDiskSync.mockReset();
updateDynamixConfig.mockClear();

getState.mockReturnValue({
paths: {
'dynamix-config': ['/etc/default.cfg', '/boot/config/plugins/dynamix/dynamix.cfg'],
},
});

service = new DynamixConfigRefreshService();
});

afterEach(() => {
service.onModuleDestroy();
vi.runOnlyPendingTimers();
vi.useRealTimers();
});

it('loads on init and dispatches loaded status', () => {
loadDynamixConfigFromDiskSync.mockReturnValue({ notify: { path: '/tmp/notifications' } });

service.onModuleInit();

expect(loadDynamixConfigFromDiskSync).toHaveBeenCalledWith([
'/etc/default.cfg',
'/boot/config/plugins/dynamix/dynamix.cfg',
]);
expect(updateDynamixConfig).toHaveBeenCalledWith({
notify: { path: '/tmp/notifications' },
status: FileLoadStatus.LOADED,
});
expect(dispatch).toHaveBeenCalledTimes(1);
});

it('skips dispatch when loaded config is unchanged', () => {
loadDynamixConfigFromDiskSync.mockReturnValue({ notify: { path: '/tmp/notifications' } });

service.onModuleInit();
vi.advanceTimersByTime(5000);

expect(dispatch).toHaveBeenCalledTimes(1);
});

it('dispatches failed status when loading throws', () => {
loadDynamixConfigFromDiskSync.mockImplementation(() => {
throw new Error('boom');
});

service.onModuleInit();

expect(updateDynamixConfig).toHaveBeenCalledWith({ status: FileLoadStatus.FAILED_LOADING });
expect(dispatch).toHaveBeenCalledTimes(1);
});
});
58 changes: 58 additions & 0 deletions api/src/unraid-api/config/dynamix-config-refresh.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { Injectable, Logger, OnModuleDestroy, OnModuleInit } from '@nestjs/common';

import { loadDynamixConfigFromDiskSync } from '@app/store/actions/load-dynamix-config-file.js';
import { store } from '@app/store/index.js';
import { updateDynamixConfig } from '@app/store/modules/dynamix.js';
import { FileLoadStatus } from '@app/store/types.js';

const DYNAMIX_REFRESH_INTERVAL_MS = 5000;

@Injectable()
export class DynamixConfigRefreshService implements OnModuleInit, OnModuleDestroy {
private readonly logger = new Logger(DynamixConfigRefreshService.name);
private refreshTimer: NodeJS.Timeout | null = null;
private lastSerializedConfig: string | null = null;

onModuleInit() {
this.refresh();
this.refreshTimer = setInterval(() => {
this.refresh();
}, DYNAMIX_REFRESH_INTERVAL_MS);
}

onModuleDestroy() {
if (!this.refreshTimer) {
return;
}

clearInterval(this.refreshTimer);
this.refreshTimer = null;
}

private refresh() {
const configPaths = store.getState().paths['dynamix-config'] ?? [];

Check failure on line 33 in api/src/unraid-api/config/dynamix-config-refresh.service.ts

View workflow job for this annotation

GitHub Actions / Test API

src/unraid-api/app/__test__/app.module.integration.spec.ts > AppModule Integration Tests

TypeError: Cannot read properties of undefined (reading 'dynamix-config') ❯ DynamixConfigRefreshService.refresh src/unraid-api/config/dynamix-config-refresh.service.ts:33:51 ❯ DynamixConfigRefreshService.onModuleInit src/unraid-api/config/dynamix-config-refresh.service.ts:17:14 ❯ MapIterator.iteratee ../node_modules/.pnpm/@nestjs+core@11.1.6_@nestjs+common@11.1.6_class-transformer@0.5.1_class-validator@0.14._bc2bc0d32b3785c29eaf84bdeadd843f/node_modules/@nestjs/core/hooks/on-module-init.hook.js:22:43 ❯ MapIterator.next ../node_modules/.pnpm/iterare@1.2.1/node_modules/iterare/src/map.ts:9:39 ❯ IteratorWithOperators.next ../node_modules/.pnpm/iterare@1.2.1/node_modules/iterare/src/iterate.ts:19:28 ❯ IteratorWithOperators.toArray ../node_modules/.pnpm/iterare@1.2.1/node_modules/iterare/src/iterate.ts:227:22 ❯ callOperator ../node_modules/.pnpm/@nestjs+core@11.1.6_@nestjs+common@11.1.6_class-transformer@0.5.1_class-validator@0.14._bc2bc0d32b3785c29eaf84bdeadd843f/node_modules/@nestjs/core/hooks/on-module-init.hook.js:23:10 ❯ callModuleInitHook ../node_modules/.pnpm/@nestjs+core@11.1.6_@nestjs+common@11.1.6_class-transformer@0.5.1_class-validator@0.14._bc2bc0d32b3785c29eaf84bdeadd843f/node_modules/@nestjs/core/hooks/on-module-init.hook.js:43:23 ❯ Proxy.callInitHook ../node_modules/.pnpm/@nestjs+core@11.1.6_@nestjs+common@11.1.6_class-transformer@0.5.1_class-validator@0.14._bc2bc0d32b3785c29eaf84bdeadd843f/node_modules/@nestjs/core/nest-application-context.js:242:50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Guard paths before indexing to prevent startup crash.
Line 33 can throw when store.getState().paths is undefined (matches CI failure), which breaks module init.

💡 Proposed fix
-    private refresh() {
-        const configPaths = store.getState().paths['dynamix-config'] ?? [];
-
-        try {
+    private refresh() {
+        try {
+            const configPaths = store.getState().paths?.['dynamix-config'] ?? [];
             const config = loadDynamixConfigFromDiskSync(configPaths);
             const serializedConfig = JSON.stringify(config);
🧰 Tools
🪛 GitHub Actions: CI - Main (API)

[error] 33-33: Cannot read properties of undefined (reading 'dynamix-config') during DynamixConfigRefreshService.refresh invoked on AppModule initialization (AppModule Integration Tests).

🪛 GitHub Check: Test API

[failure] 33-33: src/unraid-api/app/test/app.module.integration.spec.ts > AppModule Integration Tests
TypeError: Cannot read properties of undefined (reading 'dynamix-config')
❯ DynamixConfigRefreshService.refresh src/unraid-api/config/dynamix-config-refresh.service.ts:33:51
❯ DynamixConfigRefreshService.onModuleInit src/unraid-api/config/dynamix-config-refresh.service.ts:17:14
❯ MapIterator.iteratee ../node_modules/.pnpm/@nestjs+core@11.1.6_@nestjs+common@11.1.6_class-transformer@0.5.1_class-validator@0.14.bc2bc0d32b3785c29eaf84bdeadd843f/node_modules/@nestjs/core/hooks/on-module-init.hook.js:22:43
❯ MapIterator.next ../node_modules/.pnpm/iterare@1.2.1/node_modules/iterare/src/map.ts:9:39
❯ IteratorWithOperators.next ../node_modules/.pnpm/iterare@1.2.1/node_modules/iterare/src/iterate.ts:19:28
❯ IteratorWithOperators.toArray ../node_modules/.pnpm/iterare@1.2.1/node_modules/iterare/src/iterate.ts:227:22
❯ callOperator ../node_modules/.pnpm/@nestjs+core@11.1.6
@nestjs+common@11.1.6_class-transformer@0.5.1_class-validator@0.14.bc2bc0d32b3785c29eaf84bdeadd843f/node_modules/@nestjs/core/hooks/on-module-init.hook.js:23:10
❯ callModuleInitHook ../node_modules/.pnpm/@nestjs+core@11.1.6
@nestjs+common@11.1.6_class-transformer@0.5.1_class-validator@0.14.bc2bc0d32b3785c29eaf84bdeadd843f/node_modules/@nestjs/core/hooks/on-module-init.hook.js:43:23
❯ Proxy.callInitHook ../node_modules/.pnpm/@nestjs+core@11.1.6
@nestjs+common@11.1.6_class-transformer@0.5.1_class-validator@0.14._bc2bc0d32b3785c29eaf84bdeadd843f/node_modules/@nestjs/core/nest-application-context.js:242:50

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/src/unraid-api/config/dynamix-config-refresh.service.ts` at line 33,
Guard access to store.getState().paths before indexing to avoid crashes when
paths is undefined: replace the direct indexing in the assignment to configPaths
with a safe access (e.g. use optional chaining or a default object). Concretely,
update the line that assigns configPaths (currently using
store.getState().paths['dynamix-config'] ?? []) to something like
store.getState().paths?.['dynamix-config'] ?? [] or first capture const paths =
store.getState().paths ?? {} and then read paths['dynamix-config'] ?? []; this
change in dynamix-config-refresh.service.ts (the assignment creating
configPaths) will prevent startup errors when paths is undefined.


try {
const config = loadDynamixConfigFromDiskSync(configPaths);
const serializedConfig = JSON.stringify(config);
if (serializedConfig === this.lastSerializedConfig) {
return;
Comment on lines +38 to +39

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Re-dispatch loaded status after transient refresh failures

The unchanged-config guard returns before any status update, which means a transient read error can permanently leave dynamix.status as FAILED_LOADING: the catch block dispatches failure but keeps lastSerializedConfig, and when the same config content becomes readable again, this early return skips the LOADED dispatch. In that scenario, consumers that gate behavior on FileLoadStatus.LOADED will stay in an error state until the file contents actually change.

Useful? React with 👍 / 👎.

}
Comment on lines +38 to +40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reset dedupe state after errors so successful recovery dispatches LOADED.
With current logic, a failure can leave state stuck as FAILED_LOADING if the next successful config matches the previous serialized value.

💡 Proposed fix
         } catch (error) {
             this.logger.error(error, 'Failed to refresh dynamix config from disk');
             store.dispatch(
                 updateDynamixConfig({
                     status: FileLoadStatus.FAILED_LOADING,
                 })
             );
+            this.lastSerializedConfig = null;
         }

Also applies to: 49-55

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/src/unraid-api/config/dynamix-config-refresh.service.ts` around lines 38
- 40, The dedupe check using lastSerializedConfig prevents dispatching LOADED
after a prior FAILED_LOADING when the succeeding config is identical; fix by not
updating lastSerializedConfig on failures and by clearing or nulling
lastSerializedConfig in error paths so identical successful payloads re-trigger
processing. Specifically, in the block comparing serializedConfig to
lastSerializedConfig and in the error/catch handling (references:
serializedConfig, lastSerializedConfig, and the FAILED_LOADING / LOADED
dispatches), ensure lastSerializedConfig is only assigned after successful
handling (after dispatching LOADED) and that any catch/failure path resets
lastSerializedConfig (or leaves it unset) so a later successful match will not
be deduped.


store.dispatch(
updateDynamixConfig({
...config,
status: FileLoadStatus.LOADED,
})
);
this.lastSerializedConfig = serializedConfig;
} catch (error) {
this.logger.error(error, 'Failed to refresh dynamix config from disk');
store.dispatch(
updateDynamixConfig({
status: FileLoadStatus.FAILED_LOADING,
})
);
}
}
}
5 changes: 3 additions & 2 deletions api/src/unraid-api/config/legacy-config.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ConfigModule } from '@nestjs/config';

import { apiConfig } from '@app/unraid-api/config/api-config.module.js';
import { loadAppEnvironment, loadLegacyStore } from '@app/unraid-api/config/config.loader.js';
import { DynamixConfigRefreshService } from '@app/unraid-api/config/dynamix-config-refresh.service.js';
import { StoreSyncService } from '@app/unraid-api/config/store-sync.service.js';

@Module({
Expand All @@ -14,7 +15,7 @@ import { StoreSyncService } from '@app/unraid-api/config/store-sync.service.js';
load: [loadAppEnvironment, loadLegacyStore, apiConfig],
}),
],
providers: [StoreSyncService],
exports: [StoreSyncService],
providers: [StoreSyncService, DynamixConfigRefreshService],
exports: [StoreSyncService, DynamixConfigRefreshService],
})
export class LegacyConfigModule {}
Loading