Reset state on credential loading failure

[mtmk]

When ConnectAsync throws during credential loading (e.g. missing creds file), ConnectionState stays at Connecting and the waiter TCS is never completed. A second ConnectAsync call then hangs forever waiting on that TCS.

Wrap credential loading in InitialConnectAsync with a try-catch that resets ConnectionState to Closed and faults the waiter, matching the existing pattern for socket and setup failures. Also broaden the catch in LoadCredsFile from NatsException to Exception so IO errors like FileNotFoundException are wrapped properly.

fixes #1104

[synadia-claude-reviewer]

Claude finished @mtmk's task in 2m 1s —— View job

---

PR Review: Reset state on credential loading failure

• Read CLAUDE.md for repo guidelines
• Examine PR diff
• Read changed source files for context
• Analyze correctness and thread safety
• Post review findings

---

Summary

The fix is correct and well-targeted. The new try-catch in InitialConnectAsync (NatsConnection.cs:397-417) exactly mirrors the existing pattern already used for socket failures (line 437-448) and setup failures (line 451-470):

• Resets ConnectionState to Closed inside _gate lock (allows callers to retry)
• Faults _waitForOpenConnection TCS via TrySetObservedException (unblocks any concurrent ConnectAsync waiters)
• Immediately replaces the TCS so future callers get a fresh one
• Re-throws for the initiating caller

The LoadCredsFile broadening (NatsException → Exception) is the right call: new StreamReader(path) can throw FileNotFoundException, IOException, UnauthorizedAccessException, etc. Before this fix, those would escape the catch unwrapped and not be caught as NatsException in ConnectAsync's outer loop, leaving the connection state stuck at Connecting.

No double-wrapping: when LoadCredsFile throws NatsException, the ex as NatsException cast in InitialConnectAsync succeeds, so the original exception is used directly.

One minor consistency note

LoadCredsContent (line 149 of UserCredentials.cs, unchanged) still catches only NatsException, making the two loader methods asymmetric:

// LoadCredsContent – unchanged, still NatsException only
catch (NatsException e)

// LoadCredsFile – correctly broadened in this PR
catch (Exception e)

StringReader won't throw IO errors and ParseCreds only explicitly throws NatsException, so this isn't a real bug. But broadening LoadCredsContent to Exception would match the pattern for defensive completeness.

No issues found with:

• Thread safety: all state mutations are inside _gate lock
• TCS lifecycle: old TCS faulted before new one assigned — concurrent waiters get the exception
• RetryOnInitialConnect interaction: credential errors surface as NatsException, caught by ConnectAsync's retry loop — consistent with other failure modes
• Test: Task.WhenAny + 5 s timeout is the correct pattern for hang regression tests

Verdict: LGTM. The fix is minimal, correct, and consistent with the existing error-handling pattern.

[github-actions]

🚀 Deployed on https://69dcc0b680da0513d03f8e34--teal-klepon-7e92d0.netlify.app

[mtmk]

hey @Prochy are you able to confirm this fix is working for you?

[Prochy]

hey @Prochy are you able to confirm this fix is working for you?

Just tested, it works.

[scottf]

LGTM