From 3d427dd13cfbf99ee059a098a01557c11081eb0d Mon Sep 17 00:00:00 2001 From: Nate Berkopec Date: Wed, 24 Dec 2014 12:23:03 -0600 Subject: [PATCH] Refactor Client + remove redundant excluded environment check + cleanup methods, esp. send + remove redundant private + remove unnecessary return statements, variable assignments + rename should_send/should_capture --- lib/raven/base.rb | 42 +++++++++++++------------------- lib/raven/client.rb | 49 ++++++++++++++++---------------------- lib/raven/configuration.rb | 4 ++-- spec/raven/raven_spec.rb | 14 +++++------ 4 files changed, 47 insertions(+), 62 deletions(-) diff --git a/lib/raven/base.rb b/lib/raven/base.rb index f923f2555..515f76afc 100644 --- a/lib/raven/base.rb +++ b/lib/raven/base.rb @@ -96,43 +96,35 @@ def capture(options = {}) end def capture_exception(exception, options = {}) - send_or_skip(exception) do - if (evt = Event.from_exception(exception, options)) - yield evt if block_given? - if configuration.async? - configuration.async.call(evt) - else - send(evt) - end + return false unless should_capture?(exception) + if (evt = Event.from_exception(exception, options)) + yield evt if block_given? + if configuration.async? + configuration.async.call(evt) + else + send(evt) end end end def capture_message(message, options = {}) - send_or_skip(message) do - if (evt = Event.from_message(message, options)) - yield evt if block_given? - if configuration.async? - configuration.async.call(evt) - else - send(evt) - end + return false unless should_capture?(message) + if (evt = Event.from_message(message, options)) + yield evt if block_given? + if configuration.async? + configuration.async.call(evt) + else + send(evt) end end end - def send_or_skip(exc) - should_send = if configuration.should_send - configuration.should_send.call(*[exc]) + def should_capture?(exc) + if configuration.should_capture + configuration.should_capture.call(*[exc]) else true end - - if configuration.send_in_current_environment? && should_send - yield if block_given? - else - configuration.log_excluded_environment_message - end end # Provides extra context to the exception prior to it being handled by diff --git a/lib/raven/client.rb b/lib/raven/client.rb index 5c0def91b..f9da08b18 100644 --- a/lib/raven/client.rb +++ b/lib/raven/client.rb @@ -7,9 +7,8 @@ require 'raven/transports/udp' module Raven - + # Encodes events and sends them to the Sentry server. class Client - PROTOCOL_VERSION = '5' USER_AGENT = "raven-ruby/#{Raven::VERSION}" CONTENT_TYPE = 'application/json' @@ -23,13 +22,7 @@ def initialize(configuration) end def send(event) - unless configuration.send_in_current_environment? - configuration.log_excluded_environment_message - return - end - - # Set the project ID correctly - event.project = self.configuration.project_id + return false unless configuration_allows_sending if !@state.should_try? Raven.logger.error("Not sending event due to previous failure(s): #{get_log_message(event)}") @@ -39,6 +32,7 @@ def send(event) Raven.logger.debug "Sending event #{event.id} to Sentry" content_type, encoded_data = encode(event) + begin transport.send(generate_auth_header, encoded_data, :content_type => content_type) @@ -54,23 +48,24 @@ def send(event) private - def encode(event) - hash = event.to_hash - - # apply processors - hash = @processors.reduce(hash) do |memo, processor| - processor.process(memo) + def configuration_allows_sending + if configuration.send_in_current_environment? + true + else + configuration.log_excluded_environment_message + false end + end + def encode(event) + hash = @processors.reduce(event.to_hash) { |memo, p| p.process(memo) } encoded = OkJson.encode(hash) - case self.configuration.encoding + case configuration.encoding when 'gzip' - gzipped = Zlib::Deflate.deflate(encoded) - b64_encoded = strict_encode64(gzipped) - return 'application/octet-stream', b64_encoded + ['application/octet-stream', strict_encode64(Zlib::Deflate.deflate(encoded))] else - return 'application/json', encoded + ['application/json', encoded] end end @@ -80,13 +75,13 @@ def get_log_message(event) def transport @transport ||= - case self.configuration.scheme + case configuration.scheme when 'udp' - Transports::UDP.new self.configuration + Transports::UDP.new(configuration) when 'http', 'https' - Transports::HTTP.new self.configuration + Transports::HTTP.new(configuration) else - raise Error.new("Unknown transport scheme '#{self.configuration.scheme}'") + raise Error, "Unknown transport scheme '#{self.configuration.scheme}'" end end @@ -96,14 +91,12 @@ def generate_auth_header 'sentry_version' => PROTOCOL_VERSION, 'sentry_client' => USER_AGENT, 'sentry_timestamp' => now, - 'sentry_key' => self.configuration.public_key, - 'sentry_secret' => self.configuration.secret_key, + 'sentry_key' => configuration.public_key, + 'sentry_secret' => configuration.secret_key } 'Sentry ' + fields.map { |key, value| "#{key}=#{value}" }.join(', ') end - private - def strict_encode64(string) if Base64.respond_to? :strict_encode64 Base64.strict_encode64 string diff --git a/lib/raven/configuration.rb b/lib/raven/configuration.rb index 7955f9773..6bdf439d0 100644 --- a/lib/raven/configuration.rb +++ b/lib/raven/configuration.rb @@ -87,8 +87,8 @@ class Configuration # ActionDispatch::ShowExceptions or ActionDispatch::DebugExceptions attr_accessor :catch_debugged_exceptions - # Provide a configurable callback to block or send events - attr_accessor :should_send + # Provide a configurable callback to determine event capture + attr_accessor :should_capture # additional fields to sanitize attr_accessor :sanitize_fields diff --git a/spec/raven/raven_spec.rb b/spec/raven/raven_spec.rb index d89bc3229..d1db866eb 100644 --- a/spec/raven/raven_spec.rb +++ b/spec/raven/raven_spec.rb @@ -70,17 +70,17 @@ end end - describe '.capture_exception with a should_send callback' do + describe '.capture_exception with a should_capture callback' do let(:exception) { build_exception } - it 'sends the result of Event.capture_exception according to the result of should_send' do + it 'sends the result of Event.capture_exception according to the result of should_capture' do expect(Raven).not_to receive(:send).with(event) - prior_should_send = Raven.configuration.should_send - Raven.configuration.should_send = Proc.new { false } - expect(Raven.configuration.should_send).to receive(:call).with(exception) - Raven.capture_exception(exception, options) - Raven.configuration.should_send = prior_should_send + prior_should_capture = Raven.configuration.should_capture + Raven.configuration.should_capture = Proc.new { false } + expect(Raven.configuration.should_capture).to receive(:call).with(exception) + expect(Raven.capture_exception(exception, options)).to be false + Raven.configuration.should_capture = prior_should_capture end end