From b32525d71c278be41c3d005728bfc08bdb9fb51d Mon Sep 17 00:00:00 2001
From: Joe Haines <hello@joehaines.co.uk>
Date: Tue, 4 Aug 2020 16:59:34 +0100
Subject: [PATCH] Delay serialization until delivery in thread queue

---
 CHANGELOG.md                         |  3 +++
 lib/bugsnag.rb                       | 30 ++++++++++++++++++----------
 lib/bugsnag/delivery/thread_queue.rb | 21 ++++++++++++++++---
 3 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1d85e7df5..525b7a255 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,9 @@ Changelog
   | [#571](https://github.com/bugsnag/bugsnag-ruby/pull/571)
   | [t-richards](https://github.com/t-richards)
 
+* Move serialization of Reports onto the background thread when using the thread_queue delivery method
+  | [#623](https://github.com/bugsnag/bugsnag-ruby/pull/623)
+
 ## Fixes
 
 * The `app_type` configuration option should no longer be overwritten by Bugsnag and integrations should set this more consistently
diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb
index 7d215b3d6..0a7da0bb2 100644
--- a/lib/bugsnag.rb
+++ b/lib/bugsnag.rb
@@ -303,15 +303,25 @@ def abort_reason(exception, auto_notify)
     def deliver_notification(report)
       configuration.info("Notifying #{configuration.notify_endpoint} of #{report.exceptions.last[:errorClass]}")
 
-      payload = report_to_json(report)
-      options = {:headers => report.headers}
-
-      Bugsnag::Delivery[configuration.delivery_method].deliver(
-        configuration.notify_endpoint,
-        payload,
-        configuration,
-        options
-      )
+      options = { headers: report.headers }
+
+      delivery_method = Bugsnag::Delivery[configuration.delivery_method]
+
+      if delivery_method.respond_to?(:serialize_and_deliver)
+        delivery_method.serialize_and_deliver(
+          configuration.notify_endpoint,
+          proc { report_to_json(report) },
+          configuration,
+          options
+        )
+      else
+        delivery_method.deliver(
+          configuration.notify_endpoint,
+          report_to_json(report),
+          configuration,
+          options
+        )
+      end
 
       leave_breadcrumb(
         report.summary[:error_class],
@@ -353,7 +363,7 @@ def check_endpoint_setup
     # encoding errors and redacting metadata according to "meta_data_filters"
     #
     # @param report [Report]
-    # @return string
+    # @return [String]
     def report_to_json(report)
       cleaned = cleaner.clean_object(report.as_json)
       trimmed = Bugsnag::Helpers.trim_if_needed(cleaned)
diff --git a/lib/bugsnag/delivery/thread_queue.rb b/lib/bugsnag/delivery/thread_queue.rb
index 1e3649675..50423cedb 100644
--- a/lib/bugsnag/delivery/thread_queue.rb
+++ b/lib/bugsnag/delivery/thread_queue.rb
@@ -9,8 +9,14 @@ class ThreadQueue < Synchronous
 
       class << self
         ##
-        # Queues a given payload to be delivered asynchronously.
-        def deliver(url, body, configuration, options={})
+        # Queues a given payload to be delivered asynchronously
+        #
+        # @param url [String]
+        # @param get_payload [Proc] A Proc that will return the payload.
+        # @param configuration [Bugsnag::Configuration]
+        # @param options [Hash]
+        # @return [void]
+        def serialize_and_deliver(url, get_payload, configuration, options={})
           @configuration = configuration
 
           start_once!
@@ -21,7 +27,16 @@ def deliver(url, body, configuration, options={})
           end
 
           # Add delivery to the worker thread
-          @queue.push proc { super(url, body, configuration, options) }
+          @queue.push(proc do
+            begin
+              payload = get_payload.call
+            rescue StandardError => e
+              configuration.warn("Notification to #{url} failed, #{e.inspect}")
+              configuration.warn(e.backtrace)
+            end
+
+            Synchronous.deliver(url, payload, configuration, options) unless payload.nil?
+          end)
         end
 
         private