From 6fc036e4a2e4e9af278c0db70fd069c7a4bd3d8c Mon Sep 17 00:00:00 2001 From: Filip Maj Date: Fri, 19 Jan 2024 15:57:21 -0500 Subject: [PATCH 1/5] socket-mode: prep for major release: - bump minimum node version to 18 - remove unused p-* dependencies - code changes to move to new major version of ws and web-api dependencies --- packages/socket-mode/.nycrc.json | 14 +++++ packages/socket-mode/package.json | 55 +++++++++---------- .../socket-mode/src/SocketModeClient.spec.js | 30 +++++----- packages/socket-mode/src/SocketModeClient.ts | 26 +++++---- 4 files changed, 71 insertions(+), 54 deletions(-) create mode 100644 packages/socket-mode/.nycrc.json diff --git a/packages/socket-mode/.nycrc.json b/packages/socket-mode/.nycrc.json new file mode 100644 index 000000000..6c61b19dc --- /dev/null +++ b/packages/socket-mode/.nycrc.json @@ -0,0 +1,14 @@ +{ + "include": [ + "src/**/*.ts" + ], + "exclude": [ + "**/*.spec.js" + ], + "reporter": ["lcov"], + "extension": [ + ".ts" + ], + "all": false, + "cache": true +} diff --git a/packages/socket-mode/package.json b/packages/socket-mode/package.json index 983153ab1..52fe339e9 100644 --- a/packages/socket-mode/package.json +++ b/packages/socket-mode/package.json @@ -24,8 +24,8 @@ "dist/**/*" ], "engines": { - "node": ">=12.13.0", - "npm": ">=6.12.0" + "node": ">= 18", + "npm": ">= 8.6.0" }, "repository": "slackapi/node-slack-sdk", "homepage": "https://slack.dev/node-slack-sdk/socket-mode", @@ -40,40 +40,39 @@ "build": "npm run build:clean && tsc", "build:clean": "shx rm -rf ./dist ./coverage ./.nyc_output", "lint": "eslint --ext .ts src", - "test": "npm run lint && npm run build && nyc mocha --config .mocharc.json src/*.spec.js", + "mocha": "mocha --config .mocharc.json src/*.spec.js", + "test": "npm run lint && npm run build && nyc --reporter=text-summary npm run mocha", "watch": "npx nodemon --watch 'src' --ext 'ts' --exec npm run build" }, "dependencies": { - "@slack/logger": "^3.0.0", - "@slack/web-api": "^6.11.2", - "@types/node": ">=12.0.0", - "@types/p-queue": "^2.3.2", - "@types/ws": "^7.4.7", - "eventemitter3": "^3.1.0", + "@slack/logger": "^4", + "@slack/web-api": "^7.0.1", + "@types/node": ">=18", + "@types/ws": "^8", + "eventemitter3": "^5", "finity": "^0.5.4", - "p-cancelable": "^1.1.0", - "p-queue": "^2.4.2", - "ws": "^7.5.3" + "ws": "^8" }, "devDependencies": { - "@types/chai": "^4.3.5", - "@types/mocha": "^10.0.1", - "@typescript-eslint/eslint-plugin": "^6.4.1", - "@typescript-eslint/parser": "^6.4.0", - "chai": "^4.3.8", - "eslint": "^8.47.0", - "eslint-config-airbnb-base": "^15.0.0", - "eslint-config-airbnb-typescript": "^17.1.0", - "eslint-plugin-import": "^2.28.1", + "@types/chai": "^4", + "@types/mocha": "^10", + "@types/sinon": "^17", + "@typescript-eslint/eslint-plugin": "^6", + "@typescript-eslint/parser": "^6", + "chai": "^4", + "eslint": "^8", + "eslint-config-airbnb-base": "^15", + "eslint-config-airbnb-typescript": "^17", + "eslint-plugin-import": "^2", "eslint-plugin-import-newlines": "^1.3.4", - "eslint-plugin-jsdoc": "^46.5.0", - "eslint-plugin-node": "^11.1.0", - "mocha": "^10.2.0", - "nyc": "^15.1.0", + "eslint-plugin-jsdoc": "^48", + "eslint-plugin-node": "^11", + "mocha": "^10", + "nyc": "^15", "shx": "^0.3.2", - "sinon": "^15.2.0", + "sinon": "^17", "source-map-support": "^0.5.21", - "ts-node": "^10.8.1", - "typescript": "^4.1.0" + "ts-node": "^10", + "typescript": "5.3.3" } } diff --git a/packages/socket-mode/src/SocketModeClient.spec.js b/packages/socket-mode/src/SocketModeClient.spec.js index dfba56a3f..3f40a9ff7 100644 --- a/packages/socket-mode/src/SocketModeClient.spec.js +++ b/packages/socket-mode/src/SocketModeClient.spec.js @@ -19,7 +19,7 @@ describe('SocketModeClient', () => { }); describe('slash_commands messages', () => { - const message = { + const message = Buffer.from(JSON.stringify({ "envelope_id": "1d3c79ab-0ffb-41f3-a080-d19e85f53649", "payload": { "token": "verification-token", @@ -37,7 +37,7 @@ describe('SocketModeClient', () => { }, "type": "slash_commands", "accepts_response_payload": true - }; + })); it('should be sent to two listeners', async () => { const client = new SocketModeClient({ appToken: 'xapp-' }); @@ -52,7 +52,7 @@ describe('SocketModeClient', () => { && args.retry_num === undefined && args.retry_reason === undefined; }); - await client.onWebSocketMessage({ data: JSON.stringify(message) }); + await client.onWebSocketMessage(message); await sleep(30); assert.isTrue(commandListenerCalled); assert.isTrue(slackEventListenerCalled); @@ -64,7 +64,7 @@ describe('SocketModeClient', () => { client.on("slash_commands", async ({ envelope_id }) => { passedEnvelopeId = envelope_id; }); - await client.onWebSocketMessage({ data: JSON.stringify(message) }); + await client.onWebSocketMessage(message); await sleep(30); assert.equal(passedEnvelopeId, '1d3c79ab-0ffb-41f3-a080-d19e85f53649'); }); @@ -74,14 +74,14 @@ describe('SocketModeClient', () => { client.on("slack_event", async ({ envelope_id }) => { passedEnvelopeId = envelope_id; }); - await client.onWebSocketMessage({ data: JSON.stringify(message) }); + await client.onWebSocketMessage(message); await sleep(30); assert.equal(passedEnvelopeId, '1d3c79ab-0ffb-41f3-a080-d19e85f53649'); }); }); describe('events_api messages', () => { - const message = { + const message = Buffer.from(JSON.stringify({ "envelope_id": "cda4159a-72a5-4744-aba3-4d66eb52682b", "payload": { "token": "verification-token", @@ -133,7 +133,7 @@ describe('SocketModeClient', () => { "accepts_response_payload": false, "retry_attempt": 2, "retry_reason": "timeout" - }; + })); it('should be sent to two listeners', async () => { const client = new SocketModeClient({ appToken: 'xapp-' }); @@ -154,7 +154,7 @@ describe('SocketModeClient', () => { && args.retry_num === 2 && args.retry_reason === 'timeout'; }); - await client.onWebSocketMessage({ data: JSON.stringify(message) }); + await client.onWebSocketMessage(message); await sleep(30); assert.isFalse(otherListenerCalled); assert.isTrue(eventsApiListenerCalled); @@ -167,7 +167,7 @@ describe('SocketModeClient', () => { client.on("app_mention", async ({ envelope_id }) => { passedEnvelopeId = envelope_id; }); - await client.onWebSocketMessage({ data: JSON.stringify(message) }); + await client.onWebSocketMessage(message); await sleep(30); assert.equal(passedEnvelopeId, 'cda4159a-72a5-4744-aba3-4d66eb52682b'); }); @@ -177,14 +177,14 @@ describe('SocketModeClient', () => { client.on("slack_event", async ({ envelope_id }) => { passedEnvelopeId = envelope_id; }); - await client.onWebSocketMessage({ data: JSON.stringify(message) }); + await client.onWebSocketMessage(message); await sleep(30); assert.equal(passedEnvelopeId, 'cda4159a-72a5-4744-aba3-4d66eb52682b'); }); }); describe('interactivity messages', () => { - const message = { + const message = Buffer.from(JSON.stringify({ "envelope_id": "57d6a792-4d35-4d0b-b6aa-3361493e1caf", "payload": { "type": "shortcut", @@ -206,7 +206,7 @@ describe('SocketModeClient', () => { }, "type": "interactive", "accepts_response_payload": false - }; + })); it('should be sent to two listeners', async () => { const client = new SocketModeClient({ appToken: 'xapp-' }); @@ -222,7 +222,7 @@ describe('SocketModeClient', () => { client.on("slack_event", async (args) => { slackEventListenerCalled = args.ack !== undefined && args.body !== undefined; }); - await client.onWebSocketMessage({ data: JSON.stringify(message) }); + await client.onWebSocketMessage(message); await sleep(30); assert.isFalse(otherListenerCalled); assert.isTrue(interactiveListenerCalled); @@ -235,7 +235,7 @@ describe('SocketModeClient', () => { client.on("interactive", async ({ envelope_id }) => { passedEnvelopeId = envelope_id; }); - await client.onWebSocketMessage({ data: JSON.stringify(message) }); + await client.onWebSocketMessage(message); await sleep(30); assert.equal(passedEnvelopeId, '57d6a792-4d35-4d0b-b6aa-3361493e1caf'); }); @@ -245,7 +245,7 @@ describe('SocketModeClient', () => { client.on("slack_event", async ({ envelope_id }) => { passedEnvelopeId = envelope_id; }); - await client.onWebSocketMessage({ data: JSON.stringify(message) }); + await client.onWebSocketMessage(message); await sleep(30); assert.equal(passedEnvelopeId, '57d6a792-4d35-4d0b-b6aa-3361493e1caf'); }); diff --git a/packages/socket-mode/src/SocketModeClient.ts b/packages/socket-mode/src/SocketModeClient.ts index b3af15a11..51022d6b8 100644 --- a/packages/socket-mode/src/SocketModeClient.ts +++ b/packages/socket-mode/src/SocketModeClient.ts @@ -433,7 +433,7 @@ export class SocketModeClient extends EventEmitter { private async retrieveWSSURL(): Promise { try { this.logger.debug('Going to retrieve a new WSS URL ...'); - return await this.webClient.apps.connections.open(); + return await this.webClient.apps.connections.open({}); } catch (error) { this.logger.error(`Failed to retrieve a new WSS URL for reconnection (error: ${error})`); throw error; @@ -525,17 +525,17 @@ export class SocketModeClient extends EventEmitter { websocket.addEventListener('open', (event) => { this.stateMachine.handle(Event.WebSocketOpen, event); }); - websocket.addEventListener('close', (event) => { - this.stateMachine.handle(Event.WebSocketClose, event); - }); websocket.addEventListener('error', (event) => { this.logger.error(`A WebSocket error occurred: ${event.message}`); this.emit('error', websocketErrorWithOriginal(event.error)); }); - websocket.addEventListener('message', this.onWebSocketMessage.bind(this)); + websocket.on('message', this.onWebSocketMessage.bind(this)); + websocket.on('close', (code: number, _data: Buffer) => { + this.stateMachine.handle(Event.WebSocketClose, code); + }); // Confirm WebSocket connection is still active - websocket.addEventListener('ping', ((data: Buffer) => { + websocket.on('ping', ((data: Buffer) => { if (this.pingPongLoggingEnabled) { this.logger.debug(`Received ping from Slack server (data: ${data})`); } @@ -544,7 +544,7 @@ export class SocketModeClient extends EventEmitter { // we cast this function to any as a workaround }) as any); // eslint-disable-line @typescript-eslint/no-explicit-any - websocket.addEventListener('pong', ((data: Buffer) => { + websocket.on('pong', ((data: Buffer) => { if (this.pingPongLoggingEnabled) { this.logger.debug(`Received pong from Slack server (data: ${data})`); } @@ -694,8 +694,12 @@ export class SocketModeClient extends EventEmitter { * `onmessage` handler for the client's WebSocket. * This will parse the payload and dispatch the relevant events for each incoming message. */ - protected async onWebSocketMessage({ data }: { data: string }): Promise { - this.logger.debug(`Received a message on the WebSocket: ${data}`); + protected async onWebSocketMessage(data: WebSocket.RawData, isBinary: boolean): Promise { + if (isBinary) { + this.logger.error('Unexpected binary message received!'); + return; + } + this.logger.debug(`Received a message on the WebSocket: ${data.toString()}`); // Parse message into slack event let event: { @@ -710,12 +714,12 @@ export class SocketModeClient extends EventEmitter { }; try { - event = JSON.parse(data); + event = JSON.parse(data.toString()); // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (parseError: any) { // Prevent application from crashing on a bad message, but log an error to bring attention this.logger.error( - `Unable to parse an incoming WebSocket message: ${parseError.message}`, + `Unable to parse an incoming WebSocket message: ${parseError.message}, ${data.toString()}`, ); return; } From 45a625ed7b20b96ada206cc11cfda201757efd66 Mon Sep 17 00:00:00 2001 From: Filip Maj Date: Fri, 19 Jan 2024 16:21:40 -0500 Subject: [PATCH 2/5] bump min node ref in readme to 18 --- packages/socket-mode/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/socket-mode/README.md b/packages/socket-mode/README.md index a67114c46..8500e94c1 100644 --- a/packages/socket-mode/README.md +++ b/packages/socket-mode/README.md @@ -222,7 +222,7 @@ const socketModeClient = new SocketModeClient({ ## Requirements -This package supports Node v14 and higher. It's highly recommended to use [the latest LTS version of +This package supports Node v18 and higher. It's highly recommended to use [the latest LTS version of node](https://github.com/nodejs/Release#release-schedule), and the documentation is written using syntax and features from that version. ## Getting Help From 5121945b3a6cbfd1339362bb1ecc36507a4c6979 Mon Sep 17 00:00:00 2001 From: Filip Maj Date: Tue, 23 Jan 2024 17:14:45 -0500 Subject: [PATCH 3/5] Add a couple of basic integration tests. --- packages/socket-mode/package.json | 3 +- packages/socket-mode/test/integration.spec.js | 70 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 packages/socket-mode/test/integration.spec.js diff --git a/packages/socket-mode/package.json b/packages/socket-mode/package.json index 52fe339e9..6b99878df 100644 --- a/packages/socket-mode/package.json +++ b/packages/socket-mode/package.json @@ -41,7 +41,8 @@ "build:clean": "shx rm -rf ./dist ./coverage ./.nyc_output", "lint": "eslint --ext .ts src", "mocha": "mocha --config .mocharc.json src/*.spec.js", - "test": "npm run lint && npm run build && nyc --reporter=text-summary npm run mocha", + "test:integration": "mocha --config .mocharc.json --timeout 10000 test/integration.spec.js", + "test": "npm run lint && npm run build && nyc --reporter=text-summary npm run mocha && npm run test:integration", "watch": "npx nodemon --watch 'src' --ext 'ts' --exec npm run build" }, "dependencies": { diff --git a/packages/socket-mode/test/integration.spec.js b/packages/socket-mode/test/integration.spec.js new file mode 100644 index 000000000..439966c52 --- /dev/null +++ b/packages/socket-mode/test/integration.spec.js @@ -0,0 +1,70 @@ +const { assert } = require('chai'); +const { SocketModeClient, LogLevel } = require('../.'); +const { WebSocketServer} = require('ws'); +const { createServer } = require('http'); + +const HTTP_PORT = 8080; +const WSS_PORT = 8081; +// 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}/`, + })); +}); + +// Basic WS server to fake slack WS endpoint +let wss = null; +let exposed_ws_connection = null; + +// Socket mode client pointing to the above two posers +const client = new SocketModeClient({ appToken: 'whatever', logLevel: LogLevel.ERROR, clientOptions: { + slackApiUrl: 'http://localhost:8080/' +}}); + +describe('Integration tests with a WebSocket server', () => { + beforeEach(() => { + server.listen(HTTP_PORT); + wss = new WebSocketServer({ port: WSS_PORT }); + wss.on('connection', (ws) => { + ws.on('error', (err) => { + assert.fail(err); + }); + // Send `Event.ServerHello` + ws.send(JSON.stringify({type: 'hello'})); + exposed_ws_connection = ws; + }); + }); + afterEach(() => { + server.close(); + wss.close(); + }); + it('connects to a server', async () => { + await client.start(); + // TODO: this is necessary, as await start() followed by await disconnect() causes an exception + // probably a bug! + await new Promise((res, _rej) => { + client.on('connected', () => { + res(); + }); + }); + await client.disconnect(); + }); + it('can listen on random event types and receive payload properties', async () => { + await client.start(); + client.on('connected', () => { + exposed_ws_connection.send(JSON.stringify({ + type: 'integration-test', + envelope_id: 12345, + })); + }); + await new Promise((res, _rej) => { + client.on('integration-test', (evt) => { + assert.equal(evt.envelope_id, 12345); + res(); + }); + }); + await client.disconnect(); + }); +}); From 3f499592051ee979d8fdf522a1aca462ca4f8c1d Mon Sep 17 00:00:00 2001 From: Filip Maj Date: Tue, 23 Jan 2024 17:37:32 -0500 Subject: [PATCH 4/5] Refactor setting up client and server in integration test, add a skipped-for-now suite of tests for failure modes --- packages/socket-mode/test/integration.spec.js | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/socket-mode/test/integration.spec.js b/packages/socket-mode/test/integration.spec.js index 439966c52..63a5e5e95 100644 --- a/packages/socket-mode/test/integration.spec.js +++ b/packages/socket-mode/test/integration.spec.js @@ -19,12 +19,13 @@ let wss = null; let exposed_ws_connection = null; // Socket mode client pointing to the above two posers -const client = new SocketModeClient({ appToken: 'whatever', logLevel: LogLevel.ERROR, clientOptions: { - slackApiUrl: 'http://localhost:8080/' -}}); +let client = null; describe('Integration tests with a WebSocket server', () => { beforeEach(() => { + client = new SocketModeClient({ appToken: 'whatever', logLevel: LogLevel.ERROR, clientOptions: { + slackApiUrl: 'http://localhost:8080/' + }}); server.listen(HTTP_PORT); wss = new WebSocketServer({ port: WSS_PORT }); wss.on('connection', (ws) => { @@ -39,6 +40,8 @@ describe('Integration tests with a WebSocket server', () => { afterEach(() => { server.close(); wss.close(); + exposed_ws_connection = null; + client = null; }); it('connects to a server', async () => { await client.start(); @@ -67,4 +70,32 @@ describe('Integration tests with a WebSocket server', () => { }); await client.disconnect(); }); + describe.skip('failure modes / unexpected messages sent to client', () => { + it('should deal with empty/binary messages', async () => { + await client.start(); + client.on('connected', () => { + exposed_ws_connection.send(null); + }); + // TODO: will be stuck here and the error "Unexpected binary message received!' will be raised as an ERROR log + await new Promise((res, _rej) => { + client.on('slack_event', (evt) => { + res(); + }); + }); + await client.disconnect(); + }); + it('should deal with empty string messages', async () => { + await client.start(); + client.on('connected', () => { + exposed_ws_connection.send(''); + }); + // TODO: will be stuck here and the error "Unable to parse an incoming WebSocket message: Unexpected end of JSON input,' will be raised as an ERROR log + await new Promise((res, _rej) => { + client.on('slack_event', (evt) => { + res(); + }); + }); + await client.disconnect(); + }); + }); }); From 66291dbb31b6194f77a8a27e10566342651dafd9 Mon Sep 17 00:00:00 2001 From: Filip Maj Date: Wed, 24 Jan 2024 11:56:22 -0500 Subject: [PATCH 5/5] Update README, removing unsupported events and expanding on Logger interface requirements. Tweak `start` behaviour to return once the client is connected rather than when authenticated (and still connecting). Binary and malformed JSON messages are raised now as DEBUG level logs. Added integration tests assuring the documented lifecycle events are raised. --- packages/socket-mode/README.md | 16 +- packages/socket-mode/package.json | 2 +- .../socket-mode/src/SocketModeClient.spec.js | 8 +- packages/socket-mode/src/SocketModeClient.ts | 14 +- packages/socket-mode/test/integration.spec.js | 148 ++++++++++++------ 5 files changed, 126 insertions(+), 62 deletions(-) diff --git a/packages/socket-mode/README.md b/packages/socket-mode/README.md index 8500e94c1..2ec524a4c 100644 --- a/packages/socket-mode/README.md +++ b/packages/socket-mode/README.md @@ -11,7 +11,7 @@ $ npm install @slack/socket-mode ## Usage These examples show the most common features of `Socket Mode`. You'll find even more extensive [documentation on the -package's website](https://slack.dev/node-slack-sdk/socket-mode) and our [api site](https://api.slack.com/socket-mode). +package's website](https://slack.dev/node-slack-sdk/socket-mode) and our [api site][socket-mode]. @@ -19,9 +19,9 @@ package's website](https://slack.dev/node-slack-sdk/socket-mode) and our [api si ### Initialize the client -This package is designed to support [**Socket Mode**](https://api.slack.com/socket-mode), which allows your app to receive events from Slack over a WebSocket connection. +This package is designed to support [**Socket Mode**][socket-mode], which allows your app to receive events from Slack over a WebSocket connection. -The package exports a `SocketModeClient` class. Your app will create an instance of the class for each workspace it communicates with. Creating an instance requires an **app-level token** from Slack. Apps connect to the **Socket Mode** API using an **app-level token**, which starts with `xapp`. +The package exports a `SocketModeClient` class. Your app will create an instance of the class for each workspace it communicates with. Creating an instance requires an [**app-level token**][app-token] from Slack. Apps connect to the **Socket Mode** API using an [**app-level token**][app-token], which starts with `xapp`. Note: **Socket Mode** requires the `connections:write` scope. Navigate to your [app configuration](https://api.slack.com/apps) and go to the **OAuth and Permissions** section to add the scope. @@ -83,7 +83,7 @@ socketModeClient.on('message', (event) => { ### Send a message -To respond to events and send messages back into Slack, we recommend using the `@slack/web-api` package with a `bot token`. +To respond to events and send messages back into Slack, we recommend using the `@slack/web-api` package with a [bot token](https://api.slack.com/authentication/token-types#bot). ```javascript const { SocketModeClient } = require('@slack/socket-mode'); @@ -140,7 +140,6 @@ In the table below, the client's states are listed, which are also the names of | `connecting` | | The client is in the process of connecting to the platform. | | `authenticated` | `(connectData)` - the response from `apps.connections.open` | The client has authenticated with the platform. This is a sub-state of `connecting`. | | `connected` | | The client is connected to the platform and incoming events will start being emitted. | -| `ready` | | The client is ready to send outgoing messages. This is a sub-state of `connected` | | `disconnecting` | | The client is no longer connected to the platform and cleaning up its resources. It will soon transition to `disconnected`. | | `reconnecting` | | The client is no longer connected to the platform and cleaning up its resources. It will soon transition to `connecting`. | | `disconnected` | `(error)` | The client is not connected to the platform. This is a steady state - no attempt to connect is occurring. The `error` argument will be `undefined` when the client initiated the disconnect (normal). | @@ -182,10 +181,11 @@ All the log levels, in order of most to least information are: `DEBUG`, `INFO`, Sending log output somewhere besides the console -You can also choose to have logs sent to a custom logger using the `logger` option. A custom logger needs to implement specific methods (known as the `Logger` interface): +You can also choose to have logs sent to a custom logger using the `logger` option. A custom logger needs to implement specific methods (known as the `Logger` interface, see the [`@slack/logger` package](https://www.npmjs.com/package/@slack/logger) for details). A minimal interface should implement the following methods: | Method | Parameters | Return type | |--------------|-------------------|-------------| +| `getLevel()` | n/a | `LogLevel` | | `setLevel()` | `level: LogLevel` | `void` | | `setName()` | `name: string` | `void` | | `debug()` | `...msgs: any[]` | `void` | @@ -207,6 +207,7 @@ const socketModeClient = new SocketModeClient({ info(...msgs): { logWritable.write('info: ' + JSON.stringify(msgs)); }, warn(...msgs): { logWritable.write('warn: ' + JSON.stringify(msgs)); }, error(...msgs): { logWritable.write('error: ' + JSON.stringify(msgs)); }, + getLevel(): { return 'info'; }, setLevel(): { }, setName(): { }, }, @@ -230,3 +231,6 @@ node](https://github.com/nodejs/Release#release-schedule), and the documentation If you get stuck, we're here to help. The following are the best ways to get assistance working through your issue: * [Issue Tracker](http://github.com/slackapi/node-slack-sdk/issues) for questions, feature requests, bug reports and general discussion related to these packages. Try searching before you create a new issue. + +[socket-mode]: https://api.slack.com/apis/connections/socket +[app-token]: https://api.slack.com/authentication/token-types#app diff --git a/packages/socket-mode/package.json b/packages/socket-mode/package.json index 6b99878df..d5177f667 100644 --- a/packages/socket-mode/package.json +++ b/packages/socket-mode/package.json @@ -41,7 +41,7 @@ "build:clean": "shx rm -rf ./dist ./coverage ./.nyc_output", "lint": "eslint --ext .ts src", "mocha": "mocha --config .mocharc.json src/*.spec.js", - "test:integration": "mocha --config .mocharc.json --timeout 10000 test/integration.spec.js", + "test:integration": "mocha --config .mocharc.json test/integration.spec.js", "test": "npm run lint && npm run build && nyc --reporter=text-summary npm run mocha && npm run test:integration", "watch": "npx nodemon --watch 'src' --ext 'ts' --exec npm run build" }, diff --git a/packages/socket-mode/src/SocketModeClient.spec.js b/packages/socket-mode/src/SocketModeClient.spec.js index 3f40a9ff7..325eced9b 100644 --- a/packages/socket-mode/src/SocketModeClient.spec.js +++ b/packages/socket-mode/src/SocketModeClient.spec.js @@ -39,7 +39,7 @@ describe('SocketModeClient', () => { "accepts_response_payload": true })); - it('should be sent to two listeners', async () => { + it('should be sent to both slash_commands and slack_event listeners', async () => { const client = new SocketModeClient({ appToken: 'xapp-' }); let commandListenerCalled = false; client.on("slash_commands", async (args) => { @@ -135,10 +135,10 @@ describe('SocketModeClient', () => { "retry_reason": "timeout" })); - it('should be sent to two listeners', async () => { + it('should be sent to the specific and generic event listeners, and should not trip an unrelated event listener', async () => { const client = new SocketModeClient({ appToken: 'xapp-' }); let otherListenerCalled = false; - client.on("app_home_opend", async () => { + client.on("app_home_opened", async () => { otherListenerCalled = true; }); let eventsApiListenerCalled = false; @@ -208,7 +208,7 @@ describe('SocketModeClient', () => { "accepts_response_payload": false })); - it('should be sent to two listeners', async () => { + it('should be sent to the specific and generic event type listeners, and should not trip an unrelated event listener', async () => { const client = new SocketModeClient({ appToken: 'xapp-' }); let otherListenerCalled = false; client.on("slash_commands", async () => { diff --git a/packages/socket-mode/src/SocketModeClient.ts b/packages/socket-mode/src/SocketModeClient.ts index 51022d6b8..67b899c36 100644 --- a/packages/socket-mode/src/SocketModeClient.ts +++ b/packages/socket-mode/src/SocketModeClient.ts @@ -134,8 +134,8 @@ export class SocketModeClient extends EventEmitter { /** * Start a Socket Mode session app. - * It may take a few milliseconds before being connected. - * This method must be called before any messages can be sent or received. + * This method must be called before any messages can be sent or received, + * or to disconnect the client via the `disconnect` method. */ public start(): Promise { this.logger.debug('Starting a Socket Mode client ...'); @@ -143,12 +143,12 @@ export class SocketModeClient extends EventEmitter { this.stateMachine.handle(Event.Start); // Return a promise that resolves with the connection information return new Promise((resolve, reject) => { - this.once(ConnectingState.Authenticated, (result) => { + this.once(State.Connected, (result) => { this.removeListener(State.Disconnected, reject); resolve(result); }); this.once(State.Disconnected, (err) => { - this.removeListener(ConnectingState.Authenticated, resolve); + this.removeListener(State.Connected, resolve); reject(err); }); }); @@ -696,7 +696,7 @@ export class SocketModeClient extends EventEmitter { */ protected async onWebSocketMessage(data: WebSocket.RawData, isBinary: boolean): Promise { if (isBinary) { - this.logger.error('Unexpected binary message received!'); + this.logger.debug('Unexpected binary message received, ignoring.'); return; } this.logger.debug(`Received a message on the WebSocket: ${data.toString()}`); @@ -718,8 +718,8 @@ export class SocketModeClient extends EventEmitter { // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (parseError: any) { // Prevent application from crashing on a bad message, but log an error to bring attention - this.logger.error( - `Unable to parse an incoming WebSocket message: ${parseError.message}, ${data.toString()}`, + this.logger.debug( + `Unable to parse an incoming WebSocket message (will ignore): ${parseError.message}, ${data.toString()}`, ); return; } diff --git a/packages/socket-mode/test/integration.spec.js b/packages/socket-mode/test/integration.spec.js index 63a5e5e95..c61504c6b 100644 --- a/packages/socket-mode/test/integration.spec.js +++ b/packages/socket-mode/test/integration.spec.js @@ -1,10 +1,12 @@ const { assert } = require('chai'); -const { SocketModeClient, LogLevel } = require('../.'); +const { SocketModeClient} = require('../src/SocketModeClient'); +const { LogLevel } = require('../src/logger'); const { WebSocketServer} = require('ws'); const { createServer } = require('http'); +const sinon = require('sinon'); -const HTTP_PORT = 8080; -const WSS_PORT = 8081; +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' }); @@ -23,9 +25,6 @@ let client = null; describe('Integration tests with a WebSocket server', () => { beforeEach(() => { - client = new SocketModeClient({ appToken: 'whatever', logLevel: LogLevel.ERROR, clientOptions: { - slackApiUrl: 'http://localhost:8080/' - }}); server.listen(HTTP_PORT); wss = new WebSocketServer({ port: WSS_PORT }); wss.on('connection', (ws) => { @@ -43,59 +42,120 @@ describe('Integration tests with a WebSocket server', () => { exposed_ws_connection = null; client = null; }); - it('connects to a server', async () => { - await client.start(); - // TODO: this is necessary, as await start() followed by await disconnect() causes an exception - // probably a bug! - await new Promise((res, _rej) => { - client.on('connected', () => { - res(); - }); - }); - await client.disconnect(); - }); - it('can listen on random event types and receive payload properties', async () => { - await client.start(); - client.on('connected', () => { - exposed_ws_connection.send(JSON.stringify({ - type: 'integration-test', - envelope_id: 12345, - })); - }); - await new Promise((res, _rej) => { - client.on('integration-test', (evt) => { - assert.equal(evt.envelope_id, 12345); - res(); - }); + describe('establishing connection, receiving valid messages', () => { + beforeEach(() => { + client = new SocketModeClient({ appToken: 'whatever', logLevel: LogLevel.ERROR, clientOptions: { + slackApiUrl: `http://localhost:${HTTP_PORT}/` + }}); }); - await client.disconnect(); - }); - describe.skip('failure modes / unexpected messages sent to client', () => { - it('should deal with empty/binary messages', async () => { + it('connects to a server via `start()` and gracefully disconnects via `disconnect()`', async () => { await client.start(); + await client.disconnect(); + }); + it('can listen on random event types and receive payload properties', async () => { client.on('connected', () => { - exposed_ws_connection.send(null); + exposed_ws_connection.send(JSON.stringify({ + type: 'integration-test', + envelope_id: 12345, + })); }); - // TODO: will be stuck here and the error "Unexpected binary message received!' will be raised as an ERROR log + await client.start(); await new Promise((res, _rej) => { - client.on('slack_event', (evt) => { + client.on('integration-test', (evt) => { + assert.equal(evt.envelope_id, 12345); res(); }); }); await client.disconnect(); }); - it('should deal with empty string messages', async () => { + }); + describe('failure modes / unexpected messages sent to client', () => { + let debugLoggerSpy = sinon.stub(); + const noop = () => {}; + beforeEach(() => { + client = new SocketModeClient({ appToken: 'whatever', clientOptions: { + slackApiUrl: `http://localhost:${HTTP_PORT}/` + }, logger: { + debug: debugLoggerSpy, + info: noop, + error: noop, + getLevel: () => 'debug', + }}); + }); + afterEach(() => { + debugLoggerSpy.resetHistory(); + }); + it('should ignore binary messages', async () => { + client.on('connected', () => { + exposed_ws_connection.send(null); + }); await client.start(); + await sleep(10); + assert.isTrue(debugLoggerSpy.calledWith(sinon.match('Unexpected binary message received'))); + await client.disconnect(); + }); + it('should debug-log that a malformed JSON message was received', async () => { client.on('connected', () => { exposed_ws_connection.send(''); }); - // TODO: will be stuck here and the error "Unable to parse an incoming WebSocket message: Unexpected end of JSON input,' will be raised as an ERROR log - await new Promise((res, _rej) => { - client.on('slack_event', (evt) => { - res(); - }); - }); + await client.start(); + await sleep(10); + assert.isTrue(debugLoggerSpy.calledWith(sinon.match('Unable to parse an incoming WebSocket message'))); + await client.disconnect(); + }); + }); + describe('lifecycle events', () => { + beforeEach(() => { + client = new SocketModeClient({ appToken: 'whatever', logLevel: LogLevel.ERROR, clientOptions: { + slackApiUrl: `http://localhost:${HTTP_PORT}/` + }}); + }); + it('raises connecting event during `start()`', async () => { + let raised = false; + client.on('connecting', () => { raised = true; }); + await client.start(); + assert.isTrue(raised); + await client.disconnect(); + }); + it('raises authenticated event during `start()`', async () => { + let raised = false; + client.on('authenticated', () => { raised = true; }); + await client.start(); + assert.isTrue(raised); + await client.disconnect(); + }); + it('raises connected event during `start()`', async () => { + let raised = false; + client.on('connected', () => { raised = true; }); + await client.start(); + assert.isTrue(raised); + await client.disconnect(); + }); + it('raises disconnecting event during `disconnect()`', async () => { + let raised = false; + client.on('disconnecting', () => { raised = true; }); + await client.start(); await client.disconnect(); + assert.isTrue(raised); + }); + it.skip('raises reconnecting event after `disconnect()`', async () => { + // TODO: doesnt seem to work, maybe need to set up client with reconnecting configuration + let raised = false; + client.on('reconnecting', () => { raised = true; }); + await client.start(); + await client.disconnect(); + assert.isTrue(raised); + }); + it('raises disconnected event after `disconnect()`', async () => { + let raised = false; + client.on('disconnected', () => { raised = true; }); + await client.start(); + await client.disconnect(); + assert.isTrue(raised); }); }); }); + +function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +}