From 95dd6bc09ce499365eb02a987f4893f48dde3cdf Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 7 Jul 2017 11:23:25 +0100 Subject: [PATCH] Add error handling to the slack API requests, and associated tests --- .../modals/admin-plugins-chat-test.js.es6 | 2 +- .../provider/slack/slack_provider.rb | 15 ++++++++++ .../provider/slack/slack_provider_spec.rb | 28 ++++++++++++------- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/assets/javascripts/admin/controllers/modals/admin-plugins-chat-test.js.es6 b/assets/javascripts/admin/controllers/modals/admin-plugins-chat-test.js.es6 index 0c97dfa..1eacb71 100644 --- a/assets/javascripts/admin/controllers/modals/admin-plugins-chat-test.js.es6 +++ b/assets/javascripts/admin/controllers/modals/admin-plugins-chat-test.js.es6 @@ -28,7 +28,7 @@ export default Ember.Controller.extend(ModalFunctionality, { var response = e.jqXHR.responseJSON var error_key = 'chat_integration.test_modal.error' - debugger; + if(response['error_key']){ error_key = response['error_key'] } diff --git a/lib/discourse_chat/provider/slack/slack_provider.rb b/lib/discourse_chat/provider/slack/slack_provider.rb index a8e368f..5302ef8 100644 --- a/lib/discourse_chat/provider/slack/slack_provider.rb +++ b/lib/discourse_chat/provider/slack/slack_provider.rb @@ -96,6 +96,21 @@ module DiscourseChat::Provider::SlackProvider response = http.request(Net::HTTP::Post.new(uri)) + unless response.kind_of? Net::HTTPSuccess + raise ::DiscourseChat::ProviderError.new info: {request: uri, response_code:response.code, response_body:response.body} + end + + json = JSON.parse(response.body) + + unless json["ok"] == true + if json.key?("error") and (json["error"] == 'channel_not_found' or json["error"] == 'is_archived') + error_key = 'chat_integration.provider.slack.errors.channel_not_found' + else + error_key = json.to_s + end + raise ::DiscourseChat::ProviderError.new info: {error_key: error_key, request: uri, response_code:response.code, response_body:response.body} + end + DiscourseChat.pstore_set("slack_topic_#{post.topic.id}_#{channel}", JSON.parse(response.body) ) response end diff --git a/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb b/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb index 4827480..7a93862 100644 --- a/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb +++ b/spec/lib/discourse_chat/provider/slack/slack_provider_spec.rb @@ -57,23 +57,25 @@ RSpec.describe DiscourseChat::Provider::SlackProvider do SiteSetting.chat_integration_slack_enabled = true end - before do - @stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(body: "success") + it 'sends a webhook request' do + stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(body: "success") + described_class.trigger_notification(post, '#general') + expect(stub1).to have_been_requested.once end - - - it 'sends a webhook request' do - expect(@stub1).to have_been_requested.times(0) - described_class.trigger_notification(post, '#general') - expect(@stub1).to have_been_requested.once + it 'handles errors correctly' do + stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(status: 400, body: "error") + expect(stub1).to have_been_requested.times(0) + expect{described_class.trigger_notification(post, '#general')}.to raise_exception(::DiscourseChat::ProviderError) + expect(stub1).to have_been_requested.once end describe 'with api token' do before do SiteSetting.chat_integration_slack_access_token = "magic" - @stub2 = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).to_return(body: "{\"success\":true, \"ts\": \"#{Time.now.to_i}.012345\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\"} }", headers: {'Content-Type' => 'application/json'}) - @stub3 = stub_request(:post, %r{https://slack.com/api/chat.update}).to_return(body: '{"success":true, "ts": "some_message_id"}', headers: {'Content-Type' => 'application/json'}) + @stub1 = stub_request(:post, SiteSetting.chat_integration_slack_outbound_webhook_url).to_return(body: "success") + @stub2 = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).to_return(body: "{\"ok\":true, \"ts\": \"#{Time.now.to_i}.012345\", \"message\": {\"attachments\": [], \"username\":\"blah\", \"text\":\"blah2\"} }", headers: {'Content-Type' => 'application/json'}) + @stub3 = stub_request(:post, %r{https://slack.com/api/chat.update}).to_return(body: '{"ok":true, "ts": "some_message_id"}', headers: {'Content-Type' => 'application/json'}) end it 'sends an api request' do @@ -84,6 +86,12 @@ RSpec.describe DiscourseChat::Provider::SlackProvider do expect(@stub2).to have_been_requested.once end + it 'handles errors correctly' do + @stub2 = stub_request(:post, %r{https://slack.com/api/chat.postMessage}).to_return(body: "{\"ok\":false }", headers: {'Content-Type' => 'application/json'}) + expect{described_class.trigger_notification(post, '#general')}.to raise_exception(::DiscourseChat::ProviderError) + expect(@stub2).to have_been_requested.once + end + it 'correctly merges replies' do second_post = Fabricate(:post, topic: post.topic, post_number:2) expect(@stub2).to have_been_requested.times(0)