fix: ensure CLI output is fully flushed before exit#566
fix: ensure CLI output is fully flushed before exit#566Xy2002 wants to merge 1 commit intoneovateai:masterfrom
Conversation
|
Hi, I noticed this PR successfully addresses the truncation issue described in #565, but it hasn't been merged for a while. I suspect the hesitation might be due to the architectural impact of turning all output-related methods into As an alternative, what if we implement a "drain-on-exit" pattern instead? This intercept-at-the-exit approach is much less intrusive while still solving the issue reliably. Suggested Alternative:export function safeExit(code = 0) {
const drainStdout = process.stdout.write('');
const drainStderr = process.stderr.write('');
if (!drainStdout || !drainStderr) {
let drainedCount = 0;
const targetCount = (drainStdout ? 0 : 1) + (drainStderr ? 0 : 1);
const onDrain = () => {
drainedCount++;
if (drainedCount >= targetCount) process.exit(code);
};
if (!drainStdout) process.stdout.once('drain', onDrain);
if (!drainStderr) process.stderr.once('drain', onDrain);
// Force exit after a short timeout if drain doesn't happen
setTimeout(() => process.exit(code), 1000).unref();
} else {
process.exit(code);
}
}Pros & Cons:
What do you think? This might be a cleaner path to finally resolving the truncation bug without refactoring the whole message flow. |
5128119 to
96ae3d9
Compare
Thanks for the suggestion — I agree the async‑infection was the main architectural downside of my original approach. A “drain‑on‑exit” guard is a cleaner, low‑intrusion fix for this class of truncation. I’ve implemented a minimal variant of this in runQuiet: instead of making output methods async, we wait for stdout/stderr to flush right before exit. This keeps the rest of the codebase synchronous and avoids refactoring the message flow. One small tweak to your proposal: I prefer the explicit write('', cb) drain instead of relying on drain events only, because it’s deterministic even when write() returns true. So, I think this is the right direction, and I’m happy to align with that approach rather than async‑ifying output throughout the codebase. |
This PR fixes an issue where the CLI output would be truncated when piped to other processes. This was happening because process.exit() was being called immediately after console.log() or process.stdout.write(), without waiting for the internal buffer to drain.
fix #565