From 1d9db20b919b57817d63fac43e810905f4c28f62 Mon Sep 17 00:00:00 2001 From: Emilio Cristalli Date: Sat, 2 Feb 2019 22:17:56 -0300 Subject: [PATCH] Use formatter in Mattermost notifier --- .rubocop_todo.yml | 1 - .../google_chat_notifier.rb | 16 +- lib/exception_notifier/mattermost_notifier.rb | 164 ++++-------------- lib/exception_notifier/modules/formatter.rb | 15 +- .../mattermost_notifier_test.rb | 16 +- .../modules/formatter_test.rb | 26 ++- 6 files changed, 88 insertions(+), 150 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 1f0ed3ac..d447edba 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -56,5 +56,4 @@ Style/ClassVars: Style/MethodMissing: Exclude: - 'lib/exception_notifier/email_notifier.rb' - - 'lib/exception_notifier/mattermost_notifier.rb' - 'lib/exception_notifier/teams_notifier.rb' diff --git a/lib/exception_notifier/google_chat_notifier.rb b/lib/exception_notifier/google_chat_notifier.rb index 8218df35..94ce8480 100644 --- a/lib/exception_notifier/google_chat_notifier.rb +++ b/lib/exception_notifier/google_chat_notifier.rb @@ -21,11 +21,21 @@ def body(exception, formatter) formatter.subtitle, '', formatter.title, - "*#{exception.message.tr('`', "'")}*", - formatter.request_message, - formatter.backtrace_message + "*#{exception.message.tr('`', "'")}*" ] + if (request = formatter.request_message.presence) + text << '' + text << '*Request:*' + text << request + end + + if (backtrace = formatter.backtrace_message.presence) + text << '' + text << '*Backtrace:*' + text << backtrace + end + text.compact.join("\n") end end diff --git a/lib/exception_notifier/mattermost_notifier.rb b/lib/exception_notifier/mattermost_notifier.rb index 827b53b9..16554655 100644 --- a/lib/exception_notifier/mattermost_notifier.rb +++ b/lib/exception_notifier/mattermost_notifier.rb @@ -1,163 +1,75 @@ -require 'action_dispatch' -require 'active_support/core_ext/time' +require 'httparty' module ExceptionNotifier - class MattermostNotifier - include ExceptionNotifier::BacktraceCleaner - - class MissingController - def method_missing(*args, &block); end - end - - attr_accessor :httparty - - def initialize(options = {}) - super() - @default_options = options - @httparty = HTTParty - end - + class MattermostNotifier < BaseNotifier def call(exception, options = {}) - @options = options.merge(@default_options) + @options = options.merge(base_options) @exception = exception - @backtrace = exception.backtrace ? clean_backtrace(exception) : nil - @env = @options.delete(:env) + @formatter = Formatter.new( + exception, + env: @options.delete(:env), + app_name: @options.delete(:app_name), + accumulated_errors_count: @options[:accumulated_errors_count] + ) - @application_name = @options.delete(:app_name) || Rails.application.class.parent_name.underscore + avatar = @options.delete(:avatar) + channel = @options.delete(:channel) @gitlab_url = @options.delete(:git_url) - @username = @options.delete(:username) || 'Exception Notifier' - @avatar = @options.delete(:avatar) - - @channel = @options.delete(:channel) @webhook_url = @options.delete(:webhook_url) raise ArgumentError, "You must provide 'webhook_url' parameter." unless @webhook_url - if @env.nil? - @controller = @request_items = nil - else - @controller = @env['action_controller.instance'] || MissingController.new - - request = ActionDispatch::Request.new(@env) - - @request_items = { url: request.original_url, - http_method: request.method, - ip_address: request.remote_ip, - parameters: request.filtered_parameters, - timestamp: Time.current } - - if request.session['warden.user.user.key'] - current_user = User.find(request.session['warden.user.user.key'][0][0]) - @request_items[:current_user] = { id: current_user.id, email: current_user.email } - end - end - - payload = message_text.merge(user_info).merge(channel_info) + payload = { + text: message_text.compact.join("\n") + } + payload[:username] = @options.delete(:username) || 'Exception Notifier' + payload[:icon_url] = avatar if avatar + payload[:channel] = channel if channel @options[:body] = payload.to_json @options[:headers] ||= {} @options[:headers]['Content-Type'] = 'application/json' - @httparty.post(@webhook_url, @options) + HTTParty.post(@webhook_url, @options) end private - def channel_info - if @channel - { channel: @channel } - else - {} - end - end - - def user_info - infos = {} - - infos[:username] = @username if @username - infos[:icon_url] = @avatar if @avatar - - infos - end + attr_reader :formatter def message_text - text = [] - - text += ['@channel'] - text += message_header - text += message_request if @request_items - text += message_backtrace if @backtrace - text += message_issue_link if @gitlab_url - - { text: text.join("\n") } - end - - def message_header - text = [] - - errors_count = @options[:accumulated_errors_count].to_i - text << "### :warning: Error 500 in #{Rails.env} :warning:" - text << "#{errors_count > 1 ? errors_count : 'An'} *#{@exception.class}* occured" + (@controller ? " in *#{controller_and_method}*." : '.') - text << "*#{@exception.message}*" - - text - end - - def message_request - text = [] - - text << '### Request' - text << '```' - text << hash_presentation(@request_items) - text << '```' - - text - end + text = [ + '@channel', + "### #{formatter.title}", + formatter.subtitle, + "*#{@exception.message}*" + ] + + if (request = formatter.request_message.presence) + text << '### Request' + text << request + end - def message_backtrace(size = 3) - text = [] + if (backtrace = formatter.backtrace_message.presence) + text << '### Backtrace' + text << backtrace + end - size = @backtrace.size < size ? @backtrace.size : size - text << '### Backtrace' - text << '```' - size.times { |i| text << '* ' + @backtrace[i] } - text << '```' + text << message_issue_link if @gitlab_url text end def message_issue_link - text = [] - - link = [@gitlab_url, @application_name, 'issues', 'new'].join('/') + link = [@gitlab_url, formatter.app_name, 'issues', 'new'].join('/') params = { 'issue[title]' => ['[BUG] Error 500 :', - controller_and_method, + formatter.controller_and_action || '', "(#{@exception.class})", @exception.message].compact.join(' ') }.to_query - text << "[Create an issue](#{link}/?#{params})" - - text - end - - def controller_and_method - if @controller - "#{@controller.controller_name}##{@controller.action_name}" - else - '' - end - end - - def hash_presentation(hash) - text = [] - - hash.each do |key, value| - text << "* #{key} : #{value}" - end - - text.join("\n") + "[Create an issue](#{link}/?#{params})" end end end diff --git a/lib/exception_notifier/modules/formatter.rb b/lib/exception_notifier/modules/formatter.rb index d946b63c..d62c7659 100644 --- a/lib/exception_notifier/modules/formatter.rb +++ b/lib/exception_notifier/modules/formatter.rb @@ -57,8 +57,6 @@ def request_message return unless request [ - '', - '*Request:*', '```', "* url : #{request.original_url}", "* http_method : #{request.method}", @@ -85,8 +83,6 @@ def backtrace_message text = [] - text << '' - text << '*Backtrace:*' text << '```' backtrace.first(3).each { |line| text << "* #{line}" } text << '```' @@ -94,6 +90,13 @@ def backtrace_message text.join("\n") end + # + # home#index + # + def controller_and_action + "#{controller.controller_name}##{controller.action_name}" if controller + end + private attr_reader :exception, :env, :errors_count @@ -106,9 +109,5 @@ def rails_app_name def controller env['action_controller.instance'] if env end - - def controller_and_action - "#{controller.controller_name}##{controller.action_name}" if controller - end end end diff --git a/test/exception_notifier/mattermost_notifier_test.rb b/test/exception_notifier/mattermost_notifier_test.rb index 953d30d8..3035b73f 100644 --- a/test/exception_notifier/mattermost_notifier_test.rb +++ b/test/exception_notifier/mattermost_notifier_test.rb @@ -27,8 +27,8 @@ def teardown body = default_body.merge( text: [ '@channel', - '### :warning: Error 500 in test :warning:', - 'An *ArgumentError* occured.', + '### ⚠️ Error occurred in test ⚠️', + 'A *ArgumentError* occurred.', '*foo*', '[Create an issue](github.com/aschen/dummy/issues/new/?issue%5Btitle%5D=%5BBUG%5D+Error+500+%3A++%28ArgumentError%29+foo)' ].join("\n") @@ -96,8 +96,8 @@ def teardown body = default_body.merge( text: [ '@channel', - '### :warning: Error 500 in test :warning:', - '5 *ArgumentError* occured.', + '### ⚠️ Error occurred in test ⚠️', + '5 *ArgumentError* occurred.', '*foo*' ].join("\n") ) @@ -116,8 +116,8 @@ def teardown body = default_body.merge( text: [ '@channel', - '### :warning: Error 500 in test :warning:', - 'An *ArgumentError* occured in *#*.', + '### ⚠️ Error occurred in test ⚠️', + 'A *ArgumentError* occurred.', '*foo*', '### Request', '```', @@ -161,8 +161,8 @@ def default_body { text: [ '@channel', - '### :warning: Error 500 in test :warning:', - 'An *ArgumentError* occured.', + '### ⚠️ Error occurred in test ⚠️', + 'A *ArgumentError* occurred.', '*foo*' ].join("\n"), username: 'Exception Notifier' diff --git a/test/exception_notifier/modules/formatter_test.rb b/test/exception_notifier/modules/formatter_test.rb index c2baf46c..2199e1ec 100644 --- a/test/exception_notifier/modules/formatter_test.rb +++ b/test/exception_notifier/modules/formatter_test.rb @@ -66,8 +66,6 @@ def index; end # test 'request_message when env set' do text = [ - '', - '*Request:*', '```', '* url : http://test.address/?id=foo', '* http_method : GET', @@ -98,8 +96,6 @@ def index; end # test 'backtrace_message when backtrace set' do text = [ - '', - '*Backtrace:*', '```', "* app/controllers/my_controller.rb:53:in `my_controller_params'", "* app/controllers/my_controller.rb:34:in `update'", @@ -119,4 +115,26 @@ def index; end formatter = ExceptionNotifier::Formatter.new(@exception) assert_nil formatter.backtrace_message end + + # + # #controller_and_action + # + test 'correct controller_and_action if controller is present' do + controller = HomeController.new + controller.process(:index) + + env = Rack::MockRequest.env_for( + '/', 'action_controller.instance' => controller + ) + + formatter = ExceptionNotifier::Formatter.new(@exception, env: env) + assert_equal 'home#index', formatter.controller_and_action + end + + test 'controller_and_action is nil if no controller' do + env = Rack::MockRequest.env_for('/') + + formatter = ExceptionNotifier::Formatter.new(@exception, env: env) + assert_nil formatter.controller_and_action + end end