Make null streamLength when catching exception - #740
Conversation
📝 WalkthroughWalkthroughInline image stream lengths now use nullable ChangesInline image length propagation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java`:
- Around line 631-632: Update the IOException handler in OperatorParser to pass
the caught exception e to LOGGER.log along with the existing message, preserving
the stack trace while keeping streamLength null for downstream handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fc1fe867-0671-4da3-8135-1bf89e2aa9bb
📒 Files selected for processing (2)
validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.javavalidation-model/src/main/java/org/verapdf/gf/model/impl/operator/inlineimage/GFOp_EI.java
| } catch (IOException e) { | ||
| LOGGER.log(Level.SEVERE, "Error during computing inline image data stream length", e); | ||
| LOGGER.log(Level.SEVERE, "Error during computing inline image data stream length"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the referenced source line and surrounding logging practice.
if [ -f validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java ]; then
echo "== referenced source =="
sed -n '610,645p' validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java | nl -ba -v610
else
echo "referenced file missing"
fi
echo
echo "== LOGGER usage in OperatorParser =="
rg -n 'LOGGER\.log|Logger\.' validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java || true
echo
echo "== nearby catch/logging patterns for IOException in source =="
python3 - <<'PY'
from pathlib import Path
p = Path('validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java')
if p.exists():
text = p.read_text()
for start in range(text.find('LOGGER.log(Level.SEVERE'), 0, -1):
pass
idx = text.find('LOGGER.log')
print('first LOGGER.log index', idx)
PYRepository: veraPDF/veraPDF-validation
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Determine whether the caught IOException is re-thrown/attached,
# whether the Java Logger overload accepting a Throwable is available,
# and inspect whether line 631 is inside an inline-image parsing path.
echo "== imports and class declarations =="
sed -n '1,80p' validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java | nl -ba -v1
echo
echo "== method context =="
python3 - <<'PY'
from pathlib import Path
p = Path('validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java')
lines = p.read_text().splitlines()
for i in range(500, 660):
print(f'{i+1}: {lines[i]}')
PY
echo
echo "== references to streamLength and inline image handling =="
rg -n 'streamLength|inline image|Inline|inline' validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java || true
echo
echo "== Java Logger overload availability via javadoc/source in toolchain =="
if command -v javac >/dev/null 2>&1; then
javac -version
javadoc -help 2>&1 | head -20 || true
echo "checking java.util.logging.Logger.signature in JDT classpath?"
else
echo "javac unavailable"
fiRepository: veraPDF/veraPDF-validation
Length of output: 241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java'
echo "== referenced source =="
if [ -f "$FILE" ]; then
sed -n '610,645p' "$FILE" | cat -n
else
echo "referenced file missing"
fi
echo
echo "== LOGGER usage in OperatorParser =="
rg -n 'LOGGER\.log|Logger\.' "$FILE" || true
echo
echo "== method context =="
python3 - <<'PY'
from pathlib import Path
p = Path('validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java')
if not p.exists():
raise SystemExit(0)
lines = p.read_text().splitlines()
for i in range(500, 660):
if i < len(lines):
print(f'{i+1}: {lines[i]}')
PY
echo
echo "== references to streamLength and inline image handling =="
rg -n 'streamLength|inline image|Inline|inline' "$FILE" || true
echo
echo "== Java Logger overload availability =="
if command -v javac >/dev/null 2>&1; then
javac -version
javadoc -help 2>&1 | head -20 || true
else
echo "javac unavailable"
fiRepository: veraPDF/veraPDF-validation
Length of output: 11650
Preserve the exception cause in the log.
Keeping streamLength as null is correct for downstream handling, but omitting e drops the stack trace when getStreamLength() fails. Log the thrown exception as the cause.
🔧 Proposed fix
- LOGGER.log(Level.SEVERE, "Error during computing inline image data stream length");
+ LOGGER.log(Level.SEVERE, "Error during computing inline image data stream length", e);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (IOException e) { | |
| LOGGER.log(Level.SEVERE, "Error during computing inline image data stream length", e); | |
| LOGGER.log(Level.SEVERE, "Error during computing inline image data stream length"); | |
| } catch (IOException e) { | |
| LOGGER.log(Level.SEVERE, "Error during computing inline image data stream length", e); |
🤖 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
`@validation-model/src/main/java/org/verapdf/gf/model/factory/operators/OperatorParser.java`
around lines 631 - 632, Update the IOException handler in OperatorParser to pass
the caught exception e to LOGGER.log along with the existing message, preserving
the stack trace while keeping streamLength null for downstream handling.
Summary by CodeRabbit