fix(mods): stop Manage Mods going blank on a null tag from the mod database - #371
Conversation
The detail endpoint returns a null among Vanilla Variants' category tags. The Manage Mods filter derivations folded case on every entry during the first render, so null.toLowerCase() threw, and with no error boundary the whole page went blank for anyone with that mod installed (#370). Every reader in installedFilters.ts now goes through one helper that keeps string entries only and reads a malformed list as empty, so a shape the server does not honour can no longer take the page down.
Zaldaryon
left a comment
There was a problem hiding this comment.
Request changes
The fix correctly filters non-string tag and author entries, and the reported Vanilla Variants payload is covered. One malformed ModDB shape still crashes the same filter path: a null entry inside the otherwise valid releases array. The inline comment identifies the exact dereference and requested correction.
Please also complete the repository template evidence by marking npm run build:unpack in the checklist and adding the Testing and Related issues sections. The remote build and all required checks are green, but the PR description should record the commands and link #370 directly.
Verification
- Local npm ci: passed with 0 reported vulnerabilities.
- Local npm run typecheck: passed.
- Local npm run lint:ci: passed with 15 existing React Hooks warnings and 0 errors.
- Local npm run format:check: passed.
- Local npm run test:coverage: 161 files passed, 2,028 tests passed, 2 skipped; 93.40% statements, 90.20% branches, 92.68% functions, and 94.96% lines.
- Focused installedFilters.test.ts: 27 tests passed.
- Local npm run build:unpack: passed.
- git diff --check: passed.
- Remote run 33960193866 passed typecheck, lint, test, Ubuntu and Windows test matrix, Ubuntu and Windows builds, and SonarCloud. macOS is skipped by repository policy.
The PR remains on the correct dev base and is current with dev. I am not approving or merging until the malformed release entry is handled and the PR evidence is completed.
Guarding the outer releases field let { releases: [null] } through, and
release.tags then threw on the entry: the same render crash this helper
exists to rule out. Each entry is now checked as well.
Zaldaryon
left a comment
There was a problem hiding this comment.
Approved
The follow-up commit 86e2dd7 closes the prior finding. releasesOf() now filters null and non-object entries before either reader accesses release.tags. The regression covers [null, validRelease] through both game-version option discovery and matching, while valid releases remain available. I found no remaining blocking issue.
Verification
npm ci: passed with 0 vulnerabilities reported.npm run typecheck: passed.npm run lint:ci: passed with 0 errors and 15 pre-existing React Hooks warnings.npm run format:check: passed.npx vitest run tests/domain/mods/installedFilters.test.ts: 28 tests passed.npm run test:coverage: 161 files, 2,029 tests passed, 2 skipped; 93.40% statements, 90.20% branches, 92.68% functions, and 94.96% lines.npm run build:unpack: passed on Linux with Electron 44.1.1.git diff --check: passed.- Remote CI run 33963603711 passed typecheck, lint, the aggregate test gate, Ubuntu and Windows test matrix legs, Ubuntu and Windows builds, and SonarCloud. macOS is skipped by repository policy.
The PR targets dev, is current with its base, has no unresolved threads, and the reviewed diff contains no private paths, credentials, generated files, or agent traces.
Part of #370, the blank Manage Mods page reported by Vinni_Pukh, b0b4444 and Villoch on the mod database listing and by a friend of NekoJess on the Discord within eleven hours of beta.7. This is the hotfix for the crash itself; the error boundary (#372) and the boundary validation named in the issue follow separately.
What broke
installedModTagsinsrc/domain/mods/installedFilters.ts, new in #348, collects every_mod.tagsentry across the installed mods and folds case withtoLowerCase()during the first render of the page. The mod database's detail endpoint returns["Cosmetics", "Crafting", "Storage", null]for Vanilla Variants, a very popular mod.null.toLowerCase()threw, and with no error boundary in the renderer, React unmounted the whole tree. Reproduced against the live API, not a fixture:The list endpoint (
/api/mods) is clean across all 8371 mods; the detail endpoint the launcher actually reads is the one that carries the null. The hand-written fixtures followed the documented shape, which is why the suite was green.The fix
One helper,
textEntries, keeps only string entries and reads a field that is not a list as empty, and every reader in the module goes through it: authors, category tags, and each release's game-version tags, on both the option-list side and the matcher side. A second helper,releasesOf, reads the releases list the same way and, after review, also drops entries that are not objects:{ releases: [null] }passed the outer array check andrelease.tagsthen threw on the entry, the same crash class. Nothing in the module trusts the shape any more.Tests
Six cases built from the real shape rather than a sanitised one: a
nullamong the category tags (Vanilla Variants exactly), anullinside a release's tags, a non-string author, atagsfield that is a string, areleasesfield that is a string rather thannull(on purpose,null ?? []reads as empty by accident and the guard has to hold for anything that is not a list), and anullentry inside an otherwise valid releases list.Mutants, each applied alone against the committed tree and restored after:
textEntrieskeeps every entry, no string filterTypeErrorverbatimreleasesOfdrops the array guard for?? []releasescasereleasesOfdrops the per-entry object filter[null]case,Cannot read properties of null (reading 'tags')Testing
On head
86e2dd7:npm run typecheck: passed (node, web, tests).npm run lint:ci: 0 errors, 15 pre-existing React Hooks warnings.npm run format:check: passed.npm run test:coverage: 161 files, 2029 passed, 2 skipped; 93.4% statements, 90.2% branches, 92.68% functions, 94.96% lines locally, floors 87 / 85 / 85 / 89. The CI run on this head is the reference figure if the two differ by a hundredth.npm run build:unpack: passed, Electron 44.1.1 Linux.npx vitest run tests/domain/mods/installedFilters.test.ts: 28 passed.Related issues
Type
Checklist
dev, notmain.npm run typecheckpasses.npm run lint:cipasses.npm run format:checkpasses.npm run test:coveragepasses, coverage at or above the floor invitest.config.ts.npm run build:unpackpasses.