fix: invalid configs no longer crash API #1491
Conversation
WalkthroughThe changes introduce enhanced error handling and logging for configuration file parsing and loading in the API's configuration modules. Additionally, the allowed Bash commands in the local settings file are updated to permit linting of the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ApiConfigModule
participant ApiStateConfig
App->>ApiConfigModule: loadApiConfig()
ApiConfigModule->>ApiStateConfig: parseConfig()
alt Config file empty
ApiStateConfig-->>ApiConfigModule: Log warning, return undefined
else Config file invalid JSON
ApiStateConfig-->>ApiConfigModule: Log error, throw exception
ApiConfigModule-->>ApiConfigModule: Catch, log error, set diskConfig undefined
else Config file valid
ApiStateConfig-->>ApiConfigModule: Return parsed config
end
ApiConfigModule-->>App: Return merged config (with defaults)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/unraid-api/config/api-config.module.ts (1)
46-52: Good error handling, but consider using NestJS Logger for consistency.The try-catch logic correctly prevents crashes and provides graceful fallback to defaults. However, using
console.erroris inconsistent with the rest of the codebase that uses NestJS Logger.Consider using NestJS Logger for consistency:
+import { Logger } from '@nestjs/common'; + export const loadApiConfig = async () => { + const logger = new Logger('loadApiConfig'); const defaultConfig = createDefaultConfig(); // ... existing code ... try { diskConfig = await apiConfig.parseConfig(); } catch (error) { - console.error('Failed to load API config from disk, using defaults:', error); + logger.error('Failed to load API config from disk, using defaults:', error); diskConfig = undefined; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.claude/settings.local.json(1 hunks)api/src/unraid-api/config/api-config.module.ts(1 hunks)api/src/unraid-api/config/factory/api-state.model.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`api/src/unraid-api/**/*`: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
api/src/unraid-api/**/*: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.module.tsapi/src/unraid-api/config/factory/api-state.model.ts
`**/*.{ts,tsx}`: TypeScript imports use .js extensions for ESM compatibility
**/*.{ts,tsx}: TypeScript imports use .js extensions for ESM compatibility
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.module.tsapi/src/unraid-api/config/factory/api-state.model.ts
`api/src/unraid-api/**/*`: Prefer adding new files to the Nest repo located at api/src/unraid-api/ instead of the legacy code
api/src/unraid-api/**/*: Prefer adding new files to the Nest repo located at api/src/unraid-api/ instead of the legacy code
📄 Source: CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.module.tsapi/src/unraid-api/config/factory/api-state.model.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Use pnpm ONLY for package management in the API package
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-30T14:50:52.281Z
Learning: Always run scripts from api/package.json unless requested otherwise
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Always run scripts from api/package.json unless requested
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the @UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Run tests with: pnpm --filter ./api test
Learnt from: pujitm
PR: unraid/api#1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Learnt from: elibosley
PR: unraid/api#1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations, including detailed error logging with stack traces and modification IDs. Individual FileModification implementations should focus on their core functionality without duplicating error handling.
.claude/settings.local.json (10)
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Run tests with: pnpm --filter ./api test
Learnt from: elibosley
PR: unraid/api#1120
File: plugin/package.json:0-0
Timestamp: 2025-02-05T14:43:48.568Z
Learning: In Node.js projects, npm scripts should be organized with clear namespacing (e.g., build:*, docker:*, env:*) and include proper environment validation and error handling. Each script should follow the single responsibility principle.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to web/__test__/**/* : Tests are located under web/__test__, run with pnpm test
Learnt from: mdatelle
PR: unraid/api#1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Always run scripts from api/package.json unless requested
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Use pnpm ONLY for package management in the API package
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-30T14:50:52.281Z
Learning: Always run scripts from api/package.json unless requested otherwise
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to api/**/*.{test,spec}.{ts,js} : Test suite is VITEST, do not use jest in the API package
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-30T14:50:52.281Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Test suite is VITEST, do not use jest
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Always run pnpm codegen for GraphQL code generation in the web directory
api/src/unraid-api/config/api-config.module.ts (10)
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations. It includes try/catch blocks, detailed error logging, and safe rollback mechanisms. Individual FileModification implementations (like LogRotateModification) should allow errors to propagate to this service layer rather than handling them internally.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Learnt from: elibosley
PR: unraid/api#1063
File: api/src/unraid-api/cli/sso/validate-token.command.ts:76-81
Timestamp: 2025-01-23T21:26:17.766Z
Learning: In the unraid/api codebase, store dispatches using Redux Toolkit's `unwrap()` method already handle errors appropriately and don't require additional try-catch blocks.
Learnt from: elibosley
PR: unraid/api#1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: In the Unraid API, FileModification implementations (apply/rollback methods) don't need to implement their own error handling as it's handled by the UnraidFileModifierService caller.
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations, including detailed error logging with stack traces and modification IDs. Individual FileModification implementations should focus on their core functionality without duplicating error handling.
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: In the Unraid API, FileModification implementations (like LogRotateModification) don't need to handle errors internally as error handling is managed at the UnraidFileModifierService level.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Learnt from: pujitm
PR: unraid/api#1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
🧬 Code Graph Analysis (1)
api/src/unraid-api/config/factory/api-state.model.ts (1)
api/src/unraid-api/cli/log.service.ts (1)
error(46-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
.claude/settings.local.json (1)
13-13: LGTM: Linting permission added correctly.The new permission aligns with the established pattern of using pnpm for package management and supports linting the API package.
api/src/unraid-api/config/factory/api-state.model.ts (1)
79-95: Excellent error handling and logging enhancements.The improvements provide robust handling of empty files and detailed error logging for JSON parsing failures. The debug logging with file content substring will be valuable for troubleshooting configuration issues.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/src/unraid-api/config/api-config.module.ts(1 hunks)api/src/unraid-api/config/api-config.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/config/api-config.module.ts
🧰 Additional context used
📓 Path-based instructions (6)
`api/src/unraid-api/**/*`: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
api/src/unraid-api/**/*: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.test.ts
`api/**/*.{test,spec}.{ts,js}`: Test suite is VITEST, do not use jest in the API package
api/**/*.{test,spec}.{ts,js}: Test suite is VITEST, do not use jest in the API package
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.test.ts
`**/*.{ts,tsx}`: TypeScript imports use .js extensions for ESM compatibility
**/*.{ts,tsx}: TypeScript imports use .js extensions for ESM compatibility
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.test.ts
`api/src/unraid-api/**/*`: Prefer adding new files to the Nest repo located at api/src/unraid-api/ instead of the legacy code
api/src/unraid-api/**/*: Prefer adding new files to the Nest repo located at api/src/unraid-api/ instead of the legacy code
📄 Source: CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.test.ts
`api/**/*.{test,spec}.{js,ts,tsx}`: Test suite is VITEST, do not use jest Prefer to not mock simple dependencies in tests
api/**/*.{test,spec}.{js,ts,tsx}: Test suite is VITEST, do not use jest
Prefer to not mock simple dependencies in tests
📄 Source: CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.test.ts
`**/*.test.ts`: Use `mount` from Vue Test Utils for component testing Stub compl...
**/*.test.ts: Usemountfrom Vue Test Utils for component testing
Stub complex child components that aren't the focus of the test
Mock external dependencies and services in tests
Verify that the expected elements are rendered in component tests
Test component interactions (clicks, inputs, etc.) in tests
Check for expected prop handling and event emissions in component tests
UsecreateTestingPinia()for mocking stores in component tests
Use semantic queries likefind('button')orfind('[data-test="id"]')but prefer not to use data test IDs
Find components withfindComponent(ComponentName)in tests
UsefindAllto check for multiple elements in component tests
Assert on rendered text content withwrapper.text()
Assert on element attributes withelement.attributes()
Verify element existence withexpect(element.exists()).toBe(true)
Check component state through rendered output in tests
Trigger events withawait element.trigger('click')in component tests
Set input values withawait input.setValue('value')in component tests
Test emitted events withwrapper.emitted()in component tests
Mock external services and API calls in tests
Usevi.mock()for module-level mocks in tests
Specify return values for component methods withvi.spyOn()in tests
Reset mocks between tests withvi.clearAllMocks()
Useawait nextTick()for DOM updates in async tests
UseflushPromises()for more complex promise chains in async tests
Always await async operations before making assertions in tests
Don't rely on Nuxt auto-imports in tests
Place all mock declarations at the top level in test files
Use factory functions for module mocks to avoid hoisting issues in test files
Don't mix mock declarations and module mocks incorrectly in test files
Avoid relying on Nuxt's auto-imports in test environment
Clear mocks between tests to ensure isolation
Remember thatvi.mock()calls are hoisted
📄 Source: CodeRabbit Inference Engine (.cursor/rules/web-testing-rules.mdc)
List of files the instruction was applied to:
api/src/unraid-api/config/api-config.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Run tests with: pnpm --filter ./api test
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Use pnpm ONLY for package management in the API package
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-30T14:50:52.281Z
Learning: Always run scripts from api/package.json unless requested otherwise
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Always run scripts from api/package.json unless requested
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify proper error handling in store tests
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to web/__test__/**/* : Tests are located under web/__test__, run with pnpm test
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the @UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Learnt from: pujitm
PR: unraid/api#1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
api/src/unraid-api/config/api-config.test.ts (19)
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Mock external services and API calls in tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Use factory functions for module mocks to avoid hoisting issues in test files
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify state changes by updating the store in store tests
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to api/src/unraid-api/**/* : Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to api/**/*.{test,spec}.{ts,js} : Test suite is VITEST, do not use jest in the API package
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify state changes after actions in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Test action side effects and state changes in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to web/test/mocks/**/*.ts : Frequently used mocks are stored under `web/test/mocks`
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify proper error handling in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Mock external dependencies and services in tests
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Use `vi.mock()` for module-level mocks in tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-30T14:50:52.281Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Prefer to not mock simple dependencies in tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Remember that `vi.mock()` calls are hoisted
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-30T14:50:52.281Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Test suite is VITEST, do not use jest
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Test getter dependencies are properly mocked in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Reset mocks between tests with `vi.clearAllMocks()`
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Specify return values for component methods with `vi.spyOn()` in tests
🧬 Code Graph Analysis (1)
api/src/unraid-api/config/api-config.test.ts (2)
api/src/core/utils/files/file-exists.ts (1)
fileExists(5-8)api/src/unraid-api/config/api-config.module.ts (1)
loadApiConfig(35-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
api/src/unraid-api/config/api-config.test.ts (7)
151-157: Good test setup with proper mock management.The beforeEach block properly clears mocks, mocks console.error, and resets modules for isolation between tests.
159-171: Comprehensive test for non-existent config file scenario.The test properly mocks
fileExiststo return false and verifies that the default configuration is returned with expected structure.
173-192: Well-structured test for valid config file merging.The test properly simulates an existing file with valid JSON and verifies that disk config is merged with defaults while preserving the version override behavior.
194-220: Excellent error handling test with file overwrite verification.This test comprehensively verifies the error handling path when JSON parsing fails, including:
- Error logging verification
- File overwrite attempt
- Success logging verification
- Default config fallback
222-248: Good coverage of write failure scenario.The test properly simulates both JSON parsing failure and subsequent write failure, verifying graceful error handling and appropriate logging.
250-264: Proper handling of empty file scenario.The test verifies that empty files don't trigger error logging and return default configuration, which aligns with the enhanced error handling mentioned in the AI summary.
266-279: Important test for version override behavior.This test ensures that the version field is always overridden with the current API_VERSION, regardless of what's in the disk config, which is critical for API versioning consistency.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/unraid-api/plugin/plugin.service.ts (1)
68-74: Good graceful degradation, but use NestJS Logger for consistency.The try-catch block properly handles
loadApiConfigfailures and allows the method to continue with an empty plugin list. However, use the existing NestJS Logger instead ofconsole.errorfor consistency with the rest of the service.- } catch (error) { - console.error('Failed to load API config for plugin discovery, using empty list:', error); - } + } catch (error) { + PluginService.logger.error('Failed to load API config for plugin discovery, using empty list:', error); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
api/src/core/utils/misc/parse-config.ts(1 hunks)api/src/unraid-api/config/api-config.module.ts(1 hunks)api/src/unraid-api/config/api-config.test.ts(2 hunks)api/src/unraid-api/plugin/plugin.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- api/src/unraid-api/config/api-config.module.ts
- api/src/unraid-api/config/api-config.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.{ts,tsx}`: TypeScript imports use .js extensions for ESM compatibility
**/*.{ts,tsx}: TypeScript imports use .js extensions for ESM compatibility
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/core/utils/misc/parse-config.tsapi/src/unraid-api/plugin/plugin.service.ts
`api/src/unraid-api/**/*`: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
api/src/unraid-api/**/*: Prefer adding new files to the NestJS repo located at api/src/unraid-api/ instead of the legacy code
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
api/src/unraid-api/plugin/plugin.service.ts
`api/src/unraid-api/**/*`: Prefer adding new files to the Nest repo located at api/src/unraid-api/ instead of the legacy code
api/src/unraid-api/**/*: Prefer adding new files to the Nest repo located at api/src/unraid-api/ instead of the legacy code
📄 Source: CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
List of files the instruction was applied to:
api/src/unraid-api/plugin/plugin.service.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Run tests with: pnpm --filter ./api test
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Use pnpm ONLY for package management in the API package
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-30T14:50:52.281Z
Learning: Always run scripts from api/package.json unless requested otherwise
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Always run scripts from api/package.json unless requested
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify proper error handling in store tests
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to web/__test__/**/* : Tests are located under web/__test__, run with pnpm test
Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the @UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Learnt from: pujitm
PR: unraid/api#1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.
api/src/core/utils/misc/parse-config.ts (1)
Learnt from: elibosley
PR: unraid/api#963
File: web/helpers/markdown.ts:1-13
Timestamp: 2024-11-18T20:52:47.800Z
Learning: All configurations and direct usage of `marked` should be centralized in the `safeParseMarkdown` function in `web/helpers/markdown.ts` to maintain security and consistency.
api/src/unraid-api/plugin/plugin.service.ts (7)
Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.
Learnt from: elibosley
PR: unraid/api#1211
File: api/src/unraid-api/graph/connect/connect-settings.service.ts:48-53
Timestamp: 2025-03-13T16:17:55.820Z
Learning: In NestJS services, dynamic imports should be wrapped in try/catch blocks with proper error logging and fallback handling to prevent unhandled rejections from crashing the application.
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Learnt from: pujitm
PR: unraid/api#1352
File: api/src/unraid-api/cli/dependency.service.ts:114-117
Timestamp: 2025-04-21T18:25:16.491Z
Learning: Error masking through try/catch blocks is not desirable for the `npmInstall` method in `DependencyService` as errors should propagate to the caller.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify proper error handling in store tests
Learnt from: elibosley
PR: unraid/api#1120
File: plugin/plugins/dynamix.unraid.net.plg:35-38
Timestamp: 2025-02-05T21:10:48.136Z
Learning: When providing error handling guidance for Unraid plugins, direct users to use the web GUI (Plugins > Installed Plugins) rather than suggesting command-line actions.
Learnt from: pujitm
PR: unraid/api#1352
File: api/src/unraid-api/cli/dependency.service.ts:114-117
Timestamp: 2025-04-21T18:25:16.491Z
Learning: Methods in `DependencyService` like `npmInstall` and `rebuildVendorArchive` intentionally propagate errors from execa calls upward rather than handling them internally, allowing callers to implement appropriate error handling. This follows the pattern documented in the `rebuildVendorArchive` method's docstring.
🧬 Code Graph Analysis (1)
api/src/core/utils/misc/parse-config.ts (1)
api/src/core/errors/app-error.ts (1)
AppError(4-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Web App
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
api/src/core/utils/misc/parse-config.ts (1)
127-134: Excellent error handling enhancement.The try-catch block properly handles
parseInifailures and provides clear error context. The error message format appropriately handles both Error instances and string errors.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.9.2](v4.9.1...v4.9.2) (2025-07-09) ### Bug Fixes * invalid configs no longer crash API ([#1491](#1491)) ([6bf3f77](6bf3f77)) * invalid state for unraid plugin ([#1492](#1492)) ([39b8f45](39b8f45)) * release note escaping ([5b6bcb6](5b6bcb6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
Bug Fixes
Chores
./apipackage using a filtered command.Tests