Skip to content

Commit 0605fb5

Browse files
While sanity-checking new feature, found the NPE was still being thrown on disconnect. Traced problem to NPE due to no onDisconnect being registered and no check against nulls when invoked. Added wrapping method that is null safe. Since disconnect is different from close, added an onClose event registry and test for that. Checked tests, all pass.
1 parent bf218a6 commit 0605fb5

File tree

5 files changed

+92
-14
lines changed

5 files changed

+92
-14
lines changed

src/main/java/net/twasi/obsremotejava/OBSCommunicator.java

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,7 @@
44
import com.google.gson.JsonElement;
55
import com.google.gson.JsonObject;
66
import com.google.gson.JsonParser;
7-
import net.twasi.obsremotejava.callbacks.Callback;
8-
import net.twasi.obsremotejava.callbacks.ErrorCallback;
9-
import net.twasi.obsremotejava.callbacks.StringCallback;
10-
import net.twasi.obsremotejava.callbacks.VoidCallback;
7+
import net.twasi.obsremotejava.callbacks.*;
118
import net.twasi.obsremotejava.events.EventType;
129
import net.twasi.obsremotejava.events.responses.*;
1310
import net.twasi.obsremotejava.objects.throwables.InvalidResponseTypeError;
@@ -113,6 +110,7 @@ public class OBSCommunicator {
113110

114111
private Callback<GetVersionResponse> onConnect;
115112
private VoidCallback onDisconnect;
113+
private CloseCallback onClose;
116114
private StringCallback onConnectionFailed;
117115
private ErrorCallback onError;
118116

@@ -162,12 +160,9 @@ public void onError(Session session, Throwable throwable) {
162160
@OnWebSocketClose
163161
public void onClose(int statusCode, String reason) {
164162
log.info(String.format("Connection closed: %d - %s%n", statusCode, reason));
163+
runOnDisconnect();
165164
this.closeLatch.countDown(); // trigger latch
166-
try {
167-
this.onDisconnect.run();
168-
} catch (Throwable t) {
169-
log.error("Unable to disconnect OBS Client", t);
170-
}
165+
runOnClosed(statusCode, reason);
171166
}
172167

173168
@OnWebSocketConnect
@@ -391,6 +386,8 @@ public void registerOnDisconnect(VoidCallback onDisconnect) {
391386
this.onDisconnect = onDisconnect;
392387
}
393388

389+
public void registerOnClose(CloseCallback closeCallback) { this.onClose = closeCallback; }
390+
394391
public void registerOnConnectionFailed(StringCallback onConnectionFailed) {
395392
this.onConnectionFailed = onConnectionFailed;
396393
}
@@ -726,4 +723,34 @@ private void runOnConnect(GetVersionResponse versionInfo) {
726723
log.error("Unable to run OnConnect callback", t);
727724
}
728725
}
726+
727+
void runOnDisconnect() {
728+
log.debug("Running onDisconnect");
729+
if (onDisconnect == null) {
730+
log.debug("No onDisconnect callback was registered");
731+
return;
732+
}
733+
734+
try {
735+
onDisconnect.run();
736+
} catch (Throwable t) {
737+
log.error("Unable to run OnDisconnect callback", t);
738+
}
739+
}
740+
741+
742+
private void runOnClosed(int statusCode, String reason) {
743+
log.debug("Running onClose with statusCode " + statusCode + " and reason: " + reason);
744+
745+
if(this.onClose == null) {
746+
log.debug("No onClose was registered.");
747+
return;
748+
}
749+
750+
try {
751+
onClose.run(statusCode, reason);
752+
} catch (Throwable t) {
753+
log.error("Unable to run onClose callback", t);
754+
}
755+
}
729756
}

src/main/java/net/twasi/obsremotejava/OBSRemoteController.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package net.twasi.obsremotejava;
22

33
import com.google.gson.Gson;
4-
import net.twasi.obsremotejava.callbacks.Callback;
5-
import net.twasi.obsremotejava.callbacks.ErrorCallback;
6-
import net.twasi.obsremotejava.callbacks.StringCallback;
7-
import net.twasi.obsremotejava.callbacks.VoidCallback;
4+
import net.twasi.obsremotejava.callbacks.*;
85
import net.twasi.obsremotejava.events.responses.*;
96
import net.twasi.obsremotejava.objects.throwables.OBSResponseError;
107
import net.twasi.obsremotejava.requests.GetCurrentProfile.GetCurrentProfileResponse;
@@ -121,7 +118,7 @@ public void connect() {
121118
}
122119

123120
public void disconnect() {
124-
// wait for closed socket connection
121+
// trigger the latch
125122
try {
126123
if (debug) {
127124
log.debug("Closing connection.");
@@ -131,6 +128,7 @@ public void disconnect() {
131128
runOnError("Error during closing websocket connection", e);
132129
}
133130

131+
// stop the client if it isn't already stopped or stopping
134132
if (!client.isStopped() && !client.isStopping()) {
135133
try {
136134
if (debug) {
@@ -169,6 +167,10 @@ public void registerConnectionFailedCallback(StringCallback onConnectionFailed)
169167
communicator.registerOnConnectionFailed(onConnectionFailed);
170168
}
171169

170+
public void registerCloseCallback(CloseCallback closeCallback) {
171+
communicator.registerOnClose(closeCallback);
172+
}
173+
172174
public void registerRecordingStartedCallback(VoidCallback onRecordingStarted) {
173175
communicator.registerOnRecordingStarted(onRecordingStarted);
174176
}
@@ -404,4 +406,5 @@ private void runOnConnectionFailed(String message, Throwable throwable) {
404406
log.error("Unable to run OnConnectionFailed callback", e);
405407
}
406408
}
409+
407410
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package net.twasi.obsremotejava.callbacks;
2+
3+
public interface CloseCallback {
4+
void run(int statusCode, String reason);
5+
}

src/main/java/net/twasi/obsremotejava/requests/GetVersion/GetVersionResponse.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,12 @@ public String getObsWebsocketVersion() {
1616
public String getObsStudioVersion() {
1717
return obsStudioVersion;
1818
}
19+
20+
@Override
21+
public String toString() {
22+
return "GetVersionResponse{" +
23+
"obsWebsocketVersion='" + obsWebsocketVersion + '\'' +
24+
", obsStudioVersion='" + obsStudioVersion + '\'' +
25+
'}';
26+
}
1927
}

src/test/java/net/twasi/obsremotejava/test/OBSRemoteControllerUnsecuredIT.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,41 @@ void testConnectAndDisconnect() {
110110
}
111111
}
112112

113+
@Test
114+
void testCloseCallback() {
115+
116+
// Given a controller that connects successfully
117+
final OBSRemoteController controller = new OBSRemoteController(obsAddress, true,
118+
obsPassword, true);
119+
120+
if (controller.isFailed()) {
121+
fail("Failed to connect to websocket");
122+
}
123+
124+
// When we register a Close callback
125+
AtomicReference<Boolean> testSuccessful = new AtomicReference<>(Boolean.FALSE);
126+
AtomicReference<String> testFailedReason = new AtomicReference<>();
127+
128+
controller.registerCloseCallback((int statusCode, String reason) -> testSuccessful.set(Boolean.TRUE));
129+
130+
// When we disconnect
131+
try {
132+
controller.disconnect();
133+
controller.await();
134+
} catch (InterruptedException e) {
135+
e.printStackTrace();
136+
}
137+
138+
// Then the close callback should have been called
139+
if (testFailedReason.get() != null) {
140+
fail(testFailedReason.get());
141+
}
142+
143+
if (!testSuccessful.get()) {
144+
fail("Disconnect didn't work");
145+
}
146+
}
147+
113148
@Test
114149
void disconnectShouldNotHaveErrorsWhenNoConnectDisconnectCallbacksRegistered() {
115150
AtomicReference<String> testFailedReason = new AtomicReference<>();

0 commit comments

Comments
 (0)