- | {{rule.channel}} |
+
+ {{#if rule.error_key}}
+
+ {{d-button action="showError" actionParam=rule.error_key class="delete btn-danger" icon="exclamation-triangle"}}
+
+ {{/if}}
+
+ {{rule.channel}}
+ |
{{rule.filterName}} |
@@ -44,7 +59,7 @@
|
{{d-button action="edit" actionParam=rule icon="pencil" class="edit" title="chat_integration.rule_table.edit_rule"}}
- {{d-button action="delete" actionParam=rule icon="trash-o" class="delete btn-danger" title="chat_integration.rule_table.delete_rule"}}
+ {{d-button action="delete" actionParam=rule icon="trash-o" class="delete" title="chat_integration.rule_table.delete_rule"}}
|
{{/each}}
diff --git a/assets/stylesheets/chat-integration-admin.scss b/assets/stylesheets/chat-integration-admin.scss
index 27a6276..d5aaad3 100644
--- a/assets/stylesheets/chat-integration-admin.scss
+++ b/assets/stylesheets/chat-integration-admin.scss
@@ -13,6 +13,19 @@
margin-top: 10px;
}
+ div.error {
+ font-size: 1.1em;
+ font-weight:bold;
+ max-width: 100%;
+ margin-top: 10px;
+ margin-bottom: 10px;
+ background-color: dark-light-diff($quaternary, $secondary, 70%, -70%);
+ padding: 15px;
+
+
+ }
+
+
}
#chat_integration_edit_rule_modal{
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 93266fc..10e374c 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -4,6 +4,8 @@ en:
menu_title: "Chat Integrations"
settings: "Settings"
no_providers: "You need to enable some providers in the plugin settings"
+ rules_with_errors: "Some rules for this provider failed last time they were triggered. Click the error icon(s) to learn more."
+ rule_exception: "An unknown error occured when this rule was last triggered. Check the site logs for more information."
all_categories: "(all categories)"
all_tags: "(all tags)"
create_rule: "Create Rule"
@@ -43,6 +45,9 @@ en:
slack:
title: "Slack"
channel_instructions: "e.g. #channel, @username."
+ errors:
+ action_prohibited: "The bot does not have permission to post to that channel"
+ channel_not_found: "The specified channel does not exist on slack"
#######################################
########## TELEGRAM STRINGS ###########
diff --git a/lib/discourse_chat/manager.rb b/lib/discourse_chat/manager.rb
index b9952d9..76b8098 100644
--- a/lib/discourse_chat/manager.rb
+++ b/lib/discourse_chat/manager.rb
@@ -61,11 +61,30 @@ module DiscourseChat
# Loop through each rule, and trigger appropriate notifications
matching_rules.each do |rule|
- Rails.logger.info("Sending notification to provider #{rule.provider}, channel #{rule.channel}")
provider = ::DiscourseChat::Provider.get_by_name(rule.provider)
is_enabled = ::DiscourseChat::Provider.is_enabled(provider)
+
if provider and is_enabled
- provider.trigger_notification(post, rule.channel)
+ begin
+ provider.trigger_notification(post, rule.channel)
+ rule.update({error_key: nil}, false) if rule.error_key
+ rescue => e
+ if e.class == DiscourseChat::ProviderError and e.info.key?(:error_key) and !e.info[:error_key].nil?
+ rule.error_key = e.info[:error_key]
+ else
+ rule.error_key = 'chat_integration.rule_exception'
+ end
+ rule.save(false) # Save without validations
+
+ # Log the error
+ Discourse.handle_job_exception(e,
+ message: "Triggering notifications failed",
+ extra: { provider_name: provider::PROVIDER_NAME,
+ channel: rule.channel,
+ post_id: post.id,
+ error_info: e.class == DiscourseChat::ProviderError ? e.info : nil }
+ )
+ end
elsif provider
# Provider is disabled, don't do anything
else
diff --git a/lib/discourse_chat/provider.rb b/lib/discourse_chat/provider.rb
index 41119a4..364bca5 100644
--- a/lib/discourse_chat/provider.rb
+++ b/lib/discourse_chat/provider.rb
@@ -1,4 +1,13 @@
module DiscourseChat
+ class ProviderError < StandardError
+ attr_accessor :info
+
+ def initialize(message = nil, info: nil)
+ super(message)
+ self.info = info.nil? ? {} : info
+ end
+ end
+
module Provider
def self.providers
constants.select do |constant|
diff --git a/lib/discourse_chat/provider/slack/slack_provider.rb b/lib/discourse_chat/provider/slack/slack_provider.rb
index 4800361..a8674df 100644
--- a/lib/discourse_chat/provider/slack/slack_provider.rb
+++ b/lib/discourse_chat/provider/slack/slack_provider.rb
@@ -105,7 +105,19 @@ module DiscourseChat::Provider::SlackProvider
req = Net::HTTP::Post.new(URI(SiteSetting.chat_integration_slack_outbound_webhook_url), 'Content-Type' =>'application/json')
req.body = message.to_json
response = http.request(req)
- response
+
+ unless response.kind_of? Net::HTTPSuccess
+ if response.code.to_s == '403'
+ error_key = 'chat_integration.provider.slack.errors.action_prohibited'
+ elsif response.body == 'channel_not_found' or response.body == 'channel_is_archived'
+ error_key = 'chat_integration.provider.slack.errors.channel_not_found'
+ else
+ error_key = nil
+ end
+ raise ::DiscourseChat::ProviderError.new info: {error_key: error_key, request: req.body, response_code:response.code, response_body:response.body}
+ end
+
+
end
def self.trigger_notification(post, channel)
diff --git a/lib/discourse_chat/rule.rb b/lib/discourse_chat/rule.rb
index d8355ac..03d82b5 100644
--- a/lib/discourse_chat/rule.rb
+++ b/lib/discourse_chat/rule.rb
@@ -2,7 +2,7 @@
# Using this means we can use a standard serializer for sending JSON to the client, and also have convenient save/update/delete methods
# Since this is now being used in two plugins, maybe it should be built into core somehow
class DiscourseChat::Rule
- attr_accessor :id, :provider, :channel, :category_id, :tags, :filter
+ attr_accessor :id, :provider, :channel, :category_id, :tags, :filter, :error_key
def initialize(h={})
@provider = ''
@@ -10,6 +10,7 @@
@category_id = nil
@tags = []
@filter = 'watch'
+ @error_key = nil
h.each {|k,v| public_send("#{k}=",v)}
end
@@ -41,7 +42,7 @@
def self.from_hash(h)
rule = DiscourseChat::Rule.new
- [:provider, :channel, :category_id, :tags, :filter].each do |sym|
+ [:provider, :channel, :category_id, :tags, :filter, :error_key].each do |sym|
rule.send("#{sym}=", h[sym]) if h[sym]
end
if h[:id]
@@ -58,6 +59,7 @@
category_id: @category_id,
tags: @tags,
filter: @filter,
+ error_key: @error_key,
}
end
@@ -70,16 +72,18 @@
from_hash hash
end
- def update(h)
- [:provider, :channel, :category_id, :tags, :filter].each do |sym|
- public_send("#{sym}=", h[sym]) if h[sym]
+ def update(h, validate=true)
+ [:provider, :channel, :category_id, :tags, :filter, :error_key].each do |sym|
+ public_send("#{sym}=", h[sym]) if h.key?(sym)
end
- save
+ save(validate)
end
- def save
- return false if not valid?
+ def save(validate=true)
+ if validate
+ return false if not valid?
+ end
unless @id && @id > 0
@id = self.class.alloc_id
@@ -88,8 +92,8 @@
return self
end
- def save!
- if not save
+ def save!(validate=true)
+ if not save(validate)
raise 'Rule not valid'
end
return self
diff --git a/plugin.rb b/plugin.rb
index 57ebf47..14999c3 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -43,6 +43,7 @@ after_initialize do
module ::Jobs
class NotifyChats < Jobs::Base
+ sidekiq_options retry: false # Don't retry, could result in duplicate notifications for some providers
def execute(args)
return if not SiteSetting.chat_integration_enabled? # Plugin may have been disabled since job triggered
@@ -113,6 +114,7 @@ after_initialize do
def update_rule
begin
rule = DiscourseChat::Rule.find(params[:id].to_i)
+ rule.error_key = nil # Reset any error on the rule
hash = params.require(:rule)
if not rule.update(hash)
@@ -135,7 +137,7 @@ after_initialize do
end
class DiscourseChat::RuleSerializer < ActiveModel::Serializer
- attributes :id, :provider, :channel, :category_id, :tags, :filter
+ attributes :id, :provider, :channel, :category_id, :tags, :filter, :error_key
end
require_dependency 'admin_constraint'
diff --git a/spec/lib/discourse_chat/manager_spec.rb b/spec/lib/discourse_chat/manager_spec.rb
index 78157ea..1a910e7 100644
--- a/spec/lib/discourse_chat/manager_spec.rb
+++ b/spec/lib/discourse_chat/manager_spec.rb
@@ -19,14 +19,22 @@ RSpec.describe DiscourseChat::Manager do
PROVIDER_NAME = "dummy".freeze
PROVIDER_ENABLED_SETTING = :chat_integration_enabled # Tie to main plugin enabled setting
@@sent_messages = []
+ @@raise_exception = nil
def self.trigger_notification(post, channel)
+ if @@raise_exception
+ raise @@raise_exception
+ end
@@sent_messages.push(post: post.id, channel: channel)
end
def self.sent_messages
@@sent_messages
end
+
+ def self.set_raise_exception(bool)
+ @@raise_exception = bool
+ end
end
end
@@ -40,7 +48,28 @@ RSpec.describe DiscourseChat::Manager do
DiscourseChat::Rule.new({provider: provider, channel: channel, filter:filter, category_id:category_id, tags:tags}).save!
end
- it "should only send notifications when provider is enabled" do
+ it "should fail gracefully when a provider throws an exception" do
+ create_rule('dummy', 'chan1', 'watch', category.id, nil)
+
+ # Triggering a ProviderError should set the error_key to the error message
+ ::DiscourseChat::Provider::DummyProvider.set_raise_exception(DiscourseChat::ProviderError.new info: {error_key:"hello"})
+ manager.trigger_notifications(first_post.id)
+ expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly()
+ expect(DiscourseChat::Rule.all.first.error_key).to eq('hello')
+
+ # Triggering a different error should set the error_key to a generic message
+ ::DiscourseChat::Provider::DummyProvider.set_raise_exception(StandardError.new "hello")
+ manager.trigger_notifications(first_post.id)
+ expect(provider.sent_messages.map{|x| x[:channel]}).to contain_exactly()
+ expect(DiscourseChat::Rule.all.first.error_key).to eq('chat_integration.rule_exception')
+
+ ::DiscourseChat::Provider::DummyProvider.set_raise_exception(nil)
+
+ manager.trigger_notifications(first_post.id)
+ expect(DiscourseChat::Rule.all.first.error_key.nil?).to be true
+ end
+
+ it "should not send notifications when provider is disabled" do
SiteSetting.chat_integration_enabled = false
create_rule('dummy', 'chan1', 'watch', category.id, nil)