From 43886d2c2077d7997d0345b36d08c45689ae1925 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Thu, 30 Mar 2017 01:02:56 +0300 Subject: [PATCH 01/11] Enable early / direct callback Currently all responses are delayed using thee response URL for slash commands, this allows for direct responses to the original request. --- src/index.js | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/index.js b/src/index.js index bdf70b6..944fe4e 100644 --- a/src/index.js +++ b/src/index.js @@ -20,7 +20,8 @@ class Slack extends EventEmitter { * @param {Object} context - The Lambda context * @param {Function} callback - The Lambda callback */ - handler(event, context, callback) { + handler(event, context, callback) { + this.callback = this.callback.bind(callback); switch(event.method) { case "GET": this.oauth(event, context, callback); break; case "POST": this.event(event, context, callback); break; @@ -28,6 +29,18 @@ class Slack extends EventEmitter { } + /** + * Allow event handlers to use the callback early, auto destructs + * + * @param {Function} callback The Lambda callback function + * @param {Object} response A response object or string + */ + callback(callback, response) { + delete this.callback; + callback(null, JSON.stringify(response)); + } + + /** * OAuth Lambda Handler * @@ -88,8 +101,6 @@ class Slack extends EventEmitter { // Events API challenge if (payload.challenge) return callback(null, payload.challenge); - else - callback(); // Ignore Bot Messages if (!this.ignoreBots || !(payload.event || payload).bot_id) { @@ -125,8 +136,12 @@ class Slack extends EventEmitter { // trigger all events events.forEach(name => this.emit(name, payload, bot, this.store)); + + // Send success signal if callback not used in handler + if ( this.callback ) + this.callback(); } } -module.exports = new Slack(); \ No newline at end of file +module.exports = new Slack(); From ccd1744d7c4f5694e59b5d08b7491ad35dc4a772 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Thu, 30 Mar 2017 01:06:54 +0300 Subject: [PATCH 02/11] Use implicit callback() callback() is triggered automatically if no error is passed when the node event loop is empty. No need for any fancy handling. --- src/index.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/index.js b/src/index.js index 944fe4e..df782f8 100644 --- a/src/index.js +++ b/src/index.js @@ -36,7 +36,6 @@ class Slack extends EventEmitter { * @param {Object} response A response object or string */ callback(callback, response) { - delete this.callback; callback(null, JSON.stringify(response)); } @@ -136,10 +135,6 @@ class Slack extends EventEmitter { // trigger all events events.forEach(name => this.emit(name, payload, bot, this.store)); - - // Send success signal if callback not used in handler - if ( this.callback ) - this.callback(); } } From 15a64b03ce7e044213bc03c6b8e194f15b5bd09a Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Thu, 30 Mar 2017 01:14:07 +0300 Subject: [PATCH 03/11] Docs for slack.callback() --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index a6ca02a..6d0402e 100644 --- a/README.md +++ b/README.md @@ -106,3 +106,11 @@ slack.send('chat.postMessage', message).then(data => { // respond to webhooks slack.send('https://hooks.slack.com/services/T0000/B000/XXXX', message); ``` +You can also respond to events directly without an API call using the callback method. This is useful if you need the `in_channel` response type to show the users slash command too. +```js +// Example for a command that takes a name eg: /greet Bob +slack.on('/greet', msg => slack.callback({ + response_type: 'in_channel', + text: `Hey there ${msg.text}!` +})) +``` From 81f40d871ad862f8810fe89e8fc1c99067cce08e Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Thu, 30 Mar 2017 02:09:03 +0300 Subject: [PATCH 04/11] Prevent rebinding issue Sometimes with lambda you're hitting a still running instance of the function so the binding happens on each call causing the callback to be prepended to the args multiple times. --- src/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index df782f8..0905d91 100644 --- a/src/index.js +++ b/src/index.js @@ -30,12 +30,14 @@ class Slack extends EventEmitter { /** - * Allow event handlers to use the callback early, auto destructs + * Allow event handlers to use the callback early + * Auto destructs to prevent rebinding * * @param {Function} callback The Lambda callback function * @param {Object} response A response object or string */ callback(callback, response) { + delete this.callback; callback(null, JSON.stringify(response)); } From c1c724623f9d5db09f4c25f0eafec25792b1b6b8 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Tue, 4 Apr 2017 08:37:38 +0100 Subject: [PATCH 05/11] Use non-native actually good querystring lib --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 6c03a36..a6e8cab 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ }, "dependencies": { "aws-sdk": "^2.7.13", - "axios": "^0.15.3" + "axios": "^0.15.3", + "qs": "^6.4.0" } } From 5324b3115afcbb27f22336d8559336db49d26f0a Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Tue, 4 Apr 2017 08:38:15 +0100 Subject: [PATCH 06/11] Use qs lib --- src/client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client.js b/src/client.js index a3f6223..91132b4 100644 --- a/src/client.js +++ b/src/client.js @@ -1,7 +1,7 @@ 'use strict'; const axios = require('axios'), - qs = require('querystring'); + qs = require('qs'); class Client { @@ -209,4 +209,4 @@ class Client { } -module.exports = Client; \ No newline at end of file +module.exports = Client; From 654898a285a45d89ad3a2a69d960728e1af85d7c Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Wed, 5 Apr 2017 10:35:34 +0100 Subject: [PATCH 07/11] Fix serialisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The way you have to send URL encoded complex messages to slack means we need some custom serialisation. Values like the attachments array will be stripped by `qs.stringify()` so we need to `JSON.stringify()` those values and then URL encode them. 🤦‍♂️ --- src/client.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/client.js b/src/client.js index 91132b4..a821977 100644 --- a/src/client.js +++ b/src/client.js @@ -1,7 +1,7 @@ 'use strict'; const axios = require('axios'), - qs = require('qs'); + qs = require('querystring'); class Client { @@ -127,7 +127,11 @@ class Client { message = Object.assign({ token: this.token, channel: this.channel }, message); // convert json except when passing in a url - if (!endPoint.match(/^http/i)) message = qs.stringify(message); + if (!endPoint.match(/^http/i)) { + message = Object.keys(message) + .map(key => `${key}=${qs.escape(JSON.stringify(message[key]).replace(/^["'](.*)["']$/, '$1'))}`) + .join('&'); + } return this.api.post(endPoint, message).then(this.getData); } From 94e88abe506cc814abe8786ae6bd4da295d8d1b3 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Wed, 5 Apr 2017 10:36:26 +0100 Subject: [PATCH 08/11] No need for qs --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index a6e8cab..6c03a36 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,6 @@ }, "dependencies": { "aws-sdk": "^2.7.13", - "axios": "^0.15.3", - "qs": "^6.4.0" + "axios": "^0.15.3" } } From e22083037a1b9da8641860395728f19b01aca235 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Wed, 5 Apr 2017 11:42:36 +0100 Subject: [PATCH 09/11] Slightly simplified pre-serialisation step --- src/client.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client.js b/src/client.js index a821977..fc0ccc8 100644 --- a/src/client.js +++ b/src/client.js @@ -128,9 +128,9 @@ class Client { // convert json except when passing in a url if (!endPoint.match(/^http/i)) { - message = Object.keys(message) - .map(key => `${key}=${qs.escape(JSON.stringify(message[key]).replace(/^["'](.*)["']$/, '$1'))}`) - .join('&'); + if (message.attachments) + message.attachments = JSON.stringify(message.attachments).replace(/^'(.*)'$/, '$1') + message = qs.stringify(message); } return this.api.post(endPoint, message).then(this.getData); } From 19f19297316e19a3d0244d6f5e6fe7aca5fc05ef Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Wed, 5 Apr 2017 11:44:29 +0100 Subject: [PATCH 10/11] Code style --- src/client.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client.js b/src/client.js index fc0ccc8..0ca08b0 100644 --- a/src/client.js +++ b/src/client.js @@ -128,8 +128,9 @@ class Client { // convert json except when passing in a url if (!endPoint.match(/^http/i)) { - if (message.attachments) - message.attachments = JSON.stringify(message.attachments).replace(/^'(.*)'$/, '$1') + if (message.attachments) { + message.attachments = JSON.stringify(message.attachments).replace(/^'(.*)'$/, '$1'); + } message = qs.stringify(message); } return this.api.post(endPoint, message).then(this.getData); From 1ed5eefc89d11f7a902bac5725c565a40b725dde Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Tue, 11 Apr 2017 16:11:48 +0100 Subject: [PATCH 11/11] binding was a fools errand AWS keeps the same function alive and in memory but the handler is called each time. This updates the callback and exposes it during handler execution. --- src/index.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/index.js b/src/index.js index 0905d91..c501bfb 100644 --- a/src/index.js +++ b/src/index.js @@ -21,7 +21,7 @@ class Slack extends EventEmitter { * @param {Function} callback - The Lambda callback */ handler(event, context, callback) { - this.callback = this.callback.bind(callback); + this.callbackFn = callback; switch(event.method) { case "GET": this.oauth(event, context, callback); break; case "POST": this.event(event, context, callback); break; @@ -31,14 +31,11 @@ class Slack extends EventEmitter { /** * Allow event handlers to use the callback early - * Auto destructs to prevent rebinding * - * @param {Function} callback The Lambda callback function * @param {Object} response A response object or string */ - callback(callback, response) { - delete this.callback; - callback(null, JSON.stringify(response)); + callback(response) { + if (this.callbackFn) this.callbackFn(null, JSON.stringify(response)); }