Skip to content

[Port to dtq-dev] Log unparsable-PDF filter-media errors as WARN, not ERROR - #1409

Merged
milanmajchrak merged 1 commit into
dtq-devfrom
903-be/filtermedia-pdf-warn
Aug 14, 2026
Merged

[Port to dtq-dev] Log unparsable-PDF filter-media errors as WARN, not ERROR#1409
milanmajchrak merged 1 commit into
dtq-devfrom
903-be/filtermedia-pdf-warn

Conversation

@jr-rk

@jr-rk jr-rk commented Aug 11, 2026

Copy link
Copy Markdown

Problem description

The nightly filter-media job logs thousands of ERROR lines (~4,655/night on the source instance) when it meets malformed/corrupt PDFs: PDFBoxThumbnail and TikaTextExtractionFilter let the parse IOException escape, and MediaFilterServiceImpl re-logs it at ERROR, tripping log-based alerting even though the job already skips the file and continues.

Analysis

Cherry-pick of customer/TUL commit 7035a4c. Both filters now catch the parse IOException, log at WARN with the item handle, and return null so the bitstream is skipped cleanly. The diff matches the TUL commit except one context line (dtq-dev has e.printStackTrace(System.err)).

Known limits, same as on TUL: an assetstore read IOException on these two paths is downgraded too, and the textextractor.use-temp-file Tika path still throws on a corrupt PDF. Vanilla has no fix for this on any branch.

Manual Testing (if applicable)

  • Added to testing scenarios (dspace-customers issue 55)

Copilot review

  • Requested review from Copilot

… ERROR

A corrupt/malformed PDF makes PDFBoxThumbnail and TikaTextExtractionFilter
throw a parse IOException that MediaFilterServiceImpl re-logs at ERROR,
flooding the nightly filter-media job (~4,655 lines/night) and tripping
log-based alerting, even though the job already skips the file and
continues. Catch the IOException in both filters, log at WARN and return
null so the bitstream is skipped cleanly.

(cherry picked from commit 7035a4c)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts DSpace’s media filtering behavior so that malformed/corrupt PDFs (and other unparsable bitstreams) are treated as data-quality issues: they are logged at WARN and skipped, rather than bubbling up as ERROR and triggering alerting during nightly filter-media runs.

Changes:

  • Update TikaTextExtractionFilter to fully read the bitstream first (preserving real assetstore read failures), and downgrade parse failures to WARN + skip.
  • Update PDFBoxThumbnail to downgrade PDFBox parse failures to WARN + skip (instead of failing the filter-media run).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
dspace-api/src/main/java/org/dspace/app/mediafilter/TikaTextExtractionFilter.java Buffers input before parsing so assetstore IO errors still propagate; downgrades parse failures to WARN and skips extraction.
dspace-api/src/main/java/org/dspace/app/mediafilter/PDFBoxThumbnail.java Downgrades PDFBox parse failures to WARN and skips thumbnail generation for malformed PDFs.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread dspace-api/src/main/java/org/dspace/app/mediafilter/TikaTextExtractionFilter.java Outdated
@jr-rk

jr-rk commented Aug 12, 2026

Copy link
Copy Markdown
Author

Addressed the review feedback in faf6d8a:

  • PDFBoxThumbnail — read the bitstream fully before Loader.loadPDF, so genuine assetstore read failures still propagate as errors; only PDF parse/render failures are downgraded to WARN + skip (now matches the Tika path).
  • Tika OOM — the buffering read is wrapped in the existing OutOfMemoryError handling (extracted to handleOutOfMemory), so an OOM during the read still emits the textextractor.use-temp-file guidance before rethrowing.
  • Test — added corrupt.pdf + testGetDestinationStreamSkipsCorruptFile, asserting a malformed bitstream is skipped (null), not thrown.
  • Refs — inline comments aligned to dspace-customers#903.

Validation: mvn -pl dspace-api checkstyle:check test-compile → 0 violations, BUILD SUCCESS. Runtime of the filter suite is CI-validated (test kernel does not bootstrap in a bare offline single-module checkout). All three Copilot threads replied to and resolved.

@jr-rk

jr-rk commented Aug 13, 2026

Copy link
Copy Markdown
Author

Follow-up (8e471a2): a critical review flagged that the textextractor.use-temp-file branch was not covered — it streamed the source straight into parser.parse(), so a corrupt PDF there still threw and reproduced the same flood on memory-constrained instances. Fixed by buffering the bitstream to a temp input file first (a genuine assetstore read failure surfaces from the copy and propagates), then parsing the local copy and catching IOException/TikaException as an unparsable-document failure → WARN + skip. Stays off-heap for large files, matching the in-memory read/parse split.

Known limitations left as follow-ups (out of scope for this log-noise fix): ImageMagickPdfThumbnailFilter has no corrupt-PDF catch (if enabled instead of PDFBox JPEG Thumbnail, it can still ERROR); no skipped-bitstream counter/telemetry; downgrade is scoped to PDF-parsing filters. Alerting must key on log level (these are WARN), not an error substring.

@jr-rk jr-rk self-assigned this Aug 13, 2026
@jr-rk
jr-rk requested a lite review from Copilot August 13, 2026 07:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

dspace-api/src/main/java/org/dspace/app/mediafilter/TikaTextExtractionFilter.java:222

  • The current catch (IOException | TikaException) wraps the entire try-with-resources, so IO failures opening/creating the temp files (e.g., temp dir permissions/disk full) get downgraded to WARN+skip as if the document were malformed. Also, when returning null on parse failure the extracted-text temp file is left behind until JVM exit (or forever in verbose mode). Narrow the catch to just the parser.parse(...) call and delete tempExtractedTextFile when skipping.
            AutoDetectParser parser = new AutoDetectParser();
            Metadata metadata = new Metadata();
            // parse the buffered copy using the above custom handler
            parser.parse(bufferedSource, handler, metadata);
        } catch (IOException | TikaException e) {

dspace-api/src/main/java/org/dspace/app/mediafilter/TikaTextExtractionFilter.java:163

  • Files.copy(source, ...) can throw before the try/catch/finally below, which means tempSourceFile won’t be deleted until JVM exit (and on long-running filter-media runs this can accumulate temp files). Add cleanup on copy failure so the buffered source temp file is removed immediately when the copy fails.

This issue also appears on line 218 of the same file.

        File tempSourceFile = File.createTempFile("dspacetextsource" + source.hashCode(), ".bin");
        tempSourceFile.deleteOnExit();
        Files.copy(source, tempSourceFile.toPath(), StandardCopyOption.REPLACE_EXISTING);

        File tempExtractedTextFile = File.createTempFile("dspacetextextract" + source.hashCode(), ".txt");

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

dspace-api/src/main/java/org/dspace/app/mediafilter/TikaTextExtractionFilter.java:94

  • New behavior returns null (skips extraction) on IOException, but there is no unit test asserting a malformed/corrupt input is skipped rather than throwing. Since this change is intended to reduce ERROR spam from corrupt PDFs, please add a test + fixture (e.g., truncated PDF) that triggers the IOException path and asserts getDestinationStream(...) returns null.
        } catch (IOException e) {
            // A malformed/non-standard source file (e.g. a PDF with a corrupt header, missing
            // xref, or truncated content) is a data-quality issue in the bitstream, not a
            // DSpace fault. Skip text extraction for it instead of failing the whole
            // filter-media run. See dataquest-dev/dspace-customers#752.
            log.warn("Unable to extract text from bitstream in Item {}: {}",
                    currentItem.getHandle(), e.getMessage());
            return null;

dspace-api/src/main/java/org/dspace/app/mediafilter/PDFBoxThumbnail.java:90

  • Catching all IOException here will also downgrade genuine source read/assetstore IO failures to WARN+skip (because RandomAccessReadBuffer reads from the provided InputStream inside this try). If the goal is to downgrade only PDF parse/render failures, read the stream fully first so read IOExceptions still propagate, then parse from the in-memory bytes and only catch IOExceptions from PDFBox parsing/rendering.
        } catch (IOException ex) {
            // A malformed/non-standard PDF (bad %PDF- header, missing xref, truncated file, etc.)
            // is a data-quality issue in the source bitstream, not a DSpace fault. Skip the
            // thumbnail instead of failing the whole filter-media run.
            // See dataquest-dev/dspace-customers#752.
            log.warn("PDF could not be parsed by PDFBox. Cannot create thumbnail (item: {}): {}",
                    currentItem::getHandle, ex::getMessage);
            return null;

@milanmajchrak
milanmajchrak force-pushed the 903-be/filtermedia-pdf-warn branch from af171d3 to 7eaadcc Compare August 14, 2026 13:37
@milanmajchrak milanmajchrak changed the title [Port to dtq-dev] Issue dspace-customers#903: log unparsable-PDF filter-media errors as WARN, not ERROR [Port to dtq-dev] Log unparsable-PDF filter-media errors as WARN, not ERROR Aug 14, 2026
@milanmajchrak
milanmajchrak merged commit b0c4850 into dtq-dev Aug 14, 2026
6 checks passed
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.

4 participants