[scottf]

Follows #1663, now merged. Replaces #1664, which GitHub closed when the base branch was deleted. Two changes to how the client handles TLS between the options and the servers.

A full round of TLS mismatches closes the connection. A mismatch between the options and a server still fails that attempt with the same message as before, "SSL required by server." when the options have no SSL context and the server requires TLS, or "SSL connection wanted by client." when the options have a context and the server offers no TLS, and the next server is tried, since a single server can be misconfigured. The options decide TLS for every server of a connection, so when every attempt of one full round over the server list fails with the mismatch, retrying cannot help. The initial connect then closes the connection and throws an IOException that starts with the mismatch text and says the connection was closed without retrying, also with reconnect-on-connect, instead of entering the reconnect loop. A reconnect round in which every attempt was a mismatch ends with the connection closed. A server that cannot be reached is not a mismatch, so a round that contains one is inconclusive and the normal reconnect rules apply. The round in the reconnect loop is detected the same way the reconnect delay handler detects it. Error listeners see one exception per attempt, as before, so a client whose every server requires TLS now reports one exception per server instead of one per retry.

tlsFirst() asks for TLS. Without any other TLS setting it selects SSLContext.getDefault() in build(), the same as secure(), instead of failing with "SSL context required for tls handshake first". That error remains only for an SSLContextFactory that returns null. A supplied context, keystore and truststore, opentls() or an opentls:// url still take precedence.

Docs: README "TLS client versus server checks" keeps its table and gains the round rule; docs/TLS_CONFIGURATION.md rules 5 to 8.

Tests, TLSConnectTests: two servers that require TLS without a context, and two plain servers with a context, each close and throw after one attempt per server with no reconnect wait; a server that requires TLS next to a plain one, in both orders, connects to the matching server; a server that requires TLS next to an unreachable port keeps reconnecting until the budget is used; a connected client whose single server starts requiring TLS closes after one reconnect attempt with reconnects remaining; tlsFirst() without a context connects to a handshake_first server. OptionsTests covers the tlsFirst() context selection. The two closing tests fail on the base branch.

[synadia-claude-reviewer]

Claude finished @scottf's task in 3m 57s —— View job

---

Review of PR #1665

• Gather context (PR diff, description, related files)
• Review NatsConnection.java changes (TLS mismatch / close-after-round logic)
• Review Options.java changes (tlsFirst() default context)
• Review test changes for coverage/correctness
• Review docs changes
• Post review summary

Summary

Both changes are implemented correctly and the control-flow reasoning checks out against the test cases.

Options.java (tlsFirst() default context): The new if (tlsFirst && !useTrustAllTls) { useDefaultTls = true; } (lines 2488-2491) is placed inside the else branch that only runs when no sslContextFactory was supplied, so the "SSLContextFactory returning null" error path (line 2521-2524) is preserved exactly as intended — that's the only way sslContext can still be null at that check. The guard against useTrustAllTls correctly lets opentls()/opentls:// take precedence without tripping the "both secure and opentls" conflict check right below it. This matches the described precedence rules and the new OptionsTests.testTlsFirstAsksForTls covers the relevant combinations.

NatsConnection.java (close after a full mismatch round): attemptTlsMismatch is written in upgradeToSecureIfNeeded (lines 804/810) purely from the already-parsed ServerInfo — no blocking I/O happens between the write and the throw, so the write is safely visible to the loop thread via the Future.get() happens-before edge before connectImpl/reconnectImplConnect read it on the next attempt; there's no realistic cross-attempt race despite the field being shared mutable state.

• connectImpl: the full-round check (tlsMismatch != null && everyAttemptTlsMismatch) is evaluated after the server-list loop, before the reconnectOnConnect branch, so it correctly fires even when reconnectOnConnect is true, matching the PR description and testTlsMismatchWithEveryServerClosesWithoutRetrying.
• reconnectImplConnect: the round boundary is detected the same way as invokeReconnectDelayHandler (first.equals(cur)), and roundAllTlsMismatch/roundAttempts are reset only when a round wasn't fully mismatched — a single-server pool correctly triggers the close on the first wrap-around, matching testServerStartsRequiringTlsClientHasNoContextClosesOnReconnect.
• A non-mismatch failure (e.g. connection refused) correctly marks the round inconclusive (everyAttemptTlsMismatch/roundAllTlsMismatch = false), so a round containing an unreachable server still falls through to normal reconnect behavior (testTlsMismatchNextToAnUnreachableServerKeepsReconnecting).

No correctness, security, or thread-safety issues found. Tests look like they exercise the specific new branches (closing after one round, mismatch-then-working-server ordering in both directions, inconclusive rounds, reconnect-time mismatch, tlsFirst default context), and the docs changes accurately reflect the new behavior.

[aricart]

LGTM