From ec3c11d8cfee09fb4d861ff5f6ebf0bddcf8097c Mon Sep 17 00:00:00 2001 From: Alec Lazarescu Date: Sat, 17 Jun 2017 21:06:33 -0400 Subject: [PATCH 1/6] Add missing error callback from team not found --- lib/SlackBot.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/SlackBot.js b/lib/SlackBot.js index fff75229e..f2fa292f8 100755 --- a/lib/SlackBot.js +++ b/lib/SlackBot.js @@ -168,6 +168,8 @@ function Slackbot(configuration) { cb(null, found_team); } }); + } else { + cb(new Error(`could not find team ${team_id}`)); } } }); From 050937136cefaab6dc573f0d2ecbeb031641f029 Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Mon, 19 Jun 2017 13:44:49 -0400 Subject: [PATCH 2/6] adds use strict directive to test files that use ES6 --- __test__/lib/Slack_web_api.test.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/__test__/lib/Slack_web_api.test.js b/__test__/lib/Slack_web_api.test.js index 523832250..c6b748928 100644 --- a/__test__/lib/Slack_web_api.test.js +++ b/__test__/lib/Slack_web_api.test.js @@ -1,3 +1,5 @@ +'use strict'; + let slackWebApi; let mockRequest; let mockResponse; @@ -206,7 +208,7 @@ describe('postForm', () => { method('some.action', 'data', cb); expect(mockRequest.post).toHaveBeenCalledTimes(1); - expect(cb).toHaveBeenCalledWith('not ok', { ok: false, error: 'not ok'}); + expect(cb).toHaveBeenCalledWith('not ok', { ok: false, error: 'not ok' }); }); }); }); @@ -219,7 +221,7 @@ describe('api methods', () => { instance = slackWebApi(mockBot, {}); cb = jest.fn(); jest.spyOn(instance, 'callAPI'); - instance.callAPI.mockImplementation(() => {}); + instance.callAPI.mockImplementation(() => { }); }); afterEach(() => { @@ -253,41 +255,41 @@ describe('api methods', () => { describe('special cases', () => { test('chat.postMessage stringifies attachments', () => { - instance.chat.postMessage({attachments: []}, cb); - expect(instance.callAPI).toHaveBeenCalledWith('chat.postMessage', {attachments: '[]'}, cb); + instance.chat.postMessage({ attachments: [] }, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.postMessage', { attachments: '[]' }, cb); }); test('chat.postMessage handles attachments as Strings', () => { jest.spyOn(JSON, 'stringify'); - instance.chat.postMessage({attachments: 'string'}, cb); - expect(instance.callAPI).toHaveBeenCalledWith('chat.postMessage', {attachments: 'string'}, cb); + instance.chat.postMessage({ attachments: 'string' }, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.postMessage', { attachments: 'string' }, cb); expect(JSON.stringify).not.toHaveBeenCalled(); }); test('chat.postMessage handles attachments stringification errors', () => { const error = new Error('WHOOPSIE'); jest.spyOn(JSON, 'stringify').mockImplementation(() => { throw error; }); - instance.chat.postMessage({attachments: []}, cb); + instance.chat.postMessage({ attachments: [] }, cb); expect(instance.callAPI).toHaveBeenCalledWith('chat.postMessage', {}, cb); expect(JSON.stringify).toHaveBeenCalled(); }); test('chat.update stringifies attachments', () => { - instance.chat.update({attachments: []}, cb); - expect(instance.callAPI).toHaveBeenCalledWith('chat.update', {attachments: '[]'}, cb); + instance.chat.update({ attachments: [] }, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.update', { attachments: '[]' }, cb); }); test('chat.update handles attachments as Strings', () => { jest.spyOn(JSON, 'stringify'); - instance.chat.update({attachments: 'string'}, cb); - expect(instance.callAPI).toHaveBeenCalledWith('chat.update', {attachments: 'string'}, cb); + instance.chat.update({ attachments: 'string' }, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.update', { attachments: 'string' }, cb); expect(JSON.stringify).not.toHaveBeenCalled(); }); test('chat.postMessage handles attachments stringification errors', () => { const error = new Error('WHOOPSIE'); jest.spyOn(JSON, 'stringify').mockImplementation(() => { throw error; }); - instance.chat.update({attachments: []}, cb); + instance.chat.update({ attachments: [] }, cb); expect(instance.callAPI).toHaveBeenCalledWith('chat.update', {}, cb); expect(JSON.stringify).toHaveBeenCalled(); }); From c5d0fa1815d53b320c8c67f35d46dac845621d01 Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Mon, 19 Jun 2017 14:27:09 -0400 Subject: [PATCH 3/6] adds use strict --- __test__/lib/Botkit.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/__test__/lib/Botkit.test.js b/__test__/lib/Botkit.test.js index cbf8e837c..06a3eb450 100644 --- a/__test__/lib/Botkit.test.js +++ b/__test__/lib/Botkit.test.js @@ -1,3 +1,5 @@ +'use strict'; + let botkit; jest.mock('../../lib/CoreBot', () => 'corebot'); From aeb1b025f1fd8f69879ebe33096ad3bd3eb63456 Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Mon, 19 Jun 2017 14:27:17 -0400 Subject: [PATCH 4/6] fixes linting errors --- lib/CoreBot.js | 2 +- lib/Slack_web_api.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/CoreBot.js b/lib/CoreBot.js index 0183972cf..2541a985f 100755 --- a/lib/CoreBot.js +++ b/lib/CoreBot.js @@ -1007,7 +1007,7 @@ function Botkit(configuration) { } if (typeof(events) == 'string') { - events = events.split(/\,/g).map(function(str) { return str.trim() }) + events = events.split(/\,/g).map(function(str) { return str.trim(); }); } for (var e = 0; e < events.length; e++) { diff --git a/lib/Slack_web_api.js b/lib/Slack_web_api.js index 73a6a75e0..d17de8a3f 100755 --- a/lib/Slack_web_api.js +++ b/lib/Slack_web_api.js @@ -227,7 +227,7 @@ module.exports = function(bot, config) { request.post(params, function(error, response, body) { bot.debug('Got response', error, body); - if(response.statusCode == 429) { + if (response.statusCode == 429) { return cb(new Error('Rate limit exceeded')); } if (!error && response.statusCode == 200) { From c0769d36f205f5a7c92bba5714ce79e5dd60120d Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Mon, 19 Jun 2017 14:35:14 -0400 Subject: [PATCH 5/6] adds test coverage for 429 rate limiting --- __test__/lib/Slack_web_api.test.js | 15 ++++++++++++++- lib/Slack_web_api.js | 16 ++++++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/__test__/lib/Slack_web_api.test.js b/__test__/lib/Slack_web_api.test.js index c6b748928..1067b0e21 100644 --- a/__test__/lib/Slack_web_api.test.js +++ b/__test__/lib/Slack_web_api.test.js @@ -174,7 +174,20 @@ describe('postForm', () => { expect(cb).toHaveBeenCalledWith(error); }); - test(`${methodName}: handles non 200 response code`, () => { + test(`${methodName}: handles 429 response code`, () => { + mockRequest.post.mockImplementation((params, callback) => { + callback(null, { statusCode: 429 }, null); + }); + + method('some.action', 'data', cb); + + expect(mockRequest.post).toHaveBeenCalledTimes(1); + expect(cb).toHaveBeenCalledTimes(1); + const firstArg = cb.mock.calls[0][0]; + expect(firstArg.message).toBe('Rate limit exceeded'); + }); + + test(`${methodName}: handles other response codes`, () => { mockRequest.post.mockImplementation((params, callback) => { callback(null, { statusCode: 400 }, null); }); diff --git a/lib/Slack_web_api.js b/lib/Slack_web_api.js index d17de8a3f..41d88a774 100755 --- a/lib/Slack_web_api.js +++ b/lib/Slack_web_api.js @@ -3,7 +3,7 @@ var request = require('request'); /** * Does nothing. Takes no params, returns nothing. It's a no-op! */ -function noop() {} +function noop() { } /** * Returns an interface to the Slack API in the context of the given bot @@ -187,7 +187,7 @@ module.exports = function(bot, config) { }; function sanitizeOptions(options) { - if (options.attachments && typeof(options.attachments) != 'string') { + if (options.attachments && typeof (options.attachments) != 'string') { try { options.attachments = JSON.stringify(options.attachments); } catch (err) { @@ -227,10 +227,11 @@ module.exports = function(bot, config) { request.post(params, function(error, response, body) { bot.debug('Got response', error, body); - if (response.statusCode == 429) { - return cb(new Error('Rate limit exceeded')); + if (error) { + return cb(error); } - if (!error && response.statusCode == 200) { + + if (response.statusCode == 200) { var json; try { json = JSON.parse(body); @@ -239,8 +240,11 @@ module.exports = function(bot, config) { } return cb((json.ok ? null : json.error), json); + } else if (response.statusCode == 429) { + return cb(new Error('Rate limit exceeded')); + } else { + return cb(new Error('Invalid response')); } - return cb(error || new Error('Invalid response')); }); } }; From 7e03e63c15fcd0c17f472c967cf27ecbf7f4fe1a Mon Sep 17 00:00:00 2001 From: Peter Swimm Date: Mon, 19 Jun 2017 15:52:04 -0500 Subject: [PATCH 6/6] Update cisco-spark.md --- docs/provisioning/cisco-spark.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/provisioning/cisco-spark.md b/docs/provisioning/cisco-spark.md index 9814a15c2..3ac4f398b 100644 --- a/docs/provisioning/cisco-spark.md +++ b/docs/provisioning/cisco-spark.md @@ -18,7 +18,9 @@ Take note of the bot username, you'll need it later. **Note about your icon**: Cisco requires you host an avatar for your bot before you can create it in their portal. This bot needs to be a 512x512px image icon hosted anywhere on the web. This can be changed later. -You can copy and paste this URL for a Botkit icon you can use right away: `https://raw.githubusercontent.com/howdyai/botkit-starter-ciscospark/master/public/default_icon.png` +You can copy and paste this URL for a Botkit icon you can use right away: + +https://raw.githubusercontent.com/howdyai/botkit-starter-ciscospark/master/public/default_icon.png ### 3. Copy your access token