Skip to content

Translate SearchIterator C++ exceptions - #297

Open
chapmanjacobd wants to merge 2 commits into
openzim:mainfrom
chapmanjacobd:fix-search-iterator-exceptions
Open

chapmanjacobd wants to merge 2 commits into
openzim:mainfrom
chapmanjacobd:fix-search-iterator-exceptions

Conversation

@chapmanjacobd

@chapmanjacobd chapmanjacobd commented Nov 7, 2026 •

Copy link
Copy Markdown

Problem

The Cython declarations for zim::SearchIterator getters omit except +. If a stale full-text index references an entry that is no longer present, iterating search results lets zim::EntryNotFound escape through the CPython evaluation loop and abort the process.

I reproduced this with the streetzim osm-hawaii-2026-09-26.zim archive and query a; Archive.check() returns true, but the native iterator aborts.

Change

Add Cython exception translation to search-result getters and iteration, archive-entry iteration, suggestion iteration, entry title/redirect properties, and query setters. Exceptions from these calls become catchable Python exceptions rather than escaping into CPython.

Remove unnecessary exception translation from the simple Blob.data(), end(), and size() accessors. Add a minimal changelog entry.

Validation

  • Original SearchIterator fix: pytest -q passed (210 tests, 5 warnings); rebuilding and rerunning the stale-index reproducer raised RuntimeError: Cannot find entry instead of aborting.
  • Expanded annotations: Cython C++ code generation and whitespace checks passed; the full suite and reproducer have not been rerun for the expanded changes.

Declare all SearchIterator getters with Cython exception translation so stale full-text references become Python errors instead of aborting the interpreter.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@benoit74

benoit74 commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

@chapmanjacobd why are you fixing only these 4 methods? I feel like we miss the except + annotation on many places, and would prefer to fix all of them at once, so that it becomes something known to work and worth checking in next PRs.

Please also add a succint entry in CHANGELOG.md

@chapmanjacobd

Copy link
Copy Markdown
Author

Well each one adds a little bit of overhead. I think it makes sense to not be 100% exhaustive. But I'll add a few more where the operations are large and where they are likely needed the most and add a changelog entry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@chapmanjacobd

Copy link
Copy Markdown
Author

Also removed except + from Blob.data(), end(), and size(): these are simple buffer accessors, and dangling-pointer/lifetime bugs are not C++ exceptions, so exception translation would not catch them anyway.

This branch has not been deployed

No deployments
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