Skip to content

Auth Error is allowed to be set before being connected#1475

Merged
scottf merged 2 commits intomainfrom
auth-error-b4-connected
Nov 7, 2025
Merged

Auth Error is allowed to be set before being connected#1475
scottf merged 2 commits intomainfrom
auth-error-b4-connected

Conversation

@scottf
Copy link
Contributor

@scottf scottf commented Nov 7, 2025

No description provided.

// If we are connected && we get an authentication error, save it
if (this.isConnected() && this.isAuthenticationError(errorText) && currentServer != null) {
// If we get an authentication error, save it
if (this.isAuthenticationError(errorText) && currentServer != null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can happen on any login where the credentials are wrong

jsServer.close();
if (jsServer != null) {
jsServer.close();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a code guard

try (NatsTestServer ts = new NatsTestServer(
NatsTestServer.builder()
.configFilePath("src/test/resources/tls_first.conf")
.connectValidateTlsFirstMode())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newest server runner validates connection in a different way, so it needs to know when it's tls first.

.connectValidateInitialDelay(100L)
.connectValidateSubsequentDelay(25L)
.connectValidateTries(15)
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want the long running server to have plenty of time to start up.

.maxReconnects(0)
.errorListener(new ErrorListener() {})
.token("notthetoken".toCharArray())
.build();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the test that was flapping/failing

// See the License for the specific language governing permissions and
// limitations under the License.

package io.nats.client;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why the diff is so large here, not that much was changed

NatsServerRunner.setDefaultValidateTries(5);
NatsServerRunner.setDefaultInitialValidateDelay(100);
NatsServerRunner.setDefaultSubsequentValidateDelay(50);
}
Copy link
Contributor Author

@scottf scottf Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This static block is the only change.

@scottf scottf requested a review from MauriceVanVeen November 7, 2025 00:49
Copy link
Member

@MauriceVanVeen MauriceVanVeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@scottf scottf merged commit 9809a26 into main Nov 7, 2025
5 checks passed
@scottf scottf deleted the auth-error-b4-connected branch November 7, 2025 13:04
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.

2 participants