Session tracking fixes

CHANGELOG.md

@@ -5,8 +5,8 @@ Changelog
 
 ### Enhancements
 
-* Adds support for tracking sessions and crash rate by setting the configuration option `configuration.track_sessions` to `true`.
-  Sessions can be manually created using `Bugsnag.start_session`, and manually delivered using `Bugsnag.send_sessions`.
+* Adds support for tracking sessions and crash rate by setting the configuration option `configuration.auto_capture_sessions` to `true`.
+  Sessions can be manually created using `Bugsnag.start_session`.
   | [#411](https://github.com/bugsnag/bugsnag-ruby/pull/411)
 
 ## 6.4.0 (21 Dec 2017)

bugsnag.gemspec

@@ -18,4 +18,5 @@ Gem::Specification.new do |s|
   ]
   s.require_paths = ["lib"]
   s.required_ruby_version = '>= 1.9.2'
+  s.add_runtime_dependency 'concurrent-ruby', '~> 1.0'
 end

example/rails-51/config/initializers/bugsnag.rb

@@ -1,4 +1,4 @@
 Bugsnag.configure do |config|
   config.api_key = "YOUR_API_KEY"
-  config.track_sessions = true
+  config.auto_capture_sessions = true
 end

lib/bugsnag.rb

@@ -39,8 +39,6 @@ def configure
         configuration.warn("No valid API key has been set, notifications will not be sent")
         @key_warning = true
       end
-
-      session_tracker.config = configuration
     end
 
     # Explicitly notify of an exception
@@ -116,8 +114,9 @@ def notify(exception, auto_notify=false, &block)
 
         # Deliver
         configuration.info("Notifying #{configuration.endpoint} of #{report.exceptions.last[:errorClass]}")
-        options = {:headers => report.headers, :trim_payload => true}
-        Bugsnag::Delivery[configuration.delivery_method].deliver(configuration.endpoint, report.as_json, configuration, options)
+        options = {:headers => report.headers}
+        payload = ::JSON.dump(Bugsnag::Helpers.trim_if_needed(report.as_json))
+        Bugsnag::Delivery[configuration.delivery_method].deliver(configuration.endpoint, payload, configuration, options)
       end
     end
 
@@ -127,9 +126,14 @@ def configuration
       @configuration || LOCK.synchronize { @configuration ||= Bugsnag::Configuration.new }
     end
 
+    # Session tracking
     def session_tracker
       @session_tracker = nil unless defined?(@session_tracker)
-      @session_tracker || LOCK.synchronize { @session_tracker ||= Bugsnag::SessionTracker.new(configuration)}
+      @session_tracker || LOCK.synchronize { @session_tracker ||= Bugsnag::SessionTracker.new}
+    end
+
+    def start_session
+      session_tracker.start_session
     end
 
     # Allow access to "before notify" callbacks

lib/bugsnag/configuration.rb

@@ -23,7 +23,7 @@ class Configuration
     attr_accessor :app_type
     attr_accessor :meta_data_filters
     attr_accessor :endpoint
-    attr_accessor :logger 
+    attr_accessor :logger
     attr_accessor :middleware
     attr_accessor :internal_middleware
     attr_accessor :proxy_host
@@ -33,6 +33,7 @@ class Configuration
     attr_accessor :timeout
     attr_accessor :hostname
     attr_accessor :ignore_classes
+    attr_accessor :auto_capture_sessions
     attr_accessor :track_sessions
     attr_accessor :session_endpoint
 
@@ -49,6 +50,8 @@ class Configuration
       "rack.request.form_vars"
     ].freeze
 
+    alias :track_sessions :auto_capture_sessions
+
     def initialize
       @mutex = Mutex.new
 
@@ -61,7 +64,7 @@ def initialize
       self.hostname = default_hostname
       self.timeout = 15
       self.notify_release_stages = nil
-      self.track_sessions = false
+      self.auto_capture_sessions = false
       self.session_endpoint = DEFAULT_SESSION_ENDPOINT
 
       # SystemExit and Interrupt are common Exception types seen with successful

lib/bugsnag/delivery/synchronous.rb

@@ -4,18 +4,13 @@
 module Bugsnag
   module Delivery
     class Synchronous
-      BACKOFF_THREADS = {}
-      BACKOFF_REQUESTS = {}
-      BACKOFF_LOCK = Mutex.new
-
       class << self
         def deliver(url, body, configuration, options={})
           begin
             response = request(url, body, configuration, options)
             configuration.debug("Request to #{url} completed, status: #{response.code}")
-            success = options[:success] || '200'
-            if options[:backoff] && !(response.code == success)
-              backoff(url, body, configuration, options)
+            if response.code[0] != "2"
+              configuration.warn("Notifications to #{url} was reported unsuccessful with code #{response.code}")
             end
           rescue StandardError => e
             # KLUDGE: Since we don't re-raise http exceptions, this breaks rspec
@@ -31,11 +26,6 @@ def deliver(url, body, configuration, options={})
         def request(url, body, configuration, options)
           uri = URI.parse(url)
 
-          if options[:trim_payload]
-            body = Bugsnag::Helpers.trim_if_needed(body)
-          end
-          payload = ::JSON.dump(body)
-
           if configuration.proxy_host
             http = Net::HTTP.new(uri.host, uri.port, configuration.proxy_host, configuration.proxy_port, configuration.proxy_user, configuration.proxy_password)
           else
@@ -54,90 +44,11 @@ def request(url, body, configuration, options)
           headers.merge!(default_headers)
 
           request = Net::HTTP::Post.new(path(uri), headers)
-          request.body = payload
+          request.body = body
 
           http.request(request)
         end
 
-        def backoff(url, body, configuration, options)
-          # Ensure we have the latest configuration for making these requests
-          @latest_configuration = configuration
-
-          BACKOFF_LOCK.lock
-          begin
-            # Define an exit function once to handle outstanding requests
-            @registered_at_exit = false unless defined?(@registered_at_exit)
-            if !@registered_at_exit
-              @registered_at_exit = true
-              at_exit do
-                backoff_exit
-              end
-            end
-            if BACKOFF_REQUESTS[url] && !BACKOFF_REQUESTS[url].empty?
-              last_request = BACKOFF_REQUESTS[url].last
-              new_body_length = ::JSON.dump(body).length
-              old_body_length = ::JSON.dump(last_request[:body]).length
-              if new_body_length + old_body_length >= Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
-                BACKOFF_REQUESTS[url].push({:body => body, :options => options})
-              else
-                Bugsnag::Helpers::deep_merge!(last_request, {:body => body, :options => options})
-              end
-            else
-              BACKOFF_REQUESTS[url] = [{:body => body, :options => options}]
-            end
-            if !(BACKOFF_THREADS[url] && BACKOFF_THREADS[url].status)
-              spawn_backoff_thread(url)
-            end
-          ensure
-            BACKOFF_LOCK.unlock
-          end
-        end
-
-        def backoff_exit
-          # Kill existing threads
-          BACKOFF_THREADS.each do |url, thread|
-            thread.exit
-          end
-          # Retry outstanding requests once, then exit
-          BACKOFF_REQUESTS.each do |url, requests|
-            requests.map! do |req|
-              response = request(url, req[:body], @latest_configuration, req[:options])
-              success = req[:options][:success] || '200'
-              response.code == success
-            end
-            requests.reject! { |i| i }
-            @latest_configuration.warn("Requests to #{url} finished, #{requests.size} failed")
-          end
-        end
-
-        def spawn_backoff_thread(url)
-          new_thread = Thread.new(url) do |url|
-            interval = 2
-            while BACKOFF_REQUESTS[url].size > 0
-              sleep(interval)
-              interval = interval * 2
-              interval = 600 if interval > 600
-              BACKOFF_LOCK.lock
-              begin
-                BACKOFF_REQUESTS[url].map! do |req|
-                  response = request(url, req[:body], @latest_configuration, req[:options])
-                  success = req[:options][:success] || '200'
-                  if response.code == success
-                    @latest_configuration.debug("Request to #{url} completed, status: #{response.code}")
-                    false
-                  else
-                    req
-                  end
-                end
-                BACKOFF_REQUESTS[url].reject! { |i| !i }
-              ensure
-                BACKOFF_LOCK.unlock
-              end
-            end
-          end
-          BACKOFF_THREADS[url] = new_thread
-        end
-
         def path(uri)
           uri.path == "" ? "/" : uri.path
         end

lib/bugsnag/integrations/rack.rb

@@ -34,7 +34,9 @@ def initialize(app)
     def call(env)
       # Set the request data for bugsnag middleware to use
       Bugsnag.configuration.set_request_data(:rack_env, env)
-      Bugsnag.session_tracker.create_session
+      if Bugsnag.configuration.auto_capture_sessions
+        Bugsnag.start_session
+      end
 
       begin
         response = @app.call(env)