Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

socket-mode: fix bug when apps.connections.open returns an error and won't retry #1735

Merged
merged 1 commit into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions packages/socket-mode/src/SocketModeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,16 @@ export class SocketModeClient extends EventEmitter {
.transitionTo(ConnectingState.Reconnecting).withCondition(this.reconnectingCondition.bind(this))
.transitionTo(ConnectingState.Failed)
.state(ConnectingState.Reconnecting)
.do(async () => {
// Trying to reconnect after waiting for a bit...
.do(() => new Promise((res, _rej) => {
this.numOfConsecutiveReconnectionFailures += 1;
const millisBeforeRetry = this.clientPingTimeoutMillis * this.numOfConsecutiveReconnectionFailures;
this.logger.debug(`Before trying to reconnect, this client will wait for ${millisBeforeRetry} milliseconds`);
setTimeout(() => {
this.emit(ConnectingState.Authenticating);
this.logger.debug('Resolving reconnecting state to continue with reconnect...');
res(true);
}, millisBeforeRetry);
})
}))
.onSuccess().transitionTo(ConnectingState.Authenticating)
.onFailure().transitionTo(ConnectingState.Failed)
.state(ConnectingState.Authenticated)
.onEnter(this.configureAuthenticatedWebSocket.bind(this))
Expand Down
48 changes: 41 additions & 7 deletions packages/socket-mode/test/integration.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,7 @@ const sinon = require('sinon');
const HTTP_PORT = 12345;
const WSS_PORT = 23456;
// Basic HTTP server to 'fake' behaviour of `apps.connections.open` endpoint
const server = createServer((req, res) => {
res.writeHead(200, { 'content-type': 'application/json' });
res.end(JSON.stringify({
ok: true,
url: `ws://localhost:${WSS_PORT}/`,
}));
});
let server = null;

// Basic WS server to fake slack WS endpoint
let wss = null;
Expand All @@ -25,6 +19,13 @@ let client = null;

describe('Integration tests with a WebSocket server', () => {
beforeEach(() => {
server = createServer((req, res) => {
res.writeHead(200, { 'content-type': 'application/json' });
res.end(JSON.stringify({
ok: true,
url: `ws://localhost:${WSS_PORT}/`,
}));
});
server.listen(HTTP_PORT);
wss = new WebSocketServer({ port: WSS_PORT });
wss.on('connection', (ws) => {
Expand All @@ -38,7 +39,9 @@ describe('Integration tests with a WebSocket server', () => {
});
afterEach(() => {
server.close();
server = null;
wss.close();
wss = null;
exposed_ws_connection = null;
client = null;
});
Expand Down Expand Up @@ -69,6 +72,37 @@ describe('Integration tests with a WebSocket server', () => {
await client.disconnect();
});
});
describe('catastrophic server behaviour', () => {
beforeEach(() => {
client = new SocketModeClient({ appToken: 'whatever', logLevel: LogLevel.ERROR, clientOptions: {
slackApiUrl: `http://localhost:${HTTP_PORT}/`
}, clientPingTimeout: 25});
});
it('should retry if retrieving a WSS URL fails', async () => {
// Shut down the main WS-endpoint-retrieval server - we will customize its behaviour for this test
server.close();
let num_attempts = 0;
server = createServer((req, res) => {
num_attempts += 1;
res.writeHead(200, { 'content-type': 'application/json' });
if (num_attempts < 3) {
res.end(JSON.stringify({
ok: false,
error: "fatal_error",
}));
} else {
res.end(JSON.stringify({
ok: true,
url: `ws://localhost:${WSS_PORT}/`,
}));
}
});
server.listen(HTTP_PORT);
await client.start();
assert.equal(num_attempts, 3);
await client.disconnect();
});
});
describe('failure modes / unexpected messages sent to client', () => {
let debugLoggerSpy = sinon.stub();
const noop = () => {};
Expand Down
Loading