diff --git a/CHANGELOG.md b/CHANGELOG.md index e41be68..3616360 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ What changed in each release. The section for a version is lifted into that vers Newest first. Add a section before tagging. +## 0.14.0 - 2026-09-17 + +- **Interactive mode can no longer fail silently.** The radio work runs on a background thread, and only seven specific kinds of error were ever handled; anything else vanished. Nothing was logged, no message appeared, and the power-cycle dialog stayed on screen for ever, so from the operator's side the tool simply died the moment the radio was power-cycled, with nothing to report to anybody. Every failure is now caught wherever it happens. +- **Every failure now leaves something you can send.** A crash report is written to the temporary directory (on Linux, `/tmp/tait-codeplug-crash-.log`), naming what the tool was doing, its version, the platform and the full error. The same report is printed to the terminal after the full-screen editor hands it back, which is the other half of why nothing was visible before: the editor owns the screen, so anything printed as the tool died went down with it. Intermittent faults are the ones that need this most, because the run that reproduces the problem is rarely the run you are watching. +- **An error while drawing the screen is reported rather than fatal.** Progress updates and the code that displays a codeplug after a read both run on the interface thread, where a fault used to unwind the whole application instead of being shown. + ## 0.13.0 - 2026-09-17 - **`apt install tait-codeplug` now works**, on Debian, Ubuntu and Raspberry Pi OS, for `amd64`, `arm64` and `armhf`. Three lines to add the [packet-net apt repository](https://github.com/packet-net/apt) and it installs like any other package, and `apt upgrade` keeps it current along with everything else on the machine; the README has the lines. The package is the same self-contained single-file binary as the release asset for that architecture, built by the same publish with the same flags, so nothing needs .NET installed. A release now attaches the three `.deb` files as assets too, for installing one by hand. diff --git a/README.md b/README.md index fd0638e..5d7904c 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,16 @@ itself with `tait-codeplug --upgrade`. Or build it yourself: `dotnet run --project src/M0LTE.Tait.Codeplug.Cli -- ...` (.NET 10 SDK). +### If it goes wrong + +Interactive mode writes a crash report to the temporary directory +(`/tmp/tait-codeplug-crash-.log` on Linux) and prints the same thing to the terminal once +it has handed the screen back. Paste that into an +[issue](https://github.com/M0LTE/tait-codeplug/issues): it names the version, the platform, what the +tool was doing and the full error. The one-shot verbs print their errors straight to the terminal, so +running the same operation as `read` or `patch` rather than in interactive mode is also a quick way +to see what a failure actually says. + ## Interactive mode Run it with no arguments and you get a screen instead of a verb: pick a port, read the radio, edit the packet-relevant essentials, write it back. diff --git a/src/M0LTE.Tait.Codeplug.Cli/CrashReport.cs b/src/M0LTE.Tait.Codeplug.Cli/CrashReport.cs new file mode 100644 index 0000000..a3cf9c9 --- /dev/null +++ b/src/M0LTE.Tait.Codeplug.Cli/CrashReport.cs @@ -0,0 +1,71 @@ +using System.Globalization; +using System.Runtime.InteropServices; +using System.Text; + +namespace M0LTE.Tait.Codeplug.Cli; + +/// +/// What the tool writes down when something fails, so that a failure is never just a tool that +/// vanished. +/// +/// Interactive mode owns the whole terminal. An exception that escapes to the top takes the +/// application down, and by the time the runtime prints the trace the screen has already been handed +/// back, so on a lot of terminals the useful part is gone with it. That is how a real fault on a real +/// radio presented: "crashes just after putting the radio into programming mode, no error visible", +/// which is the one report nobody can act on. A read that fails intermittently is exactly the case +/// that needs the evidence kept, because the next run may well work. +/// +/// So a failure is recorded in three places, deliberately overlapping: the in-app log while the +/// screen is still up, a file that outlives the process, and stderr after the terminal has been +/// restored. The file is the one that matters for a fault someone has to report second-hand. +/// +internal static class CrashReport +{ + /// The report's own file name. Timestamped so repeated failures do not overwrite each + /// other, and sortable, because the interesting one is usually the last. + internal static string FileName(DateTimeOffset when) => + string.Create(CultureInfo.InvariantCulture, $"tait-codeplug-crash-{when:yyyyMMdd-HHmmss-fff}.log"); + + /// + /// The whole report as text. Everything needed to act on a failure without being sat at the + /// machine: what was being attempted, what the version was, what the platform is, and the + /// exception with its stack and any inner exceptions (ToString carries all three). + /// + internal static string Format(string context, Exception exception, DateTimeOffset when) + { + ArgumentNullException.ThrowIfNull(exception); + + var text = new StringBuilder(); + text.Append("tait-codeplug crash report").Append('\n'); + text.Append("when: ").Append(when.ToString("yyyy-MM-dd HH:mm:ss zzz", CultureInfo.InvariantCulture)).Append('\n'); + string version = CliVersion.Current; + text.Append("version: ").Append(version.Length == 0 ? "(unknown)" : version).Append('\n'); + text.Append("platform: ").Append(RuntimeInformation.OSDescription.Trim()) + .Append(' ').Append(RuntimeInformation.RuntimeIdentifier).Append('\n'); + text.Append("doing: ").Append(context).Append('\n'); + text.Append('\n'); + text.Append(exception.ToString()).Append('\n'); + return text.ToString(); + } + + /// + /// Write somewhere it will outlive the process, returning the path, or + /// null if it could not be written. Best effort by design: this runs while something has already + /// gone wrong, so it must never throw and add a second failure to the first. The caller still has + /// the report text either way. + /// + internal static string? Write(string report, DateTimeOffset when) + { + try + { + string path = Path.Combine(Path.GetTempPath(), FileName(when)); + File.WriteAllText(path, report); + return path; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException + or NotSupportedException or ArgumentException) + { + return null; + } + } +} diff --git a/src/M0LTE.Tait.Codeplug.Cli/Tui.cs b/src/M0LTE.Tait.Codeplug.Cli/Tui.cs index df91039..7de59df 100644 --- a/src/M0LTE.Tait.Codeplug.Cli/Tui.cs +++ b/src/M0LTE.Tait.Codeplug.Cli/Tui.cs @@ -73,6 +73,15 @@ internal static class Tui private static DateTime _lastInputUtc = DateTime.UtcNow; + /// + /// Failures recorded during the session, echoed to stderr once the terminal has been handed back. + /// Written to from the worker thread as well as the UI thread, hence the lock. + /// + private static readonly List Reports = []; + + /// The process-wide handler is added once, however many times the screen is opened. + private static bool _lastResortInstalled; + /// A codeplug to open on, or null to start empty. /// Where came from, for the log line. /// A Terminal.Gui driver name to force, or null to let it choose. See @@ -80,6 +89,8 @@ internal static class Tui internal static int Run(CodeplugImage? initial = null, string? source = null, string? driver = null) { _app = Application.Create(); + bool fatal = false; + InstallLastResortHandler(); try { if (driver is not null) @@ -108,6 +119,16 @@ internal static int Run(CodeplugImage? initial = null, string? source = null, st SetBusy(false, StatusText()); _app.Run(_window); } + catch (Exception ex) + { + // Last line of defence. Anything that gets here has already taken the screen down, so the + // only thing left to do is make sure it is written somewhere before the process ends. + // Deliberately catches everything: a fault that kills the tool is exactly the one worth + // reporting, and rethrowing would hand the user an unhandled-exception trace printed into + // a terminal that has just been reset, which is where the last one went. + Record("running the interactive screen", ex); + fatal = true; + } finally { // The loop rate is a static on the library, so hand it back as we found it rather than @@ -120,7 +141,10 @@ internal static int Run(CodeplugImage? initial = null, string? source = null, st _app.Dispose(); } - return 0; + // The terminal is ours again, so anything recorded while the screen was up can finally be + // said out loud. This is the half of the reporting a user actually sees. + EchoReports(); + return fatal ? 2 : 0; } /// @@ -626,7 +650,7 @@ private static void PromptToPowerCycle(string title) } /// Progress arrives on the worker thread; everything it touches lives on the UI thread. - private static void OnProgress(object? sender, ProgrammerProgress p) => _app.Invoke(() => ApplyProgress(p)); + private static void OnProgress(object? sender, ProgrammerProgress p) => OnUiThread(() => ApplyProgress(p)); private static void ApplyProgress(ProgrammerProgress p) { @@ -689,7 +713,9 @@ private static void RunOffThread(Func work, Action onSuccess) try { T result = work(); - _app.Invoke(() => + // onSuccess decodes and displays what came back, so it is as capable of throwing as + // the radio work was, and it runs on the UI thread where an escape is fatal. + OnUiThread(() => { FinishRadioOperation(); onSuccess(result); @@ -699,27 +725,128 @@ private static void RunOffThread(Func work, Action onSuccess) catch (OperationCanceledException) { // Cancelling is a decision, not a fault: no dialog, and the prompt is already gone. - _app.Invoke(() => + OnUiThread(() => { FinishRadioOperation(); SetBusy(false, StatusText()); }); } - catch (Exception ex) when (ex is IOException or TimeoutException or InvalidOperationException - or ArgumentException or UnauthorizedAccessException or FormatException - or NotSupportedException) + catch (Exception ex) { - _app.Invoke(() => + // Everything, not a list of the failures we thought of. This used to catch seven + // specific types, and anything else escaped into a discarded Task: nothing was + // logged, no dialog appeared, the power-cycle prompt was never dismissed and the + // runtime swallowed the exception when the task was collected. The tool looked like + // it had died the moment the radio was power-cycled, with nothing to show anyone. + // A radio error and a bug in our own code both have to end up on the screen. + Record("talking to the radio", ex); + OnUiThread(() => { FinishRadioOperation(); Log($"error: {ex.Message}"); SetBusy(false, StatusText()); - Error("Radio error", ex.Message); + Error("Radio error", Describe(ex)); }); } }); } + /// + /// Run on the UI thread without letting a failure in it take the + /// application down. IApplication.Invoke queues onto the main loop, so an exception thrown + /// inside one is on the UI thread and outside whatever try/catch queued it: it unwinds the main + /// loop instead, which is a silent death rather than an error message. + /// + private static void OnUiThread(Action action) + { + try + { + _app.Invoke(() => + { + try + { + action(); + } + catch (Exception ex) + { + Record("updating the screen", ex); + } + }); + } + catch (Exception ex) + { + // Queueing itself can fail if the application is already going away, which is precisely + // when a worker is most likely to be reporting something. The report is recorded before + // this is ever called, so losing the on-screen half of it costs nothing. + Record("handing work to the screen", ex); + } + } + + /// + /// Catch what nothing else can: a failure on a thread we did not start, which ends the process + /// without unwinding through any of our own handlers. Nothing can be shown at that point and the + /// stderr echo will not run, but the report still reaches a file, which is the half that matters + /// for an intermittent fault somebody has to report afterwards. + /// + private static void InstallLastResortHandler() + { + if (_lastResortInstalled) + { + return; + } + + _lastResortInstalled = true; + AppDomain.CurrentDomain.UnhandledException += (_, e) => + { + if (e.ExceptionObject is Exception ex) + { + Record("an unhandled failure", ex); + } + }; + } + + /// Record a failure durably: to a file that outlives the process, and to the list echoed + /// to stderr on the way out. Safe to call from any thread, and never throws. + private static void Record(string context, Exception exception) + { + DateTimeOffset now = DateTimeOffset.Now; + string report = CrashReport.Format(context, exception, now); + string? path = CrashReport.Write(report, now); + + lock (Reports) + { + Reports.Add(path is null ? report : $"{report}(this report was also written to {path})\n"); + } + } + + /// Print anything recorded to stderr, once the terminal has been restored. + private static void EchoReports() + { + lock (Reports) + { + foreach (string report in Reports) + { + Console.Error.WriteLine(); + Console.Error.Write(report); + } + + if (Reports.Count > 0) + { + Console.Error.WriteLine( + "Please report this at https://github.com/M0LTE/tait-codeplug/issues with the text above."); + } + + Reports.Clear(); + } + } + + /// What to put in front of someone: the message, and the type when the message alone does + /// not say what went wrong (a NullReferenceException's does not). + private static string Describe(Exception exception) => + string.IsNullOrWhiteSpace(exception.Message) + ? exception.GetType().Name + : $"{exception.Message}\n\n({exception.GetType().Name}; details written to the crash report on exit.)"; + /// Take the prompt and the bar down, whatever the operation's outcome was. private static void FinishRadioOperation() { diff --git a/tests/M0LTE.Tait.Codeplug.Tests/CrashReportTests.cs b/tests/M0LTE.Tait.Codeplug.Tests/CrashReportTests.cs new file mode 100644 index 0000000..c71846a --- /dev/null +++ b/tests/M0LTE.Tait.Codeplug.Tests/CrashReportTests.cs @@ -0,0 +1,106 @@ +using M0LTE.Tait.Codeplug.Cli; + +namespace M0LTE.Tait.Codeplug.Tests; + +/// +/// A failure in interactive mode used to be invisible: the screen owns the terminal, so an exception +/// that reached the top took the application down and the runtime printed its trace into a terminal +/// that had just been handed back, where it was wiped. The report below is what replaces that, and +/// what someone pastes into an issue, so the things worth pinning are that it says what the tool was +/// doing, and that trying to save it can never itself throw - it only ever runs when something has +/// already gone wrong. +/// +public class CrashReportTests +{ + private static readonly DateTimeOffset When = new(2026, 9, 17, 20, 15, 30, TimeSpan.Zero); + + [Fact] + public void The_report_says_what_the_tool_was_doing_when_it_failed() + { + var boom = new InvalidOperationException("the radio stopped answering"); + + string report = CrashReport.Format("talking to the radio", boom, When); + + report.Should().Contain("talking to the radio", "a stack trace alone does not say what was being attempted"); + report.Should().Contain("the radio stopped answering"); + report.Should().Contain(nameof(InvalidOperationException), "the type says what kind of fault it was"); + report.Should().Contain("2026-09-17", "an intermittent fault is reported later, so it needs its own timestamp"); + } + + [Fact] + public void The_report_carries_the_inner_exception_too() + { + // The interesting half of a wrapped failure is almost always the inner one. + var inner = new TimeoutException("no programming response within the transaction deadline"); + var outer = new InvalidOperationException("read failed", inner); + + CrashReport.Format("reading the radio", outer, When) + .Should().Contain("no programming response within the transaction deadline"); + } + + [Fact] + public void A_report_with_no_stack_trace_is_still_a_report() + { + // An exception that was constructed but never thrown has no stack. Formatting must not depend + // on there being one, because a report that throws while reporting leaves us where we started. + Action act = () => CrashReport.Format("starting up", new InvalidOperationException("never thrown"), When); + + act.Should().NotThrow(); + } + + [Fact] + public void File_names_are_timestamped_so_repeated_failures_do_not_overwrite_each_other() + { + // The fault that prompted this was intermittent: a session can fail more than once, and the + // run that finally reproduces it must not erase the one before. + string first = CrashReport.FileName(When); + string second = CrashReport.FileName(When.AddMilliseconds(4)); + + first.Should().NotBe(second); + first.Should().StartWith("tait-codeplug-crash-").And.EndWith(".log"); + first.Should().Contain("20260917"); + } + + [Fact] + public void File_names_sort_oldest_first() + { + // Whoever is looking wants the last one, so the names have to order the way the clock does. + string[] names = + [ + CrashReport.FileName(When.AddHours(2)), + CrashReport.FileName(When), + CrashReport.FileName(When.AddMinutes(1)), + ]; + + names.Order(StringComparer.Ordinal).Should().Equal( + CrashReport.FileName(When), + CrashReport.FileName(When.AddMinutes(1)), + CrashReport.FileName(When.AddHours(2))); + } + + [Fact] + public void Writing_a_report_puts_it_somewhere_readable_and_returns_where() + { + string? path = CrashReport.Write("a report", When); + + path.Should().NotBeNull(); + try + { + File.ReadAllText(path!).Should().Be("a report"); + } + finally + { + File.Delete(path!); + } + } + + [Fact] + public void Failing_to_write_the_report_is_not_a_second_failure() + { + // Reporting runs when things are already bad, and a read-only or full disk is exactly when a + // tool is most likely to be failing. Returning null lets the caller fall back to stderr. + Func act = () => CrashReport.Write(null!, When); + + act.Should().NotThrow(); + } +}