Reconnect Delay Behavior and options cleanup

[scottf]

• adds the new ReconnectDelayBehavior enum (BeforeSubsequentRounds default, BeforeAllRounds new) wired into NatsConnection's reconnect loop
• add the PROP_RECONNECT_DELAY_HANDLER_CLASS and PROP_RECONNECT_DELAY_BEHAVIOR properties.
• adds SubjectValidationType.get(String) and the PROP_SUBJECT_VALIDATION_TYPE property
  • deprecates PROP_NO_SUBJECT_VALIDATION / PROP_STRICT_SUBJECT_VALIDATION and the corresponding builder methods.
• Includes OptionsTests coverage and a CoverageReconnectDelayHandler test utility.

[synadia-claude-reviewer]

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

---

PR Review

• Gather context and understand the request
• Read PR diff and changed files
• Review Options.java changes
• Review NatsConnection.java changes
• Review OptionsTests.java changes
• Post inline comments for specific issues
• Post overall review summary

---

Summary

The PR is well-structured: clean enum design, consistent get(String) factory on both enums, proper deprecation of the old boolean properties, good null-safety in the builder. Tests cover the Options API thoroughly. A few issues to address:

Bugs / must-fix

1. Copy-paste Javadoc on ReconnectDelayBehavior — the class-level doc still says "Whether subject strings should be validated…" (copied from SubjectValidationType). Inline comment on Options.java:299

Design / worth clarifying

2. Every mode delays before the first attempt — invokeReconnectDelayHandler(0) fires before any server is tried, so a configured reconnectWait blocks even the first reconnect attempt. The Every Javadoc says "before each full round" which is ambiguous about round 0. If this is intentional, tightening the Javadoc to say "including before the first attempt" would prevent surprises. Inline comment on NatsConnection.java:450

Minor observations

3. PROP_RECONNECT_DELAY_HANDLER_CLASS missing PFX prefix — "reconnect.delay.handler.class" (no io.nats.client. prefix), unlike PROP_RECONNECT_DELAY_BEHAVIOR and most other constants. getPropertyValue handles both forms, so it's functional. The same pattern exists on PROP_SSL_CONTEXT_FACTORY_CLASS, so it appears intentional for class-name properties, but the inconsistency could confuse users setting this via system properties. Inline comment on Options.java:502

4. No test for PROP_RECONNECT_DELAY_HANDLER_CLASS — the new class-name property has no coverage in OptionsTests. A test setting that property to a concrete ReconnectDelayHandler implementation class would close this gap.

5. No integration test for Every reconnect timing — the new tests verify the Options API in isolation, but there's no test that verifies Every mode actually produces a delay before the first connection attempt. Given this is the core behavioral change, a unit/integration test against a mock server pool would add confidence.

[scottf]

not going to implement integration test for reconnect timing

[scottf]

@claude please re-review

[synadia-claude-reviewer]

Claude finished @scottf's task in 1m 51s —— View job

---

Re-Review

• Read current state of changed files
• Verify fixes for previously reported issues
• Check for any remaining or new issues
• Post updated review

---

All five findings from the previous review have been addressed:

1. Copy-paste Javadoc on ReconnectDelayBehavior — Fixed. The class-level doc now correctly reads "Controls when the ReconnectDelayHandler is invoked during reconnect attempts." (Options.java:296)

2. Every mode / first-round delay ambiguity — Fixed. The enum constant was renamed from Every to BeforeAllRounds, and its Javadoc now states explicitly that getWaitTime "will be called with totalTries greater than or equal to 0", making round-0 behaviour clear. (Options.java:308–311)

3. PROP_RECONNECT_DELAY_HANDLER_CLASS missing PFX — Fixed. The constant is now defined as PFX + "reconnect.delay.handler.class", consistent with the rest of the file. (Options.java:502)

4. No test for PROP_RECONNECT_DELAY_HANDLER_CLASS — Fixed. A new CoverageReconnectDelayHandler class was added and the existing test was extended to cover: successful reflective instantiation, correct return value from getWaitTime, and IllegalArgumentException on an unknown class name. (OptionsTests.java:1417–1435, CoverageReconnectDelayHandler.java)

5. No integration test for Every/BeforeAllRounds reconnect timing — Not implemented (acknowledged by author).

The code is clean. No new issues found.

[Jarema]

LGTM