Skip to content

fix: preserve DevTunnel error message when HostAsync fails#283

Merged
PureWeen merged 2 commits intomainfrom
fix/can-you-figure-out-why-when-i-click-star-20260304-2347
Mar 5, 2026
Merged

fix: preserve DevTunnel error message when HostAsync fails#283
PureWeen merged 2 commits intomainfrom
fix/can-you-figure-out-why-when-i-click-star-20260304-2347

Conversation

@PureWeen
Copy link
Copy Markdown
Owner

@PureWeen PureWeen commented Mar 4, 2026

Problem

When clicking "Start Dev Tunnels", the button briefly flashes a spinner then returns to its initial state with no error message — no QR code appears and no explanation is shown.

Root Cause

In DevTunnelService.HostAsync(), when TryHostTunnelAsync() fails (both initial and retry attempts), Stop() is called for cleanup at line 237. Stop() transitions through Stopping → NotStarted, and SetState(NotStarted) clears _errorMessage (since only the Error state preserves it). The error that was set by SetError() during the failed attempt is wiped before the UI can display it.

The UI then shows the start button again (state = NotStarted) instead of the error panel (state = Error).

Fix

  1. Preserve error across Stop(): Save _errorMessage before Stop(), then re-set it via SetError() after cleanup so the user sees what went wrong.
  2. Catch block cleanup: Call Stop() before SetError() in the catch block so the WsBridge started earlier in the method is properly cleaned up on unexpected exceptions.

Tests Added

  • Stop_ClearsErrorMessage_ByDesign — confirms Stop() clears error (documents the root cause behavior)
  • HostAsync_WhenTunnelFails_PreservesErrorMessage — end-to-end: verifies failed HostAsync ends in Error state with a message
  • ErrorPreservation_SaveAndRestore_AcrossStop — tests the save/restore pattern used in the fix
  • SetError_SetsStateToError_AndPreservesMessage — verifies SetError behavior
  • SetError_FiresOnStateChanged — verifies event fires

All 85 DevTunnel tests pass. All 1970 passing tests remain green (13 pre-existing PopupThemeTests failures unrelated).

@PureWeen
Copy link
Copy Markdown
Owner Author

PureWeen commented Mar 5, 2026

PR Review Squad — PR #283 Round 1

CI Status: ⚠️ No checks reported

Summary

Good fix — preserves DevTunnel error message across Stop() call using save/restore pattern. Catch block correctly adds Stop() before SetError().

🟡 MODERATE

  1. DevTunnelServiceTests.cs:376-398 (5/5) — HostAsync_WhenTunnelFails_PreservesErrorMessage is vacuous: all assertions are inside if (!result). If devtunnel CLI is installed in the test environment, HostAsync returns true, the if block is skipped, and the test passes without validating anything. Add Assert.False(result) unconditionally or use a mock/stub.

  2. DevTunnelService.cs:237-241 (3/5) — if (!string.IsNullOrEmpty(lastError)) skips error restore if TryHostTunnelAsync returned false without calling SetError. Service lands in NotStarted with no error — the original bug. Fix: SetError(lastError ?? "DevTunnel failed to start").

🟢 MINOR

  • DevTunnelService.cs:235 (5/5) — _errorMessage race with Exited handler (tiny window, low risk)
  • DevTunnelService.cs:237 (2/5) — Triple OnStateChanged notification (Stop + SetError)

Recommended Action: ⚠️ Request changes

Two specific asks: (1) make the test non-vacuous, (2) add fallback error message for null lastError.

PureWeen and others added 2 commits March 5, 2026 09:58
When HostAsync() failed to start a tunnel, Stop() was called for cleanup
which transitions state to NotStarted, clearing _errorMessage via
SetState(). This caused the UI to flash back to the initial state with
no error feedback — the user saw no QR code and no explanation.

Fix: Save _errorMessage before Stop() and restore it via SetError()
afterward so the Error state and message are visible to the user.

Also fix the catch block to call Stop() before SetError() so the
WsBridge started earlier in the method is properly cleaned up on
unexpected exceptions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
  add Assert.False(result) so the test always validates the failure path
  SetError(lastError ?? "DevTunnel failed to start") so failed HostAsync
  always ends in Error state even when TryHostTunnelAsync returns false
  without setting an error message

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen PureWeen force-pushed the fix/can-you-figure-out-why-when-i-click-star-20260304-2347 branch from f6bd3b7 to 45a5dad Compare March 5, 2026 16:00
@PureWeen PureWeen merged commit 92a0182 into main Mar 5, 2026
@PureWeen PureWeen deleted the fix/can-you-figure-out-why-when-i-click-star-20260304-2347 branch March 5, 2026 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant