Repository navigation
Stream CR-only progress ticks in run() instead of buffering them into one message - #2731
Conversation
During a plugin install or update the download progress line clumps into one very long line instead of updating in place. A captured example was a single 3486 character message holding 41 carriage returns and one trailing line feed. Root cause is in run(): the subprocess pipe was read with fgets(), which only breaks on LF. A command that emits classic CR-only progress ticks therefore keeps filling the same fgets() read until a LF finally arrives (or EOF), and the whole progress sequence leaves run() as one message. The receiving side splits on CR per received message, so once the ticks are already glued together inside a single message it can no longer render them in place. Read the pipe character by character and flush on CR or LF, so every tick is passed on as its own message. 'multiplugin' in the same directory already reads its child output this way and is the reference for the pattern. Two details are handled beyond that reference: a final chunk without a terminator is still flushed before returning, and fgetc() returning false at end of stream ends the loop explicitly rather than relying on the silent false to string cast. Return value (pclose) and the emitted bytes are unchanged; only the message boundaries differ. Ordinary LF-only output is unaffected.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. Walkthrough
ChangesPlugin output handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change preserves live plugin progress updates and replay behavior without an identified regression. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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. A rabbit watched the output stream, Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
Unraid Bot (unraid-bot)
added
QA Passed
|
This is a great fix, thanks! |
Unraid Bot (unraid-bot)
removed
the
7.4.0
Fixes the plugin-install progress garbling reported on the Unraid forum, confirmed by Jorge (@jorgeb).
The bug
emhttp/plugins/dynamix.plugin.manager/scripts/plugin'srun()read subprocess output withfgets(), which only splits on\n. A command whose output uses\r-only progress ticks (no\nbetween them, the classic in-place progress bar) gets every tick buffered into onefgets()read that doesn't return until an actual\nshows up or the process exits. That single, already-clumped blob then goes out as one nchan message. The client's per-message\rhandling inrenderMessage()operates on whole received messages, but cannot separate progress updates inside an already-clumped message. Reproduced live: a real install produced a single 3486-character message with 41 embedded\r.The fix
Ports the pattern this repo already uses correctly in the sibling script
multiplugin: read one character at a time withfgetc()and flush on\ror\n, so each tick becomes its own message.function run($command) { $run = popen($command,'r'); - while (!feof($run)) write(fgets($run)); + $line = ''; + while (!feof($run)) { + $char = fgetc($run); + if ($char === false) break; + $line .= $char; + if (in_array($char,["\r","\n"])) {write($line); $line = '';} + } + if ($line !== '') write($line); return pclose($run); }The browser renderer also removes the final character only when it is a newline. This preserves unterminated output and a literal
0, which previously reached the client but were truncated during rendering. The carriage-return replacement branch andmultipluginare unchanged.Two small, deliberate differences from
multiplugin's version, both strictly better and both verified against real output:$char === falsebreak on end-of-stream, instead of relying onfgets-style implicit end-of-stream handling. This also means a stream that errors without ever setting EOF can't spin the loop forever, which the old code was theoretically exposed to.if ($line !== '') write($line)after the loop, so output with no final terminator (or output that's just the literal string"0") still reaches the client.multiplugin's own!empty($line)guard would drop a bare"0", sinceempty("0")is true in PHP; this uses!== ''instead.Testing
Emitted bytes are unchanged in every case tested (only the message boundaries differ):
\r-only progress ticks, normal\n-terminated lines, a mix of both, output with no trailing terminator, a bare"0"with no terminator, and no output at all. Verified against the five call sites ofrun()in this file (pre_hooks,post_hooks, and the three run variants) that none of them depend on the old message granularity, only on the return value.php -lpasses.One pre-existing property, unchanged by this PR and already present in
multiplugin: a stream that mixes\n-only lines with later\r\n-terminated lines can lose one line at the transition, because the client's renderer treats a\r-terminated message as "replace the previous line." Not introduced here, and unlikely on the Linux run-scripts this path executes (wget/curl-style progress is\r-only, not CRLF).Renderer follow-up validation
On Unraid 7.4.0-beta.1.3 with the original PR plugin plus the one-line renderer change, a real browser install showed live download progress, separate normal lines, the complete unterminated message, and literal
0. A deliberate exit-7 fixture still displayed Error and the command failure. Six focused checks against the extracted renderer passed: unterminated tail, zero, LF lines, spaces, blank lines, and repeated CR updates.The rebuilt PR plugin version 2026.09.16.2354 was installed cleanly on Unraid 7.4.0-beta.1.3 after removing the temporary renderer edit. After a browser reload, the real download fixture displayed the complete trailing message and literal
0; the exit-7 fixture still displayed Error correctly. Installed renderer and reader hashes matched the tested sources, and all eight reader contract checks passed again. Build and CodeRabbit checks passed for head c4d48c8.