Skip to content

fix(kiwix): self-heal missing library XML#733

Open
cuyua9 wants to merge 1 commit intoCrosstalk-Solutions:devfrom
cuyua9:fix-kiwix-library-xml-self-heal
Open

fix(kiwix): self-heal missing library XML#733
cuyua9 wants to merge 1 commit intoCrosstalk-Solutions:devfrom
cuyua9:fix-kiwix-library-xml-self-heal

Conversation

@cuyua9
Copy link
Copy Markdown

@cuyua9 cuyua9 commented Apr 16, 2026

Summary

  • validate the Kiwix library XML when Kiwix is already in library mode
  • rebuild /storage/zim/kiwix-library.xml from disk when the file is missing or invalid
  • keep the existing legacy migration path unchanged

Validation

  • cd admin && npm run typecheck
  • cd admin && npx eslint providers/kiwix_migration_provider.ts app/services/kiwix_library_service.ts
  • cd admin && npm run build

Copy link
Copy Markdown
Collaborator

@chriscrosstalk chriscrosstalk left a comment

Choose a reason for hiding this comment

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

Thanks @cuyua9 — this is a real defensive gap worth closing. When kiwix-library.xml goes missing (upgrade glitch, manual wipe, third-party forks that don't follow the same migration path), Kiwix silently serves an empty library and users get confused. Rebuilding from disk on startup is exactly the right instinct, and delegating to the existing rebuildFromDisk() keeps the blast radius small.

I retargeted the PR from main to dev — we base all fixes on dev so they flow into the next release cycle. No action needed from you there.

Two things I'd like to see tightened before merge:

1. Drop the cosmetic refactors that aren't part of the fix. The diff has a handful of unrelated style changes piggybacking on the real change:

  • 'fs/promises''node:fs/promises' and 'path''node:path' — these aren't consistent with the rest of the codebase, which uses the unprefixed forms everywhere.
  • const Service = (await import('#models/service')).default unwrapped into two lines
  • Service.query().where(...) split into const query = ...; await query.where(...)
  • A logger.info(...) reformatted from one line to multi-line

None of those change behavior, and they make the diff noisier + hurt git blame for the surrounding lines. Can you revert them so the PR contains only the self-heal logic?

2. Simplify the catch block. In ensureValidLibraryXml:

if (err.code && err.code !== 'ENOENT') {
  throw err
}
if (!err.code && !(err instanceof Error)) {
  throw err
}

The second branch is dead code — everything thrown through here is a standard Error (either from node's readFile or from _validateLibraryXml's new Error(...)). You can drop to:

if (err.code && err.code !== 'ENOENT') throw err
logger.warn(...)
await this.rebuildFromDisk()
return true

Happy to approve once those are cleaned up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants