Remove Exceptions stack trace from logs - #719
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis change removes caught exception arguments and direct stack-trace printing from logging and error handlers across COS, parser, document, font, image, and serialization code. Messages, log levels, control flow, and fallback behavior remain unchanged. ChangesCOS and serialization logging
Document and external error logging
Font, function, and image logging
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/verapdf/cos/visitor/Writer.java (1)
126-161: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve a failure signal for serialization I/O.
These catch blocks are now empty. A failed write in scalar, array, dictionary, null, header, or cross-reference serialization is treated as success.
COSDocument.saveTocan then copy a partial temporary PDF and return normally.
close()also hides close failures.getOffset()converts an output error to0, which can produce invalid cross-reference offsets.Remove direct stack-trace printing, but log a structured message without the throwable or propagate
VeraPDFParserException. Do not make serialization failures silent.Also applies to: 164-189, 258-261, 289-292, 354-355, 372-385
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/verapdf/cos/visitor/Writer.java` around lines 126 - 161, Update all empty I/O catches in Writer, including scalar/array/dictionary/null/header/cross-reference visitors, close(), and getOffset(), so serialization failures are never treated as success. Remove direct stack-trace printing and either propagate VeraPDFParserException or emit a structured error message without the throwable. Ensure getOffset() does not convert output failures to 0, allowing COSDocument.saveTo to detect and reject partial serialization.
🧹 Nitpick comments (1)
src/main/java/org/verapdf/pd/function/PDType0Function.java (1)
157-157: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign throwable logging in
PDType0Function.If the cleanup applies across the class, remove the
eargument from the remainingLOGGER.log(..., e)calls at lines 297 and 319. The current mix makes stack-trace logging inconsistent for warnings in the same class.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/verapdf/pd/function/PDType0Function.java` at line 157, Standardize warning logging in PDType0Function by removing the throwable argument from the remaining LOGGER.log calls near the handlers at lines 297 and 319, matching the existing “Can not parse function” warning style while preserving their warning messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/java/org/verapdf/cos/visitor/Writer.java`:
- Around line 126-161: Update all empty I/O catches in Writer, including
scalar/array/dictionary/null/header/cross-reference visitors, close(), and
getOffset(), so serialization failures are never treated as success. Remove
direct stack-trace printing and either propagate VeraPDFParserException or emit
a structured error message without the throwable. Ensure getOffset() does not
convert output failures to 0, allowing COSDocument.saveTo to detect and reject
partial serialization.
---
Nitpick comments:
In `@src/main/java/org/verapdf/pd/function/PDType0Function.java`:
- Line 157: Standardize warning logging in PDType0Function by removing the
throwable argument from the remaining LOGGER.log calls near the handlers at
lines 297 and 319, matching the existing “Can not parse function” warning style
while preserving their warning messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b786f2d6-d5b6-465d-a4c7-459bbd74c2b5
📒 Files selected for processing (27)
src/main/java/org/verapdf/cos/COSArray.javasrc/main/java/org/verapdf/cos/COSDocument.javasrc/main/java/org/verapdf/cos/COSStream.javasrc/main/java/org/verapdf/cos/filters/COSFilterRegistry.javasrc/main/java/org/verapdf/cos/visitor/COSCopier.javasrc/main/java/org/verapdf/cos/visitor/Writer.javasrc/main/java/org/verapdf/external/ICCProfile.javasrc/main/java/org/verapdf/external/JPEG2000.javasrc/main/java/org/verapdf/io/Reader.javasrc/main/java/org/verapdf/parser/PDFParser.javasrc/main/java/org/verapdf/pd/PDDocument.javasrc/main/java/org/verapdf/pd/colors/PDIndexed.javasrc/main/java/org/verapdf/pd/encryption/StandardSecurityHandler.javasrc/main/java/org/verapdf/pd/font/PDCIDFont.javasrc/main/java/org/verapdf/pd/font/PDFont.javasrc/main/java/org/verapdf/pd/font/cmap/CMap.javasrc/main/java/org/verapdf/pd/font/cmap/CMapFactory.javasrc/main/java/org/verapdf/pd/font/cmap/CMapFile.javasrc/main/java/org/verapdf/pd/font/cmap/PDCMap.javasrc/main/java/org/verapdf/pd/font/stdmetrics/StandardFontMetricsFactory.javasrc/main/java/org/verapdf/pd/font/truetype/AdobeGlyphList.javasrc/main/java/org/verapdf/pd/font/truetype/PDTrueTypeFont.javasrc/main/java/org/verapdf/pd/font/type1/PDType1Font.javasrc/main/java/org/verapdf/pd/font/type3/PDType3Font.javasrc/main/java/org/verapdf/pd/function/PDType0Function.javasrc/main/java/org/verapdf/pd/images/PDXImage.javasrc/main/java/org/verapdf/tools/StaticResources.java
💤 Files with no reviewable changes (1)
- src/main/java/org/verapdf/cos/visitor/COSCopier.java
2 participants
Summary by CodeRabbit