diff --git a/app/pybind_parse.cpp b/app/pybind_parse.cpp index e5193c14d..d622da707 100644 --- a/app/pybind_parse.cpp +++ b/app/pybind_parse.cpp @@ -848,6 +848,23 @@ PYBIND11_MODULE(pdf_parsers, m) { Returns: List[str]: A list of keys for the currently loaded documents.)") + + .def("take_load_error", + [](docling::docling_parser &self, const std::string &key) -> std::map { + return self.take_load_error(key); + }, + pybind11::arg("key"), + R"( + Return and clear why the last load of `key` failed. + + Parameters: + key (str): The key passed to the failed load call. + + Returns: + dict[str, str]: {"category": ..., "message": ...}, or an empty dict when + no failure is recorded for the key. category is one of "password", + "damaged", "unsupported", "io" and "unknown"; message is the text of + the underlying (qpdf) error.)") .def("load_document", [](docling::docling_parser &self, @@ -1116,6 +1133,23 @@ PYBIND11_MODULE(pdf_parsers, m) { Returns: bool: True if the document was successfully loaded.)") + .def("take_load_error", + [](docling::docling_threaded_parser& self, const std::string& key) -> std::map { + return self.take_load_error(key); + }, + pybind11::arg("key"), + R"( + Return and clear why the last load of `key` failed. + + Parameters: + key (str): The key passed to the failed load call. + + Returns: + dict[str, str]: {"category": ..., "message": ...}, or an empty dict when + no failure is recorded for the key. category is one of "password", + "damaged", "unsupported", "io" and "unknown"; message is the text of + the underlying (qpdf) error.)") + .def("load_document_from_bytesio", [](docling::docling_threaded_parser& self, const std::string& key, diff --git a/docling_parse/pdf_parser.py b/docling_parse/pdf_parser.py index e42d71737..d6c1857ec 100644 --- a/docling_parse/pdf_parser.py +++ b/docling_parse/pdf_parser.py @@ -3,7 +3,7 @@ import hashlib import logging import math -from enum import IntEnum +from enum import Enum, IntEnum from io import BytesIO from pathlib import Path from typing import Any, Dict, Iterator, List, Optional, Sequence, Tuple, Union @@ -77,6 +77,78 @@ _log = logging.getLogger(__name__) +class PdfLoadErrorCategory(str, Enum): + """Why docling-parse could not load a document. + + The values are the categories the native loader reports, see + `src/parse/qpdf/load_error.h`. + """ + + PASSWORD = "password" + """The document is encrypted and the password is missing or wrong.""" + DAMAGED = "damaged" + """qpdf could not repair the file.""" + UNSUPPORTED = "unsupported" + """The file uses a feature qpdf does not implement.""" + IO = "io" + """The file is missing or unreadable.""" + UNKNOWN = "unknown" + + +class PdfLoadError(RuntimeError): + """docling-parse could not load a document. + + A RuntimeError, so handlers written for the former generic error keep + working, and the message keeps its former prefix. `category` says what + went wrong; `detail` is the message of the underlying (qpdf) failure. + """ + + def __init__( + self, + key: str, + category: PdfLoadErrorCategory, + detail: str, + *, + reason: str | None = None, + ): + self.key = key + self.category = category + self.detail = detail + + message = f"Failed to load document with key {key}" + description = reason or detail + if description: + message = f"{message}: {description}" + super().__init__(message) + + +class PdfPasswordError(PdfLoadError): + """The document is encrypted and was opened without a valid password. + + `password_supplied` tells whether the caller gave a (wrong) password or + none at all: qpdf reports both as the same invalid-password error. + """ + + def __init__(self, key: str, detail: str, *, password_supplied: bool): + self.password_supplied = password_supplied + if password_supplied: + reason = "the supplied password is incorrect" + else: + reason = "the document is encrypted and requires a password" + super().__init__(key, PdfLoadErrorCategory.PASSWORD, detail, reason=reason) + + +def _load_error(native_parser, key: str, password: str | None) -> PdfLoadError: + """Build the exception for a failed load from the native parser's record.""" + info = native_parser.take_load_error(key) + category = PdfLoadErrorCategory(info.get("category", PdfLoadErrorCategory.UNKNOWN)) + detail = info.get("message", "") + + if category is PdfLoadErrorCategory.PASSWORD: + return PdfPasswordError(key, detail, password_supplied=password is not None) + return PdfLoadError(key, category, detail) + + class PdfAnnotations(BaseModel): """PDF document annotations including form fields, language, metadata, and table of contents. @@ -1004,7 +1076,14 @@ def load( decode_config: DecodeConfig | None = None, content_config: ContentConfig | None = None, ) -> PdfDocument: + """Load a document and return a handle to its pages. + Raises: + PdfPasswordError: The document is encrypted and `password` is + missing or wrong. + PdfLoadError: The document could not be loaded for another + reason; see its `category` and `detail`. + """ if isinstance(path_or_stream, str): path_or_stream = Path(path_or_stream) @@ -1048,7 +1127,7 @@ def load( return result_doc else: - raise RuntimeError(f"Failed to load document with key {key}") + raise _load_error(self.parser, key, password) def _load_document( self, @@ -1571,6 +1650,12 @@ def load( Returns: str: The document key. + + Raises: + PdfPasswordError: The document is encrypted and `password` is + missing or wrong. + PdfLoadError: The document could not be loaded for another + reason; see its `category` and `detail`. """ if page_numbers is not None and page_range is not None: raise ValueError("page_numbers and page_range are mutually exclusive") @@ -1608,7 +1693,7 @@ def load( ) if not success: - raise RuntimeError(f"Failed to load document with key {key}") + raise _load_error(self._parser, key, password) self._page_counts[key] = self._parser.number_of_pages(key) self._scheduled_page_counts[key] = self._parser.scheduled_number_of_pages(key) diff --git a/src/parse/pdf_decoders/document.h b/src/parse/pdf_decoders/document.h index aa6d21840..21459d33b 100644 --- a/src/parse/pdf_decoders/document.h +++ b/src/parse/pdf_decoders/document.h @@ -11,8 +11,10 @@ #include #include #include +#include #include +#include #include #include @@ -34,6 +36,12 @@ namespace pdflib std::string get_filename() { return filename; } + // Why the last process_document_* call returned false: a category + // from load_error and the message of the underlying failure. Both + // are empty while no load has failed. + const std::string& get_load_error_category() const { return load_error_category; } + const std::string& get_load_error_message() const { return load_error_message; } + nlohmann::json get_annotations(); nlohmann::json get_meta_xml(); @@ -83,6 +91,8 @@ namespace pdflib // object the file does not contain (docling-parse#374). void ensure_page_content_present(int page_number); + void record_load_error(const std::string& category, const std::string& message); + void update_timings(pdf_timings& timings_, bool set_timer); private: @@ -112,6 +122,9 @@ namespace pdflib // and read by the worker threads afterwards. bool qpdf_reported_damage; + std::string load_error_category; + std::string load_error_message; + nlohmann::json json_annots; bool annots_loaded; @@ -235,6 +248,7 @@ namespace pdflib std::ifstream ifs(fs_path, std::ios::binary | std::ios::ate); if(!ifs.is_open()) { + record_load_error(load_error::IO, "could not open file: " + filename); LOG_S(ERROR) << "could not open file: " << filename; return false; } @@ -245,6 +259,7 @@ namespace pdflib auto file_buffer = std::make_shared(file_size, '\0'); if(!ifs.read(file_buffer->data(), file_size)) { + record_load_error(load_error::IO, "could not read file: " + filename); LOG_S(ERROR) << "could not read file: " << filename; return false; } @@ -270,6 +285,9 @@ namespace pdflib buffer = _buffer; password = _password; + load_error_category.clear(); + load_error_message.clear(); + LOG_S(INFO) << "start processing buffer of size " << buffer->size() << " by qpdf ..."; utils::timer timer; @@ -308,8 +326,16 @@ namespace pdflib LOG_S(INFO) << "#-pages: " << number_of_pages; } + catch(const QPDFExc & exc) + { + // qpdf says why: an invalid password, a file it could not repair, ... + record_load_error(load_error::category_of(exc.getErrorCode()), exc.what()); + LOG_S(ERROR) << "could not process buffer by qpdf: " << exc.what(); + return false; + } catch(const std::exception & exc) { + record_load_error(load_error::UNKNOWN, exc.what()); LOG_S(ERROR) << "could not process buffer by qpdf: " << exc.what(); return false; } @@ -321,6 +347,13 @@ namespace pdflib return true; } + void pdf_decoder::record_load_error(const std::string& category, + const std::string& message) + { + load_error_category = category; + load_error_message = message; + } + std::pair > pdf_decoder::get_thread_safe_page_buffer( int page_ind, bool keep_qpdf_warnings) diff --git a/src/parse/qpdf/load_error.h b/src/parse/qpdf/load_error.h new file mode 100644 index 000000000..9c272f90b --- /dev/null +++ b/src/parse/qpdf/load_error.h @@ -0,0 +1,36 @@ +//-*-C++-*- + +#ifndef QPDF_LOAD_ERROR_H +#define QPDF_LOAD_ERROR_H + +#include + +#include + +namespace pdflib +{ + // Why a document could not be loaded. The strings cross into Python as + // docling_parse.pdf_parser.PdfLoadErrorCategory, so they change together. + namespace load_error + { + constexpr const char* PASSWORD = "password"; // encrypted; password missing or wrong + constexpr const char* DAMAGED = "damaged"; // qpdf could not repair the file + constexpr const char* UNSUPPORTED = "unsupported"; // a feature qpdf does not implement + constexpr const char* IO = "io"; // file missing or unreadable + constexpr const char* UNKNOWN = "unknown"; + + inline std::string category_of(qpdf_error_code_e code) + { + switch(code) + { + case qpdf_e_password: return PASSWORD; + case qpdf_e_damaged_pdf: return DAMAGED; + case qpdf_e_unsupported: return UNSUPPORTED; + case qpdf_e_system: return IO; + default: return UNKNOWN; + } + } + } +} + +#endif diff --git a/src/pybind/docling_parser.h b/src/pybind/docling_parser.h index 8d82ec732..9a724048e 100644 --- a/src/pybind/docling_parser.h +++ b/src/pybind/docling_parser.h @@ -5,7 +5,10 @@ #include #include +#include #include +#include +#include #ifdef _WIN32 #include #include @@ -53,6 +56,10 @@ namespace docling int number_of_pages(std::string key); + // Return and clear why the last load of `key` failed: + // {"category": ..., "message": ...}, or an empty map. + std::map take_load_error(std::string key); + nlohmann::json get_annotations(std::string key); nlohmann::json get_meta_xml(std::string key); @@ -76,6 +83,9 @@ namespace docling page_decoder_ptr_type find_page_decoder(const std::string& key, int page); void add_page_decoder(const std::string& key, int page, page_decoder_ptr_type page_decoder); void remove_page_decoders(const std::string& key); + void record_load_error(const std::string& key, + const std::string& category, + const std::string& message); void remove_page_decoder(const std::string& key, int page); void trim_page_decoders(); @@ -84,6 +94,10 @@ namespace docling std::string pdf_resources_dir; std::unordered_map doc_decoders; + + // Failed loads by key, until Python collects the reason. + std::unordered_map> load_errors; + std::list page_decoders; int max_concurrent_results; std::atomic total_processed_pages{0}; @@ -151,11 +165,34 @@ namespace docling return (doc_decoders.count(key)==1); } + std::map docling_parser::take_load_error(std::string key) + { + std::map result; + + auto itr = load_errors.find(key); + if(itr != load_errors.end()) + { + result = itr->second; + load_errors.erase(itr); + } + + return result; + } + + void docling_parser::record_load_error(const std::string& key, + const std::string& category, + const std::string& message) + { + load_errors[key] = {{"category", category}, {"message", message}}; + } + bool docling_parser::load_document(std::string key, std::string filename, std::optional password, bool keep_qpdf_warnings) { + load_errors.erase(key); + #ifdef _WIN32 // Convert UTF-8 string to UTF-16 wstring std::wstring_convert > converter; @@ -177,6 +214,9 @@ namespace docling { // do not keep a decoder that never parsed the document: it would // report is_loaded()=true and a number_of_pages of -1 + record_load_error(key, + doc_decoders.at(key)->get_load_error_category(), + doc_decoders.at(key)->get_load_error_message()); doc_decoders.erase(key); LOG_S(ERROR) << "could not process document for key=" << key; return false; @@ -185,6 +225,7 @@ namespace docling return true; } + record_load_error(key, pdflib::load_error::IO, "file not found: " + filename); LOG_S(ERROR) << "File not found: " << filename; return false; } @@ -203,6 +244,8 @@ namespace docling throw std::runtime_error("Expected a BytesIO object"); } + load_errors.erase(key); + // Seek to the beginning of the BytesIO stream bytes_io.attr("seek")(0); @@ -226,6 +269,9 @@ namespace docling { // do not keep a decoder that never parsed the document: it would // report is_loaded()=true and a number_of_pages of -1 + record_load_error(key, + doc_decoders.at(key)->get_load_error_category(), + doc_decoders.at(key)->get_load_error_message()); doc_decoders.erase(key); LOG_S(ERROR) << "could not decode bytesio object for key="< take_load_error(std::string key); + nlohmann::json get_annotations(std::string key) const; bool unload_document(std::string key); @@ -91,6 +97,9 @@ namespace docling std::optional> page_numbers, std::optional> page_range) const; void validate_unload_state() const; + void record_load_error(const std::string& key, + const std::string& category, + const std::string& message); void reset_after_completion(); void build_task_queue(); @@ -108,6 +117,9 @@ namespace docling std::unordered_map key2doc; std::unordered_map> key2scheduled_pages; + // Failed loads by key, until Python collects the reason. + std::unordered_map> load_errors; + // Task queue: (doc_key, page_number) pairs std::queue> task_queue; std::mutex task_mutex; @@ -212,6 +224,8 @@ namespace docling return false; } + load_errors.erase(key); + #ifdef _WIN32 std::wstring_convert> converter; std::wstring wide_filename = converter.from_bytes(filename); @@ -232,6 +246,9 @@ namespace docling { // do not keep a decoder that never parsed the document: it // would schedule zero pages and report a page count of -1 + record_load_error(key, + key2doc.at(key)->get_load_error_category(), + key2doc.at(key)->get_load_error_message()); key2doc.erase(key); LOG_S(ERROR) << "could not decode file object for key=" << key; return false; @@ -239,6 +256,7 @@ namespace docling } catch(const std::exception& exc) { + record_load_error(key, pdflib::load_error::UNKNOWN, exc.what()); key2doc.erase(key); LOG_S(ERROR) << "could not decode file object for key=" << key; return false; @@ -261,6 +279,7 @@ namespace docling return true; } + record_load_error(key, pdflib::load_error::IO, "file not found: " + filename); LOG_S(ERROR) << "File not found: " << filename; return false; } @@ -279,6 +298,8 @@ namespace docling return false; } + load_errors.erase(key); + LOG_S(INFO) << __FILE__ << ":" << __LINE__ << "\t" << __FUNCTION__; if(not pybind11::hasattr(bytes_io, "read")) @@ -304,6 +325,9 @@ namespace docling { // do not keep a decoder that never parsed the document: it would // schedule zero pages and report a page count of -1 + record_load_error(key, + key2doc.at(key)->get_load_error_category(), + key2doc.at(key)->get_load_error_message()); key2doc.erase(key); key2scheduled_pages.erase(key); LOG_S(ERROR) << "could not decode bytesio object for key=" << key; @@ -312,6 +336,7 @@ namespace docling } catch(const std::exception& exc) { + record_load_error(key, pdflib::load_error::UNKNOWN, exc.what()); key2doc.erase(key); key2scheduled_pages.erase(key); LOG_S(ERROR) << "could not decode bytesio object for key=" << key; @@ -335,6 +360,30 @@ namespace docling return true; } + template + std::map + docling_threaded_base::take_load_error(std::string key) + { + std::map result; + + auto itr = load_errors.find(key); + if(itr != load_errors.end()) + { + result = itr->second; + load_errors.erase(itr); + } + + return result; + } + + template + void docling_threaded_base::record_load_error(const std::string& key, + const std::string& category, + const std::string& message) + { + load_errors[key] = {{"category", category}, {"message", message}}; + } + template int docling_threaded_base::number_of_pages(std::string key) const { diff --git a/tests/test_unit_load_failure_reason.py b/tests/test_unit_load_failure_reason.py new file mode 100644 index 000000000..445d09a51 --- /dev/null +++ b/tests/test_unit_load_failure_reason.py @@ -0,0 +1,154 @@ +#!/usr/bin/env python +"""A failed load must say why, so a caller can tell a password from a broken file. + +qpdf reports why it refused a document: an invalid password, a file it could +not repair, an unsupported feature. The loaders discarded that and every +failure surfaced as the same `RuntimeError("Failed to load document with key +...")`, so docling's default backend could not distinguish an encrypted PDF +from a corrupt one (docling#4829). + +The parsers now keep qpdf's error code and message when a load fails and +raise PdfLoadError, or PdfPasswordError for an encrypted document. Both are +subclasses of RuntimeError, so handlers written for the old error keep +working, and the message keeps its old prefix. +""" + +from io import BytesIO + +import pytest + +from docling_parse.pdf_parser import ( + DoclingPdfParser, + DoclingThreadedPdfParser, + PdfLoadError, + PdfLoadErrorCategory, + PdfPasswordError, +) +from tests.pdf_builder import simple_page_pdf + +GARBAGE = b"%PDF-1.4\nthis is not a valid pdf at all" + + +@pytest.fixture(autouse=True) +def quiet_logging(): + """Silence the qpdf/loguru error stream of the deliberately broken fixtures.""" + DoclingPdfParser(loglevel="fatal") + + +def _plain_pdf() -> bytes: + return simple_page_pdf( + "BT /F1 12 Tf 20 100 Td (ok) Tj ET", + resources="/Font << /F1 5 0 R >>", + extra_objects=["<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>"], + ) + + +def _encrypted_pdf(password: str) -> bytes: + pypdf = pytest.importorskip("pypdf", reason="pypdf writes the encrypted fixture") + + writer = pypdf.PdfWriter(clone_from=BytesIO(_plain_pdf())) + writer.encrypt(password) + + out = BytesIO() + writer.write(out) + return out.getvalue() + + +def _plain_parser(): + return DoclingPdfParser(loglevel="fatal") + + +def _threaded_parser(): + return DoclingThreadedPdfParser() + + +PARSERS = pytest.mark.parametrize( + "make_parser", [_plain_parser, _threaded_parser], ids=["plain", "threaded"] +) + + +def _load(parser, pdf: bytes, password: str | None = None): + return parser.load(BytesIO(pdf), password=password) + + +def _page_count(parser, loaded) -> int: + if isinstance(parser, DoclingThreadedPdfParser): + return parser.page_count(loaded) + return loaded.number_of_pages() + + +@PARSERS +def test_encrypted_document_without_password(make_parser): + parser = make_parser() + + with pytest.raises(PdfPasswordError) as exc_info: + _load(parser, _encrypted_pdf("secret")) + + exc = exc_info.value + assert exc.category is PdfLoadErrorCategory.PASSWORD + assert exc.password_supplied is False + assert "requires a password" in str(exc) + assert exc.detail # qpdf's own wording is kept for diagnostics + + +@PARSERS +def test_encrypted_document_with_wrong_password(make_parser): + parser = make_parser() + + with pytest.raises(PdfPasswordError) as exc_info: + _load(parser, _encrypted_pdf("secret"), password="not-it") + + exc = exc_info.value + assert exc.password_supplied is True + assert "incorrect" in str(exc) + + +@PARSERS +def test_encrypted_document_with_the_right_password_loads(make_parser): + parser = make_parser() + + loaded = _load(parser, _encrypted_pdf("secret"), password="secret") + + assert _page_count(parser, loaded) == 1 + + +@PARSERS +def test_damaged_document_is_not_mistaken_for_a_password_problem(make_parser): + parser = make_parser() + + with pytest.raises(PdfLoadError) as exc_info: + _load(parser, GARBAGE) + + exc = exc_info.value + assert not isinstance(exc, PdfPasswordError) + assert exc.category is PdfLoadErrorCategory.DAMAGED + assert exc.detail + # the old generic prefix and type survive for existing handlers + assert isinstance(exc, RuntimeError) + assert str(exc).startswith("Failed to load document with key ") + + +@PARSERS +def test_missing_file_is_an_io_failure(make_parser, tmp_path): + parser = make_parser() + missing = tmp_path / "missing.pdf" + + with pytest.raises(PdfLoadError) as exc_info: + parser.load(str(missing)) + + exc = exc_info.value + assert exc.category is PdfLoadErrorCategory.IO + assert "missing.pdf" in str(exc) + + +@PARSERS +def test_a_failure_does_not_taint_the_next_load_of_the_same_bytes(make_parser): + parser = make_parser() + pdf = _encrypted_pdf("secret") + + with pytest.raises(PdfPasswordError): + _load(parser, pdf) + + loaded = _load(parser, pdf, password="secret") + + assert _page_count(parser, loaded) == 1