Fix parser crash on chunk line-wrap from long filenames#290
Fix parser crash on chunk line-wrap from long filenames#290nitrobass24 merged 1 commit intodevelopfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new regex pattern is added to recognize and skip wrapped progress lines in lftp output where long filenames cause line wrapping. The pattern is applied during header validation and end-of-line processing to treat wrapped chunk fragments as ignorable orphan progress lines, improving parsing robustness without affecting public APIs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 the current code and only fix it if needed.
Inline comments:
In `@src/python/lftp/job_status_parser.py`:
- Around line 243-247: The chunk_wrap_pattern regex in
src/python/lftp/job_status_parser.py requires at least one
non-backtick/non-backslash character before the closing quote, causing failures
for quote-only wrapped tails; update the chunk_wrap_pattern so the leading
characters are optional but still disallow lines starting with backtick or
backslash (e.g., change the prefix to use a non-capturing optional group like
(?:[^`\\].*)? before the closing quote), preserving the rest of the pattern (the
"'\s+at\s+\d+\s..." sequence, optional "(?:\(\d+%\)\s+)?", optional speed group
"(?:(?:\d+\.?\d*\s?({sz}))\/s\s+)?", optional eta "(?:eta:({eta})\s+)?", and
final "\s*\[.*\]$"); modify the chunk_wrap_pattern variable accordingly so lines
that start exactly at the quote boundary match while still rejecting
backtick/backslash-prefixed lines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 608d1546-b23a-4881-b255-ce9fc53fd503
📒 Files selected for processing (2)
src/python/lftp/job_status_parser.pysrc/python/tests/unittests/test_lftp/test_job_status_parser.py
Long filenames cause lftp chunk progress lines to wrap across PTY boundaries, producing tail fragments like: tmos.7.1.DV.HDR.H.265-TheFarm.mkv' at 22283455338 (0%) 427.6K/s eta:28m [Receiving data] The parser didn't recognize these fragments (missing leading backtick) and raised ValueError, crashing the controller. Add chunk_wrap_m pattern to match these fragments and skip them alongside existing orphan/partial progress line handling. Add 3 regression tests covering: fragment after job, bare fragment, and fragment with [Connecting...] status. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5ef56e1 to
6a204d0
Compare
Summary
Fix lftp parser crash when long filenames cause chunk progress lines to wrap across PTY boundaries.
Problem
Files with very long names (e.g.
1923 (2022) S01E04 (1080p SKST WEB-DL DDP5.1 H264)-Vialle.mkv) produce chunk progress lines that exceed the PTY width. The line wraps, and the parser sees a tail fragment like:This fragment is missing the leading backtick (
`) and start of the filename, so thechunk_atpattern can't match it. The parser raisesValueError, which crashes the controller and restarts the app.This is the same class of bug as #253 and #260 (Unraid PTY wrapping), but for a different fragment type.
Fix
Add a
chunk_wrap_mpattern that matches these tail fragments: lines that end with' at <pos> (<pct>%) [<speed>] [eta:<time>] [<status>]but don't start with a backtick or backslash. These are safely skipped alongside existing orphan/partial progress line handling.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests