Skip to content

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

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

scottf merged 2 commits into
mainfrom
auth-error-b4-connected

Conversation

@scottf

@scottf scottf commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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);
}

@scottf scottf Nov 7, 2025 •

Copy link
Copy Markdown
Contributor Author

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

@MauriceVanVeen MauriceVanVeen left a comment

Copy link
Copy Markdown
Member

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