Skip to content

Conversation

@shrrrriya
Copy link

@shrrrriya shrrrriya commented Jan 3, 2026

Fixing #572

This PR improves user-facing error messages in kiwix-manage.

  • Clarifies the message when a requested book ID is not found
  • Adds helpful hints when adding a ZIM file fails, without changing behavior

This follows the discussion about improving error clarity without introducing
exception handling.

Copy link
Contributor

@kelson42 kelson42 left a comment

Choose a reason for hiding this comment

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

@shrrrriya Thank you for your PR. I think it would be great if you implement the code analysing the problem and delivering the precise error message in a second commit. To me so far looks good, but I let @veloman-yunkan making the technical analysis.

Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

Please first enhance kiwix::Manager::addBookFromPathAndGetId() to signal failure by throwing an exception and add unit tests for all reasons of detectable failure (this must be done in the libkiwix repository). Then change this PR to report errors by catching exceptions emerging from that function.

@shrrrriya
Copy link
Author

Thanks for the review and guidance.

I have now updated libkiwix to make
Manager::addBookFromPathAndGetId() throw specific exceptions
(FileNotFound, InvalidZim, LibraryNotWritable) and added unit tests
covering detectable failure cases.

PR: kiwix/libkiwix#1267

I will now update this PR to catch those exceptions and report precise
user-facing errors in kiwix-manage.

@kelson42
Copy link
Contributor

I will now update this PR to catch those exceptions and report precise
user-facing errors in kiwix-manage.

Thank you, don't forget this last part.

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.

3 participants