fix: always close PDF in _convert_pdf_to_images; narrow bare except in SentenceSplitter (#12942)

Co-authored-by: Timur Rakhmatullin <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Julian Risch <[email protected]>
Co-authored-by: David S. Batista <[email protected]>

haystack/components/converters/image/image_utils.py

@@ -162,54 +162,55 @@ def _convert_pdf_to_images(
 
     resolved_page_range = page_range or range(1, num_pages + 1)
 
-    for page_number in resolved_page_range:
-        if page_number < 1 or page_number > num_pages:
-            logger.warning(
-                "Page {page_number} is out of range for the PDF file {file_path}. Skipping it.",
-                page_number=page_number,
-                file_path=bytestream.meta.get("file_path"),
-            )
-            continue
+    try:
+        for page_number in resolved_page_range:
+            if page_number < 1 or page_number > num_pages:
+                logger.warning(
+                    "Page {page_number} is out of range for the PDF file {file_path}. Skipping it.",
+                    page_number=page_number,
+                    file_path=bytestream.meta.get("file_path"),
+                )
+                continue
 
-        # Get dimensions of the page
-        page = pdf[max(page_number - 1, 0)]  # Adjust for 0-based indexing
-        _, _, width, height = page.get_mediabox()
+            # Get dimensions of the page
+            page = pdf[max(page_number - 1, 0)]  # Adjust for 0-based indexing
+            _, _, width, height = page.get_mediabox()
 
-        target_resolution_dpi = 300.0
+            target_resolution_dpi = 300.0
 
-        # From pypdfium2 docs: scale (float) – A factor scaling the number of pixels per PDF canvas unit. This defines
-        # the resolution of the image. To convert a DPI value to a scale factor, multiply it by the size of 1 canvas
-        # unit in inches (usually 1/72in).
-        # https://pypdfium2.readthedocs.io/en/stable/python_api.html#pypdfium2._helpers.page.PdfPage.render
-        target_scale = target_resolution_dpi / 72.0
+            # From pypdfium2 docs: scale (float) – A factor scaling the number of pixels per PDF canvas unit. This
+            # defines the resolution of the image. To convert a DPI value to a scale factor, multiply it by the size of
+            # 1 canvas unit in inches (usually 1/72in).
+            # https://pypdfium2.readthedocs.io/en/stable/python_api.html#pypdfium2._helpers.page.PdfPage.render
+            target_scale = target_resolution_dpi / 72.0
 
-        # Calculate potential pixels for target_dpi
-        pixels_for_target_scale = width * height * target_scale**2
+            # Calculate potential pixels for target_dpi
+            pixels_for_target_scale = width * height * target_scale**2
 
-        pil_max_pixels = PILImage.MAX_IMAGE_PIXELS or int(1024 * 1024 * 1024 // 4 // 3)
-        # 90% of PIL's default limit to prevent borderline cases
-        pixel_limit = pil_max_pixels * 0.9
+            pil_max_pixels = PILImage.MAX_IMAGE_PIXELS or int(1024 * 1024 * 1024 // 4 // 3)
+            # 90% of PIL's default limit to prevent borderline cases
+            pixel_limit = pil_max_pixels * 0.9
 
-        scale = target_scale
-        if pixels_for_target_scale > pixel_limit:
-            logger.info(
-                "Large PDF detected ({pixels:.2f} pixels). Resizing the image to fit the pixel limit.",
-                pixels=pixels_for_target_scale,
-            )
-            scale = (pixel_limit / (width * height)) ** 0.5
+            scale = target_scale
+            if pixels_for_target_scale > pixel_limit:
+                logger.info(
+                    "Large PDF detected ({pixels:.2f} pixels). Resizing the image to fit the pixel limit.",
+                    pixels=pixels_for_target_scale,
+                )
+                scale = (pixel_limit / (width * height)) ** 0.5
 
-        pdf_bitmap = page.render(scale=scale)
+            pdf_bitmap = page.render(scale=scale)
 
-        image: "Image" = pdf_bitmap.to_pil()
-        pdf_bitmap.close()
-        if size is not None:
-            # Set reducing_gap=None to disable multi-step shrink; better quality.
-            # https://pillow.readthedocs.io/en/latest/reference/Image.html#PIL.Image.Image.thumbnail
-            image.thumbnail(size=size, reducing_gap=None)
+            image: "Image" = pdf_bitmap.to_pil()
+            pdf_bitmap.close()
+            if size is not None:
+                # Set reducing_gap=None to disable multi-step shrink; better quality.
+                # https://pillow.readthedocs.io/en/latest/reference/Image.html#PIL.Image.Image.thumbnail
+                image.thumbnail(size=size, reducing_gap=None)
 
-        all_pdf_images.append((page_number, image))
-
-    pdf.close()
+            all_pdf_images.append((page_number, image))
+    finally:
+        pdf.close()
 
     if return_base64:
         return [

haystack/components/preprocessors/sentence_tokenizer.py

@@ -103,7 +103,7 @@ if nltk_imports.is_successful():
             """
             try:
                 return self._re_period_context  # type: ignore
-            except:  # noqa: E722
+            except AttributeError:
                 self._re_period_context = re.compile(
                     self._period_context_fmt
                     % {

releasenotes/notes/close-pdf-on-image-conversion-error-df5231752de05e51.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Converting PDF pages to images, as ``PDFToImageContent`` and ``DocumentToImageContent`` do, now closes the
+    pypdfium2 document even when rendering a page raises an error. Before, the document was only closed after all
+    pages rendered successfully, so a failing page left it open until garbage collection.

test/components/converters/image/test_image_utils.py

@@ -96,6 +96,22 @@ class TestConvertPdfToImages:
 
         assert "Large PDF detected" in caplog.text
 
+    def test_convert_pdf_to_images_closes_pdf_when_rendering_fails(self) -> None:
+        bytestream = get_bytestream_from_source(Path("test/test_files/pdf/sample_pdf_1.pdf"))
+
+        mock_pdf_document = MagicMock()
+        mock_pdf_document.__len__.return_value = 1
+        mock_page = MagicMock()
+        mock_page.get_mediabox.return_value = (0, 0, 100, 100)
+        mock_page.render.side_effect = RuntimeError("render failed")
+        mock_pdf_document.__getitem__.return_value = mock_page
+
+        with patch("haystack.components.converters.image.image_utils.PdfDocument", return_value=mock_pdf_document):
+            with pytest.raises(RuntimeError, match="render failed"):
+                _convert_pdf_to_images(bytestream=bytestream, page_range=[1])
+
+        mock_pdf_document.close.assert_called_once()
+
 
 class TestEncodeImageToBase64:
     def test_encode_image_to_base64(self) -> None:

test/components/preprocessors/test_sentence_tokenizer.py

@@ -4,12 +4,17 @@
 
 import time
 from pathlib import Path
+from typing import Any
 from unittest.mock import patch
 
 import pytest
 from pytest import LogCaptureFixture
 
-from haystack.components.preprocessors.sentence_tokenizer import QUOTE_SPANS_RE, SentenceSplitter
+from haystack.components.preprocessors.sentence_tokenizer import (
+    QUOTE_SPANS_RE,
+    CustomPunktLanguageVars,
+    SentenceSplitter,
+)
 
 
 def test_apply_split_rules_no_join() -> None:
@@ -187,3 +192,14 @@ def test_split_sentences_performance() -> None:
     end = time.time()
 
     assert end - start < 2, f"Execution time exceeded 2 seconds: {end - start:.2f} seconds"
+
+
+def test_period_context_re_does_not_swallow_unexpected_errors() -> None:
+    class FailingLanguageVars(CustomPunktLanguageVars):
+        def __getattr__(self, name: str) -> Any:
+            if name == "_re_period_context":
+                raise RuntimeError("unexpected")
+            raise AttributeError(name)
+
+    with pytest.raises(RuntimeError, match="unexpected"):
+        FailingLanguageVars().period_context_re()