diff --git a/docs/platform-flags.md b/docs/platform-flags.md index e39db2a..dd3de74 100644 --- a/docs/platform-flags.md +++ b/docs/platform-flags.md @@ -89,10 +89,19 @@ the case it was attached to. A process-level check cannot verify a rendering cla screenshot, and this defect shipped because one was never taken. **What the first connection after unlocking actually depends on** is the `await -workspace.WaitForRendererAsync()` in `VaultViewModel.ConnectAsync`, because `TerminalDataPlane.SendAsync` -drops frames when no renderer is attached rather than queueing them. That await is the invariant; the -control's visibility is not. It currently has no timeout, so a WebView2 that fails to initialise hangs -Connect with the busy flag stuck — worth fixing on its own merits. +workspace.WaitForRendererAsync(cancellationToken)` in `VaultViewModel.ConnectAsync`, because +`TerminalDataPlane.SendAsync` drops frames when no renderer is attached rather than queueing them. That +await is the invariant; the control's visibility is not. + +It is now bounded — `TerminalWorkspaceOptions.RendererTimeout`, 15 s, plus the command's own token — +because whether the renderer attaches at all depends on a runtime this application does not install. A +missing or policy-blocked Evergreen runtime, or an AppContainer that cannot reach loopback, previously +left Connect waiting forever with `IsBusy` stuck and nothing on screen to explain it. The gate is +unchanged; only the wait is. Why 15 s and not less: attaching is near-instant in the normal case (the page +attaches while the unlock screen is still up), but a cold WebView2 profile creates a user-data directory +and starts its process tree first, and reporting a broken runtime to someone whose runtime was merely slow +is the worse error. The timeout is caught in `VaultViewModel` and reported as a message naming WebView2, +because `TimeoutException.Message` is "The operation has timed out" and names nothing. **Hiding the WebView does not pause it.** With the holder window hidden, the page keeps `visibilityState: "visible"` and `requestAnimationFrame` keeps firing at roughly 115/s — Chromium does not diff --git a/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs index c3f5591..897037d 100644 --- a/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs @@ -412,13 +412,14 @@ internal sealed partial class VaultViewModel( /// /// The renderer has to be attached before a session opens: the transport drops frames when nothing is /// connected, so a session opened earlier would lose its SessionOpened frame and then stream - /// output at a terminal that was never created. + /// output at a terminal that was never created. That wait is bounded and takes this command's token, so + /// a renderer that never arrives ends as a message rather than as a window stuck on "Connecting…". /// private async Task OpenSessionAsync(HostRowViewModel row, CancellationToken cancellationToken) { try { - await workspace.WaitForRendererAsync().ConfigureAwait(true); + await workspace.WaitForRendererAsync(cancellationToken).ConfigureAwait(true); var request = new SshConnectionRequest( row.Host.Hostname, @@ -432,6 +433,14 @@ internal sealed partial class VaultViewModel( Status = $"Connected to {row.Label}."; } + catch (TimeoutException) + { + // The renderer never attached, so nothing was connected. Reported here rather than left to + // RunAsync's generic handler because TimeoutException says only "The operation has timed out", + // and the one thing worth saying is where to look: a runtime this application does not install. + Status = "The terminal did not start, so nothing was connected. The Microsoft Edge WebView2 " + + "runtime is probably missing or blocked; install it and try again."; + } catch (SshHostKeyUnknownException exception) { // First contact. The user has to decide, and they need the fingerprint to do it. diff --git a/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs b/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs index c62a28d..22f8f51 100644 --- a/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs +++ b/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs @@ -2,6 +2,31 @@ using DodoSSH.Client.Ssh; namespace DodoSSH.Client.Terminal; +/// Tuning for the workspace. +public sealed class TerminalWorkspaceOptions +{ + /// + /// How long waits for the renderer's socket + /// before giving up. + /// + /// + /// + /// The value has to separate two cases. Attaching is normally near-instant: WebView2 starts with the + /// window and the page has usually attached while the user was still typing a passphrase. But a first + /// run on a cold profile creates a user-data directory and starts a process tree of some thirty-five + /// processes, and on a slow or loaded machine that is seconds rather than milliseconds. A renderer + /// that will never attach — no Evergreen runtime, an install blocked by policy, an AppContainer that + /// cannot reach loopback — will not attach however long the wait is. + /// + /// + /// So being generous costs only how long a genuinely broken WebView2 takes to say so, while being + /// tight costs telling someone their runtime is broken when it was merely slow. Fifteen seconds is + /// well clear of any cold start observed here and is still an answer rather than a hang. + /// + /// + public TimeSpan RendererTimeout { get; init; } = TimeSpan.FromSeconds(15); +} + /// /// Owns the loopback data plane and every live terminal session. /// @@ -16,6 +41,7 @@ public sealed class TerminalWorkspace : IAsyncDisposable private readonly TerminalDataPlane dataPlane; private readonly ISshConnectionFactory connections; private readonly TimeProvider clock; + private readonly TerminalWorkspaceOptions options; private readonly Dictionary sessions = []; private readonly CancellationTokenSource lifetime = new(); @@ -23,13 +49,19 @@ public sealed class TerminalWorkspace : IAsyncDisposable private Task? server; private int disposed; + /// Where the renderer's files come from. + /// How SSH connections are made. + /// Time source, so the pumps' flush interval is testable. + /// Tuning, or null for the defaults. public TerminalWorkspace( ITerminalAssetProvider assets, ISshConnectionFactory connections, - TimeProvider clock) + TimeProvider clock, + TerminalWorkspaceOptions? options = null) { this.connections = connections; this.clock = clock; + this.options = options ?? new TerminalWorkspaceOptions(); dataPlane = new TerminalDataPlane(assets); } @@ -44,11 +76,25 @@ public sealed class TerminalWorkspace : IAsyncDisposable /// Waits until the renderer page has attached its socket. /// /// - /// A session opened before the renderer attaches would have its SessionOpened frame - /// dropped — the transport discards frames when nothing is connected — leaving output arriving - /// for a terminal that was never created. + /// + /// A session opened before the renderer attaches would have its SessionOpened frame dropped — + /// the transport discards frames when nothing is connected — leaving output arriving for a terminal + /// that was never created. The gate is the invariant and stays. + /// + /// + /// Bounded, because whether the renderer attaches at all depends on a WebView2 runtime this process + /// does not install. An unbounded wait turned a missing runtime into a Connect that never returned, + /// with the caller's busy state never cleared and nothing on screen to explain it. Callers are + /// expected to translate the timeout into something that names the runtime, because + /// 's own message names nothing. + /// /// - public Task WaitForRendererAsync() => dataPlane.RendererAttached; + /// Abandons the wait. + /// + /// No renderer attached within . + /// + public Task WaitForRendererAsync(CancellationToken cancellationToken) => + dataPlane.RendererAttached.WaitAsync(options.RendererTimeout, cancellationToken); /// Connects to a host and starts a terminal for it. /// The session id, which identifies this terminal in the renderer. diff --git a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs index 5324310..03407a3 100644 --- a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs +++ b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs @@ -51,6 +51,10 @@ public sealed class ShellFlowTests : IAsyncLifetime // In-memory assets rather than the application's Avalonia-resource provider, which reads the // resource system at construction and needs an initialised toolkit. This is what // ITerminalAssetProvider is for; nothing in this suite renders anything. + // + // The renderer timeout is cut to milliseconds because of that: no WebView exists here, so any + // connect waits it out in full, and the shipped fifteen seconds would be fifteen seconds of a + // test suite sitting still. workspace = new TerminalWorkspace( new InMemoryTerminalAssetProvider( new Dictionary(StringComparer.Ordinal) @@ -58,7 +62,8 @@ public sealed class ShellFlowTests : IAsyncLifetime ["/terminal"] = new("text/html; charset=utf-8", ""u8.ToArray()), }), new SshNetConnectionFactory(knownHosts), - TimeProvider.System); + TimeProvider.System, + new TerminalWorkspaceOptions { RendererTimeout = TimeSpan.FromMilliseconds(250) }); shell = new MainWindowViewModel( paths, @@ -399,6 +404,29 @@ public sealed class ShellFlowTests : IAsyncLifetime server.PushCount.ShouldBe(0); } + /// + /// The workspace in this suite is never started and nothing ever attaches a renderer, which from the + /// view model's side is indistinguishable from a WebView2 that failed to initialise on a user's machine. + /// The interesting assertion is the second one: while the wait was unbounded this hung with the busy + /// flag set, so the window stayed disabled and said "Connecting…" for the rest of the session. + /// + [Fact] + public async Task ConnectingWithNoRenderer_ExplainsItselfAndReleasesTheWindow() + { + await UnlockedAsync(); + var vault = shell.Vault!; + + await AddHostAsync(vault, "prod-db"); + vault.SelectedHost = vault.Hosts[0]; + + await vault.ConnectCommand.ExecuteAsync(null); + + // Naming the runtime is the whole point: a bare "The operation has timed out" sends someone + // looking at their network or their host. + vault.Status.ShouldContain("WebView2"); + vault.IsBusy.ShouldBeFalse("a connect that gave up must not leave the window disabled"); + } + [Fact] public async Task LockingForgetsTheVault() { diff --git a/tests/DodoSSH.Client.Ssh.Tests/TerminalEndToEndTests.cs b/tests/DodoSSH.Client.Ssh.Tests/TerminalEndToEndTests.cs index 22c1d00..a7ec0e0 100644 --- a/tests/DodoSSH.Client.Ssh.Tests/TerminalEndToEndTests.cs +++ b/tests/DodoSSH.Client.Ssh.Tests/TerminalEndToEndTests.cs @@ -57,7 +57,7 @@ public sealed class TerminalEndToEndTests(SshServerFixture fixture) var token = await ReadTokenAsync(workspace.PageUrl); using var renderer = await AttachAsync(workspace.PageUrl, token); - await workspace.WaitForRendererAsync(); + await workspace.WaitForRendererAsync(TestContext.Current.CancellationToken); var sessionId = await OpenTrustedSessionAsync(workspace, knownHosts); diff --git a/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs new file mode 100644 index 0000000..8205e82 --- /dev/null +++ b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs @@ -0,0 +1,66 @@ +using System.Text; +using DodoSSH.Client.Ssh; +using NSubstitute; + +namespace DodoSSH.Client.Terminal.Tests; + +/// +/// The renderer gate: the one place the workspace waits on something outside the process. +/// +/// +/// The gate itself is not in question. drops frames when nothing +/// is attached, so a session opened before the renderer arrives loses its SessionOpened frame and +/// streams output at a terminal that was never created — and that it opens when a renderer does attach is +/// covered by . What is worth a test here is the +/// half that used to be missing: waiting for a renderer that never arrives has to end. +/// +public sealed class TerminalWorkspaceTests +{ + [Fact] + public async Task WaitingForARendererThatNeverAttaches_GivesUp() + { + // The shipped failure this stands in for is a WebView2 that never initialises — no Evergreen + // runtime, an install blocked by policy, an AppContainer that cannot reach loopback. From this + // side they are identical and all look like the listener being up with nothing ever connecting + // to it. Before the wait was bounded this test would have hung instead of failing. + await using var workspace = CreateWorkspace(TimeSpan.FromMilliseconds(250)); + workspace.Start(); + + await Should.ThrowAsync(async () => + await workspace.WaitForRendererAsync(TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task WaitingForARenderer_ObeysItsCancellationToken() + { + // The timeout is the backstop; the caller's token is what makes a Connect the user gave up on + // return at once rather than sitting out the rest of the wait. The timeout here is long enough + // that only cancellation can end this. + await using var workspace = CreateWorkspace(TimeSpan.FromMinutes(5)); + workspace.Start(); + + using var cancellation = new CancellationTokenSource(); + var wait = workspace.WaitForRendererAsync(cancellation.Token); + + await cancellation.CancelAsync(); + + await Should.ThrowAsync(async () => await wait); + } + + /// + /// The connection factory is never reached: every test here stops at the gate, and reaching a real + /// host would make this a network test. + /// + private static TerminalWorkspace CreateWorkspace(TimeSpan rendererTimeout) => + new( + new InMemoryTerminalAssetProvider( + new Dictionary(StringComparer.Ordinal) + { + [TerminalDataPlane.PagePath] = new( + "text/html; charset=utf-8", + Encoding.UTF8.GetBytes("")), + }), + Substitute.For(), + TimeProvider.System, + new TerminalWorkspaceOptions { RendererTimeout = rendererTimeout }); +}