Skip to content

Commit 0b7081a

Browse files
seratchseratch
authored
Fix #937 bolt-socket-mode establishes a connection before a SocketModeApp#start()/startAsync() method call (#939)
1 parent 92a23fa commit 0b7081a

File tree

2 files changed

+126
-50
lines changed

2 files changed

+126
-50
lines changed

‎bolt-socket-mode/src/main/java/com/slack/api/bolt/socket_mode/SocketModeApp.java‎

Lines changed: 105 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -14,57 +14,68 @@
1414
import java.io.IOException;
1515
import java.util.HashMap;
1616
import java.util.Map;
17+
import java.util.function.Supplier;
1718

1819
@Slf4j
1920
public class SocketModeApp {
20-
private boolean clientStopped = false;
21-
private final SocketModeClient client;
21+
private boolean clientStopped = true;
2222
private final App app;
23+
private final Supplier<SocketModeClient> clientFactory;
24+
private SocketModeClient client;
2325

24-
private static SocketModeClient buildSocketModeClient(
26+
// -------------------------------------------
27+
28+
private static Supplier<SocketModeClient> buildSocketModeClientFactory(
2529
App app,
2630
String appToken,
2731
SocketModeClient.Backend backend
28-
) throws IOException {
29-
final SocketModeClient client = app.slack().socketMode(appToken, backend);
30-
final SocketModeRequestParser requestParser = new SocketModeRequestParser(app.config());
31-
final Gson gson = GsonFactory.createSnakeCase(app.slack().getConfig());
32-
client.addWebSocketMessageListener(message -> {
33-
long startMillis = System.currentTimeMillis();
34-
SocketModeRequest req = requestParser.parse(message);
35-
if (req != null) {
36-
try {
37-
Response boltResponse = app.run(req.getBoltRequest());
38-
if (boltResponse.getStatusCode() != 200) {
39-
log.warn("Unsuccessful Bolt app execution (status: {}, body: {})",
40-
boltResponse.getStatusCode(), boltResponse.getBody());
41-
return;
42-
}
43-
if (boltResponse.getBody() != null) {
44-
Map<String, Object> response = new HashMap<>();
45-
if (boltResponse.getContentType().startsWith("application/json")) {
46-
response.put("envelope_id", req.getEnvelope().getEnvelopeId());
47-
response.put("payload", gson.fromJson(boltResponse.getBody(), JsonElement.class));
48-
} else {
49-
response.put("envelope_id", req.getEnvelope().getEnvelopeId());
50-
Map<String, Object> payload = new HashMap<>();
51-
payload.put("text", boltResponse.getBody());
52-
response.put("payload", payload);
32+
) {
33+
return () -> {
34+
try {
35+
final SocketModeClient client = app.slack().socketMode(appToken, backend);
36+
final SocketModeRequestParser requestParser = new SocketModeRequestParser(app.config());
37+
final Gson gson = GsonFactory.createSnakeCase(app.slack().getConfig());
38+
client.addWebSocketMessageListener(message -> {
39+
long startMillis = System.currentTimeMillis();
40+
SocketModeRequest req = requestParser.parse(message);
41+
if (req != null) {
42+
try {
43+
Response boltResponse = app.run(req.getBoltRequest());
44+
if (boltResponse.getStatusCode() != 200) {
45+
log.warn("Unsuccessful Bolt app execution (status: {}, body: {})",
46+
boltResponse.getStatusCode(), boltResponse.getBody());
47+
return;
48+
}
49+
if (boltResponse.getBody() != null) {
50+
Map<String, Object> response = new HashMap<>();
51+
if (boltResponse.getContentType().startsWith("application/json")) {
52+
response.put("envelope_id", req.getEnvelope().getEnvelopeId());
53+
response.put("payload", gson.fromJson(boltResponse.getBody(), JsonElement.class));
54+
} else {
55+
response.put("envelope_id", req.getEnvelope().getEnvelopeId());
56+
Map<String, Object> payload = new HashMap<>();
57+
payload.put("text", boltResponse.getBody());
58+
response.put("payload", payload);
59+
}
60+
client.sendSocketModeResponse(gson.toJson(response));
61+
} else {
62+
client.sendSocketModeResponse(new AckResponse(req.getEnvelope().getEnvelopeId()));
63+
}
64+
long spentMillis = System.currentTimeMillis() - startMillis;
65+
log.debug("Response time: {} milliseconds", spentMillis);
66+
return;
67+
} catch (Exception e) {
68+
log.error("Failed to handle a request: {}", e.getMessage(), e);
69+
return;
5370
}
54-
client.sendSocketModeResponse(gson.toJson(response));
55-
} else {
56-
client.sendSocketModeResponse(new AckResponse(req.getEnvelope().getEnvelopeId()));
5771
}
58-
long spentMillis = System.currentTimeMillis() - startMillis;
59-
log.debug("Response time: {} milliseconds", spentMillis);
60-
return;
61-
} catch (Exception e) {
62-
log.error("Failed to handle a request: {}", e.getMessage(), e);
63-
return;
64-
}
72+
});
73+
return client;
74+
} catch (IOException e) {
75+
log.error("Failed to start a new Socket Mode client (error: {})", e.getMessage(), e);
76+
return null;
6577
}
66-
});
67-
return client;
78+
};
6879
}
6980

7081
public SocketModeApp(App app) throws IOException {
@@ -80,14 +91,32 @@ public SocketModeApp(
8091
SocketModeClient.Backend backend,
8192
App app
8293
) throws IOException {
83-
this(buildSocketModeClient(app, appToken, backend), app);
94+
this(buildSocketModeClientFactory(app, appToken, backend), app);
95+
}
96+
97+
public SocketModeApp(Supplier<SocketModeClient> clientFactory, App app) {
98+
this.clientFactory = clientFactory;
99+
this.app = app;
84100
}
85101

102+
/**
103+
* If you would like to synchronously detect the connection error as an exception when bootstrapping,
104+
* use this constructor. The first line can throw an exception
105+
* in the case where either the token or network settings are valid.
106+
*
107+
* <code>
108+
* SocketModeClient client = Slack.getInstance().socketMode(appToken);
109+
* SocketModeApp socketModeApp = new SocketModeApp(client, app);
110+
* </code>
111+
*/
86112
public SocketModeApp(SocketModeClient socketModeClient, App app) {
87113
this.client = socketModeClient;
114+
this.clientFactory = () -> socketModeClient;
88115
this.app = app;
89116
}
90117

118+
// -------------------------------------------
119+
91120
public void start() throws Exception {
92121
run(true);
93122
}
@@ -97,21 +126,48 @@ public void startAsync() throws Exception {
97126
}
98127

99128
public void run(boolean blockCurrentThread) throws Exception {
100-
app.start();
101-
if (clientStopped) {
102-
client.connectToNewEndpoint();
129+
this.app.start();
130+
if (this.client == null) {
131+
this.client = clientFactory.get();
132+
}
133+
if (this.isClientStopped()) {
134+
this.client.connectToNewEndpoint();
103135
} else {
104-
client.connect();
136+
this.client.connect();
105137
}
106-
client.setAutoReconnectEnabled(true);
138+
this.client.setAutoReconnectEnabled(true);
139+
this.clientStopped = false;
107140
if (blockCurrentThread) {
108141
Thread.sleep(Long.MAX_VALUE);
109142
}
110143
}
111144

112145
public void stop() throws Exception {
113-
client.disconnect();
114-
clientStopped = true;
115-
app.stop();
146+
if (this.client != null && this.client.verifyConnection()) {
147+
this.client.disconnect();
148+
}
149+
this.clientStopped = true;
150+
this.app.stop();
151+
}
152+
153+
public void close() throws Exception {
154+
this.stop();
155+
this.client = null;
156+
}
157+
158+
// -------------------------------------------
159+
// Accessors
160+
// -------------------------------------------
161+
162+
public boolean isClientStopped() {
163+
return clientStopped;
164+
}
165+
166+
public SocketModeClient getClient() {
167+
return client;
168+
}
169+
170+
public App getApp() {
171+
return app;
116172
}
117173
}

‎bolt-socket-mode/src/test/java/test_locally/SocketModeAppTest.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
import java.util.concurrent.atomic.AtomicBoolean;
1818

19-
import static org.junit.Assert.assertTrue;
19+
import static org.junit.Assert.*;
2020

2121
public class SocketModeAppTest {
2222

@@ -78,6 +78,26 @@ public void payloadHandling() throws Exception {
7878
}
7979
}
8080

81+
@Test
82+
public void issue_937_it_should_not_be_connected_before_start_method_call() throws Exception {
83+
App app = new App(AppConfig.builder()
84+
.slack(slack)
85+
.singleTeamBotToken(VALID_BOT_TOKEN)
86+
.build());
87+
SocketModeApp socketModeApp = new SocketModeApp(VALID_APP_TOKEN, app);
88+
try {
89+
assertTrue(socketModeApp.isClientStopped());
90+
assertNull(socketModeApp.getClient());
91+
92+
// Check again to make sure if the auto-connection does not work in this scenario
93+
Thread.sleep(3_000L);
94+
assertTrue(socketModeApp.isClientStopped());
95+
assertNull(socketModeApp.getClient());
96+
} finally {
97+
socketModeApp.close();
98+
}
99+
}
100+
81101
// -------------------------------------------------
82102
// JavaWebSocket implementation
83103
// -------------------------------------------------

0 commit comments

Comments
 (0)