Skip to content

Commit f10c1ee

Browse files
authored
fix: DH-20307: Throw errors when worker connection has been Disconnec… (#7362)
rc/0.40.x version of (#7175) DH-20307: Don't attempt to reconnect when connections are explicitly closed. This PR is necessary to support fixes made in [deephaven-ent/iris/#3518](deephaven-ent/iris#3518). Specifically, there is a race condition where a created Core+ JS API client can fail login and will continue to spam the server with login attempts that will never succeed. That PR handles the client side piece of this by explicitly disconnecting the client when this happens. The server side fix is to stop attempting to reconnect if a connection is explicitly closed. ### Testing I have tested the 2 PRs together and have seen that this PR + the other PR stops the rogue connection spamming. I have only seen change in `connectToWorker` method of `WorkerConnection.java` actually get hit, but I have also not observed any problems caused by it. (#DH-20307)
1 parent 52aa293 commit f10c1ee

2 files changed

Lines changed: 13 additions & 3 deletions

File tree

web/client-api/src/main/java/io/deephaven/web/client/api/CoreClient.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ public Promise<Void> login(@TsTypeRef(LoginCredentials.class) JsPropertyMap<Obje
122122
LazyPromise<Void> loginPromise = new LazyPromise<>();
123123
ideConnection.addEventListenerOneShot(
124124
EventPair.of(QueryInfoConstants.EVENT_CONNECT, ignore -> loginPromise.succeed(null)),
125-
EventPair.of(CoreClient.EVENT_RECONNECT_AUTH_FAILED, loginPromise::fail));
125+
EventPair.of(CoreClient.EVENT_DISCONNECT, loginPromise::fail),
126+
EventPair.of(CoreClient.EVENT_RECONNECT_AUTH_FAILED, loginPromise::fail),
127+
EventPair.of(CoreClient.EVENT_REQUEST_FAILED, loginPromise::fail));
126128
Promise<Void> login = loginPromise.asPromise();
127129

128130
if (alreadyRunning) {

web/client-api/src/main/java/io/deephaven/web/client/api/WorkerConnection.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,12 @@ private void connectToWorker() {
332332

333333
return Promise.resolve((Object) null);
334334
}, fail -> {
335+
// Connection was explicitly closed. We don't want to change
336+
// the status unless a `forceReconnect` is called.
337+
if (state == State.Disconnected) {
338+
return null;
339+
}
340+
335341
// this is non-recoverable, connection/auth/registration failed, but we'll let it start again when
336342
// state changes
337343
state = State.Failed;
@@ -909,8 +915,9 @@ private void notifyFieldsChangeListeners(JsVariableChanges update) {
909915

910916
public Promise<Object> whenServerReady(String operationName) {
911917
switch (state) {
912-
case Failed:
913918
case Disconnected:
919+
throw new IllegalStateException("Can't " + operationName + " while connection is closed");
920+
case Failed:
914921
state = State.Reconnecting;
915922
newSessionReconnect.initialConnection();
916923
// deliberate fall-through
@@ -1389,8 +1396,9 @@ public void onOpen(BiConsumer<Void, String> callback) {
13891396
case Connected:
13901397
LazyPromise.runLater(() -> callback.accept(null, null));
13911398
break;
1392-
case Failed:
13931399
case Disconnected:
1400+
throw new IllegalStateException("Can't add onOpen callback when connection is closed");
1401+
case Failed:
13941402
state = State.Reconnecting;
13951403
newSessionReconnect.initialConnection();
13961404
// intentional fall-through

0 commit comments

Comments
 (0)