From 1c0385f0d91570fc70099ecefd1d81704ebbb612 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 13:29:38 +0000 Subject: [PATCH] Include an inhibitor helper's stderr when it exits immediately [patch] ChildProcessHold drained the helper's stderr into a no-op handler, so a helper that failed during the startup grace period was reported only by exit code. Keep the last 4 KB of stderr while the helper starts, wait for the streams to finish after it exits, and add what it wrote to the SleepBlockException message. Recording stops once startup succeeds. Fixes ktsu-dev/NoSleep#15 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4SLyvTvUJUdcrUu7F --- NoSleep.Test/ChildProcessHoldTests.cs | 36 ++++++++++ NoSleep/Platforms/ChildProcessHold.cs | 99 +++++++++++++++++++++++++-- 2 files changed, 130 insertions(+), 5 deletions(-) diff --git a/NoSleep.Test/ChildProcessHoldTests.cs b/NoSleep.Test/ChildProcessHoldTests.cs index 3530f72..b78df33 100644 --- a/NoSleep.Test/ChildProcessHoldTests.cs +++ b/NoSleep.Test/ChildProcessHoldTests.cs @@ -70,6 +70,42 @@ public void Start_WhenTheHelperExitsImmediately_Throws() StringAssert.Contains(exception.Message, "exited immediately", StringComparison.Ordinal); } + [TestMethod] + public void Start_WhenTheHelperExitsImmediately_IncludesItsStandardErrorInTheMessage() + { + if (!IsUnix) + { + Assert.Inconclusive("The helper paths used by this test only exist on Unix."); + return; + } + + SleepBlockException exception = Assert.ThrowsExactly( + () => ChildProcessHold.Start(ImmediateExitHelper, ["-c", "echo boom >&2; exit 1"])); + + StringAssert.Contains(exception.Message, "exited immediately with code 1", StringComparison.Ordinal); + StringAssert.Contains(exception.Message, "boom", StringComparison.Ordinal); + } + + [TestMethod] + public void Start_WhenTheHelperFloodsStandardError_KeepsOnlyTheLastOfItInTheMessage() + { + if (!IsUnix) + { + Assert.Inconclusive("The helper paths used by this test only exist on Unix."); + return; + } + + // 600 lines of filler (about 9 KB), then the line that explains the failure. + const string Script = "i=0; while [ $i -lt 600 ]; do echo filler-line-$i >&2; i=$((i+1)); done; echo the-real-reason >&2; exit 3"; + + SleepBlockException exception = Assert.ThrowsExactly( + () => ChildProcessHold.Start(ImmediateExitHelper, ["-c", Script])); + + StringAssert.Contains(exception.Message, "the-real-reason", StringComparison.Ordinal); + Assert.IsFalse(exception.Message.Contains("filler-line-0 ", StringComparison.Ordinal), "The oldest output should have been dropped"); + Assert.IsLessThan(4096 + 200, exception.Message.Length, "The message should stay bounded"); + } + [TestMethod] public void Start_WithAMissingHelper_Throws() { diff --git a/NoSleep/Platforms/ChildProcessHold.cs b/NoSleep/Platforms/ChildProcessHold.cs index 0a4e10a..21afaab 100644 --- a/NoSleep/Platforms/ChildProcessHold.cs +++ b/NoSleep/Platforms/ChildProcessHold.cs @@ -8,6 +8,7 @@ namespace ktsu.NoSleep.Platforms; using System.Diagnostics; using System.Globalization; using System.IO; +using System.Threading; /// /// A sleep inhibitor that exists for as long as a helper process stays alive. @@ -73,10 +74,12 @@ public static ChildProcessHold Start(string fileName, IEnumerable argume Process process = new() { StartInfo = startInfo }; - // Nothing consumes the helper's output, but an unread pipe eventually fills and blocks the writer, - // so drain both streams and drop what arrives. + // An unread pipe eventually fills and blocks the writer, so both streams are drained. Standard output + // is dropped. Standard error is kept while the helper starts up, because a helper that fails at once + // usually says why there, and that reason belongs in the exception. + StartupErrorOutput startupErrors = new(); process.OutputDataReceived += static (_, _) => { }; - process.ErrorDataReceived += static (_, _) => { }; + process.ErrorDataReceived += (_, e) => startupErrors.Add(e.Data); try { @@ -92,13 +95,25 @@ public static ChildProcessHold Start(string fileName, IEnumerable argume if (process.WaitForExit(GetMilliseconds(StartupGrace))) { + // The timed wait returns as soon as the process ends, possibly before its last lines of standard + // error have been read. The untimed wait also waits for both streams to reach end of input, which + // they do promptly now that the helper is gone. + process.WaitForExit(); int exitCode = process.ExitCode; + string errorOutput = startupErrors.Stop(); process.Dispose(); - throw new SleepBlockException(string.Create( + + string message = string.Create( CultureInfo.InvariantCulture, - $"The inhibitor helper \"{fileName}\" exited immediately with code {exitCode}.")); + $"The inhibitor helper \"{fileName}\" exited immediately with code {exitCode}."); + + throw new SleepBlockException(errorOutput.Length == 0 + ? message + : $"{message} It reported: {errorOutput}"); } + _ = startupErrors.Stop(); + return new ChildProcessHold(process) { Command = fileName, ProcessId = process.Id }; } @@ -147,4 +162,78 @@ public void Dispose() } private static int GetMilliseconds(TimeSpan timeout) => (int)timeout.TotalMilliseconds; + + /// + /// Keeps the most recent standard error lines a helper writes while it starts up, up to a fixed size. + /// + /// + /// Lines arrive on a thread-pool thread while waits, so access is locked. Once stopped, + /// later lines are dropped, so a long-running helper cannot grow the buffer. + /// + private sealed class StartupErrorOutput + { + /// + /// The most characters kept. When more arrive, the oldest are dropped, since the last lines a failing + /// helper writes are the ones most likely to say why it failed. + /// + private const int MaxLength = 4096; + + private readonly Lock gate = new(); + private readonly LinkedList lines = new(); + private int length; + private bool stopped; + + /// + /// Records a line of standard error, unless recording has stopped. + /// + /// The line, or at end of input. + public void Add(string? line) + { + if (string.IsNullOrWhiteSpace(line)) + { + return; + } + + string trimmed = line.Trim(); + + lock (gate) + { + if (stopped) + { + return; + } + + if (trimmed.Length > MaxLength) + { + trimmed = trimmed[^MaxLength..]; + } + + lines.AddLast(trimmed); + // Each line after the first costs one more character for the space that joins it to the last. + length += trimmed.Length + (lines.Count > 1 ? 1 : 0); + + while (length > MaxLength && lines.First is { } oldest) + { + length -= oldest.Value.Length + (lines.Count > 1 ? 1 : 0); + lines.RemoveFirst(); + } + } + } + + /// + /// Stops recording and returns what was recorded. + /// + /// The recorded lines joined by spaces, or an empty string when there were none. + public string Stop() + { + lock (gate) + { + stopped = true; + string text = string.Join(' ', lines); + lines.Clear(); + length = 0; + return text; + } + } + } }