Skip to content

Commit 264e185

Browse files
[Java] Catch exceptions from users and change close state correctly (dotnet#29249)
* [Java] Catch exceptions from users and change close state correctly * log * link
1 parent fe37d39 commit 264e185

File tree

2 files changed

+34
-10
lines changed

2 files changed

+34
-10
lines changed

src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -517,33 +517,44 @@ private void stopConnection(String errorMessage) {
517517
RuntimeException exception = null;
518518
this.state.lock();
519519
try {
520+
ConnectionState connectionState = this.state.getConnectionStateUnsynchronized(true);
521+
522+
if (connectionState == null)
523+
{
524+
this.logger.error("'stopConnection' called with a null ConnectionState. This is not expected, please file a bug. https://github.com/dotnet/aspnetcore/issues/new?assignees=&labels=&template=bug_report.md");
525+
return;
526+
}
527+
520528
// errorMessage gets passed in from the transport. An already existing stopError value
521529
// should take precedence.
522-
if (this.state.getConnectionStateUnsynchronized(false).stopError != null) {
523-
errorMessage = this.state.getConnectionStateUnsynchronized(false).stopError;
530+
if (connectionState.stopError != null) {
531+
errorMessage = connectionState.stopError;
524532
}
525533
if (errorMessage != null) {
526534
exception = new RuntimeException(errorMessage);
527535
logger.error("HubConnection disconnected with an error {}.", errorMessage);
528536
}
529537

530-
ConnectionState connectionState = this.state.getConnectionStateUnsynchronized(true);
531-
if (connectionState != null) {
532-
connectionState.cancelOutstandingInvocations(exception);
533-
connectionState.close();
534-
this.state.setConnectionState(null);
535-
}
538+
this.state.setConnectionState(null);
539+
connectionState.cancelOutstandingInvocations(exception);
540+
connectionState.close();
536541

537542
logger.info("HubConnection stopped.");
538-
this.state.changeState(HubConnectionState.CONNECTED, HubConnectionState.DISCONNECTED);
543+
// We can be in the CONNECTING or CONNECTED state here, depending on if the handshake response was received or not.
544+
// connectionState.close() above will exit the Start call with an error if it's still running
545+
this.state.changeState(HubConnectionState.DISCONNECTED);
539546
} finally {
540547
this.state.unlock();
541548
}
542549

543550
// Do not run these callbacks inside the hubConnectionStateLock
544551
if (onClosedCallbackList != null) {
545552
for (OnClosedCallback callback : onClosedCallbackList) {
546-
callback.invoke(exception);
553+
try {
554+
callback.invoke(exception);
555+
} catch (Exception ex) {
556+
logger.warn("Invoking 'onClosed' method failed:", ex);
557+
}
547558
}
548559
}
549560
}
@@ -1519,6 +1530,16 @@ public void changeState(HubConnectionState from, HubConnectionState to) {
15191530
}
15201531
}
15211532

1533+
public void changeState(HubConnectionState to) {
1534+
this.lock.lock();
1535+
try {
1536+
logger.debug("The HubConnection is transitioning from the {} state to the {} state.", this.hubConnectionState, to);
1537+
this.hubConnectionState = to;
1538+
} finally {
1539+
this.lock.unlock();
1540+
}
1541+
}
1542+
15221543
public void lock() {
15231544
this.lock.lock();
15241545
}

src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ public void onClosing(WebSocket webSocket, int code, String reason) {
125125
stateLock.unlock();
126126
}
127127
checkStartFailure(null);
128+
129+
// Send the close frame response if this was a server initiated close, otherwise noops
130+
webSocket.close(1000, "");
128131
}
129132

130133
@Override

0 commit comments

Comments
 (0)