Skip to content

Improve ConnectionListener#1438

Merged
scottf merged 7 commits intomainfrom
improve-connection-listener
Sep 29, 2025
Merged

Improve ConnectionListener#1438
scottf merged 7 commits intomainfrom
improve-connection-listener

Conversation

@scottf
Copy link
Contributor

@scottf scottf commented Sep 26, 2025

A new method with a default implementation is added to ConnectionListener. The caller of connection listeners now calls the new method allowing implementors to have extra details about the uri related to this connection event. This information was not always available from the Connection object server info, for instance on reconnect, the connection will be stale, and the details contain information about the uri trying to be connected to and the host uri if the try to connect uri is an ip address resolved from the host uri.

default void connectionEvent(Connection conn, Events type, String uriDetails) {
    connectionEvent(conn, type);
}


timeTraceLogger.trace("setting status to connecting");
updateStatus(Status.CONNECTING);
updateStatus(Status.CONNECTING, resolved, cur);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cur is the NatsUri from the server pool. resolved is either cur or a host resolved to an ip address

}

processConnectionEvent(Events.RESUBSCRIBED);
processConnectionEvent(Events.RESUBSCRIBED, uriDetail(currentServer));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

uri detail is a helper that converts the parameter to a string, making sure it's not null. Just a macro so I didn't have to have a lot of if == null checks

}
else {
double seconds = ((double)-remainingNanos) / 1_000_000_000.0;
double seconds = ((double) -remainingNanos) / 1_000_000_000.0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Um, thanks intellij?

// writer.stop
void tryToConnect(NatsUri cur, NatsUri resolved, long now) {
currentServer = null;
clearCurrentServer();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

clearCurrentServer sets current server to null after setting the lastServer variable, which helps reporting statuses during re/connect when current server needs to be null.

// https://github.com/nats-io/nats.java#linux-platform-note
timeTraceLogger.trace("TLS upgrade took: %.3f (s)",
((double) (NatsSystemClock.nanoTime() - start)) / NANOS_PER_SECOND);
((double) (NatsSystemClock.nanoTime() - start)) / NANOS_PER_SECOND);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

again, thanks intellij

future.get(timeoutNanos, TimeUnit.NANOSECONDS);
} finally {
}
finally {
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 must have formatted this entire method.

private void clearCurrentServer() {
if (currentServer != null) {
lastServer = currentServer;
}
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 know this looks weird, but this makes sure lastServer is retained when clearServer is called when currentServer is already null, it can happen.

if (!urls.isEmpty()) {
if (serverPool.acceptDiscoveredUrls(urls)) {
processConnectionEvent(Events.DISCOVERED_SERVERS);
processConnectionEvent(Events.DISCOVERED_SERVERS, urls.toString());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

now the connection event can see the discovered servers


if (serverInfo.isLameDuckMode()) {
processConnectionEvent(Events.LAME_DUCK);
processConnectionEvent(Events.LAME_DUCK, uriDetail(currentServer));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can see the server being lame ducked

this.callbackRunner.execute(() -> {
try {
listener.connectionEvent(this, type);
listener.connectionEvent(this, type, uriDetails);
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 the call to the new method in the ConnectionListener

*/
default void connectionEvent(Connection conn, Events type, String uriDetails) {
connectionEvent(conn, type);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

default impl required for backward compatibility. I've verified that there is no issue, I have lambda implementations of connection listener in unit tests, examples, tons of my scratch classes that are not checked in, etc. so I know this is safe to do and won't break anyone.

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 77f5797 into main Sep 29, 2025
5 checks passed
@scottf scottf deleted the improve-connection-listener branch September 29, 2025 16:42
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