Uh oh!
There was an error while loading. Please reload this page.
fix(api): prevent flash notification startup fd exhaustion - #1893
Conversation
WalkthroughAdds batched file I/O (max 32 concurrent), an initialization/hydration flow that ensures notification directories exist and queues watcher adds during hydration, improves ENOENT/missing-file handling, and updates tests with chokidar/fs mocks and cleanup utilities. No exported API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Tests
participant Service as NotificationsService
participant FS as FileSystem
participant Watcher as ChokidarWatcher
Test->>Service: initializeNotificationsState(basePath)
Service->>FS: ensureNotificationDirectories(basePath)
FS-->>Service: dirs created/exist
Service->>FS: list files (batched stat)
FS-->>Service: file list & stats
Service->>Service: buildOverviewSnapshot (load files batched)
Service-->>Test: overview published
Service->>Watcher: create watch(path, { ignoreInitial: true })
Watcher-->>Service: on('add') events (during hydration) -> queued
Service->>Service: after hydration -> replay queued adds
Service->>FS: load added files (batched, skip ENOENT)
FS-->>Service: file contents / ENOENT errors
Service->>Service: recalculateOverview -> publish
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:0c0c6d5a34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (1)
279-284: Biome static analysis: forEach callback should not return a value.
Set.add()returns the Set, so the forEach callback implicitly returns a value. While functionally correct, this violates the lint rule. Use a for-of loop or add braces to make the return void explicit.♻️ Proposed fix
const recalculate = async (type: NotificationType) => { const ids = await this.listFilesInFolder(this.paths()[type]); - ids.forEach((id) => seenPaths.add(id));+ for (const id of ids) {+ seenPaths.add(id);+ } const [notifications] = await this.loadNotificationsFromPaths(ids, {}); notifications.forEach((n) => this.increment(n.importance, overview[type.toLowerCase()])); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts` around lines 279 - 284, The forEach callback in recalculate implicitly returns the Set because seenPaths.add(id) returns a value, violating the lint rule; change the ids.forEach((id) => seenPaths.add(id)); line to an explicit void-returning form such as a for-of loop: for (const id of ids) { seenPaths.add(id); } so the callback does not return a value—update the recalculate function (and keep the subsequent this.loadNotificationsFromPaths and this.increment usage unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts`:
- Around line 279-284: The forEach callback in recalculate implicitly returns
the Set because seenPaths.add(id) returns a value, violating the lint rule;
change the ids.forEach((id) => seenPaths.add(id)); line to an explicit
void-returning form such as a for-of loop: for (const id of ids) {
seenPaths.add(id); } so the callback does not return a value—update the
recalculate function (and keep the subsequent this.loadNotificationsFromPaths
and this.increment usage unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 467c0971-7546-47a1-a532-2e8014b78c9f
📒 Files selected for processing (2)
api/src/unraid-api/graph/resolvers/notifications/loadNotificationsFile.test.tsapi/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
Uh oh!
There was an error while loading. Please reload this page.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #1893 +/- ##
==========================================
+ Coverage 49.25% 49.73% +0.48%
==========================================
Files 1020 1021 +1 Lines 69765 69966 +201 Branches 7254 7383 +129 ==========================================
+ Hits 34364 34799 +435 + Misses 35278 35044 -234
Partials 123 123 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🤖 I have created a release *beep* *boop* --- ## [4.30.0](v4.29.2...v4.30.0) (2026-03-18) ### Features * add internal boot step to onboarding flow ([#1881](#1881)) ([337aecc](337aecc)) * Add TPM licensing availability to registration ([#1908](#1908)) ([aa162eb](aa162eb)) * add UPS power ([#1874](#1874)) ([b531aed](b531aed)) * **api:** alert when usb boot has internal boot target ([#1898](#1898)) ([b94df47](b94df47)) * **api:** expose internal boot devices in array GraphQL ([#1894](#1894)) ([0736709](0736709)) * docker overview ([#1855](#1855)) ([9ef1cf1](9ef1cf1)) * **docker:** add update actions to container context menu ([#1867](#1867)) ([4ca3e06](4ca3e06)) * **docker:** disable containers page file modification ([#1870](#1870)) ([aaa0372](aaa0372)) * issues/1597: Temperature Monitoring - Thanks @MitchellThompkins ([a1be458](a1be458)) * New Crowdin updates ([#1809](#1809)) ([a7b3f07](a7b3f07)) * New Crowdin updates ([#1883](#1883)) ([14a8fa8](14a8fa8)) * **onboarding:** add new onboarding flows for Unraid OS ([#1746](#1746)) ([15bd747](15bd747)) * registration and trial actions use Account app ([#1928](#1928)) ([c2c0425](c2c0425)) * share internal boot state ([#1921](#1921)) ([8e4d44d](8e4d44d)) * **web:** show TPM move control for trial licenses ([#1911](#1911)) ([d00fb63](d00fb63)) ### Bug Fixes * Add dedicated TPM license move option ([#1909](#1909)) ([36c56f7](36c56f7)) * allow free USB targets in onboarding internal boot setup ([#1903](#1903)) ([298da54](298da54)) * API key key display truncation ([#1890](#1890)) ([b12f75c](b12f75c)) * **api:** harden PHP wrapper args for newer PHP versions ([#1901](#1901)) ([849f177](849f177)) * **api:** prevent flash notification startup fd exhaustion ([#1893](#1893)) ([4b231ad](4b231ad)) * clear stale onboarding modal session state ([#1904](#1904)) ([23f7836](23f7836)) * consistently clear onboarding draft ([#1916](#1916)) ([199d803](199d803)) * correct graphql-api.log timestamp formatting ([#1918](#1918)) ([243c5a8](243c5a8)) * **deps:** pin dependencies ([#1878](#1878)) ([db88eb8](db88eb8)) * **docker:** change "visit" to "webui" & use correct link ([#1863](#1863)) ([cab0880](cab0880)) * **docker:** improve start/stop UX with visual feedback ([#1865](#1865)) ([c084e25](c084e25)) * **docker:** remove aggressive caching to ensure data correctness ([#1864](#1864)) ([1c1bae8](1c1bae8)) * **docker:** sync template mappings in organizer to prevent false orphan warnings ([#1866](#1866)) ([38a6f0c](38a6f0c)) * onboarding internal-boot warning panel contrast and semantics ([#1927](#1927)) ([bb6f241](bb6f241)) * **onboarding:** add explicit EFI loader path for flash entry ([#1926](#1926)) ([429b438](429b438)) * **onboarding:** extend onboarding refresh timeout ([#1925](#1925)) ([e2a5f44](e2a5f44)) * **onboarding:** persist installed plugins in summary ([#1915](#1915)) ([07f4ebd](07f4ebd)) * **onboarding:** refine storage boot setup UX ([#1900](#1900)) ([1108d0a](1108d0a)) * polish onboarding flow ([#1902](#1902)) ([8742cac](8742cac)) * preserve registration device limits after refresh ([#1905](#1905)) ([234bfc7](234bfc7)) * prevent onboarding on API errors ([#1917](#1917)) ([540d6f9](540d6f9)) * remap TPM guid prefix to 01 ([#1924](#1924)) ([5360b5b](5360b5b)) * Return null for corrupted/invalid API key files and add Connect fixtures test ([#1886](#1886)) ([013e6c5](013e6c5)) * share internal boot state across onboarding ([#1920](#1920)) ([f9b293f](f9b293f)) * too many file descriptors with thousands of notifications ([#1887](#1887)) ([7956987](7956987)) * Treat onboarding patch updates as completed ([#1884](#1884)) ([d03b25e](d03b25e)) * unify onboarding internal boot state refresh ([#1923](#1923)) ([d3032c1](d3032c1)) * **web:** refresh internal boot onboarding state ([#1913](#1913)) ([1ca2129](1ca2129)) * **web:** stop showing callback errors after successful key installs ([#1892](#1892)) ([45f1402](45f1402)) --- 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
Testing
Notes
pnpm --filter @unraid/api exec tsc --noEmitstill fails on an existing unrelatedTEMPERATURE_METRICStype error insrc/unraid-api/graph/resolvers/metrics/metrics.resolver.ts.Summary by CodeRabbit
Bug Fixes
Performance
Tests