From 6a8cc410d2ff24388453b6b624548ddbfadaec6d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 20 Nov 2024 17:13:10 +1100 Subject: [PATCH] Replace broken order data Bugsnag expects a string as value, not an ActiveRecord object. The result was just "filtered" data, not showing any of the order's attributes. Since wanting to share the order data seems a common pattern, I added a convenience method for it. --- app/controllers/concerns/order_completion.rb | 8 ++------ app/controllers/concerns/order_stock_check.rb | 4 +--- app/jobs/backorder_job.rb | 4 +--- app/jobs/stock_sync_job.rb | 8 ++------ app/services/alert.rb | 7 +++++++ app/services/place_proxy_order.rb | 4 +--- spec/services/alert_spec.rb | 17 +++++++++++++++++ 7 files changed, 31 insertions(+), 21 deletions(-) diff --git a/app/controllers/concerns/order_completion.rb b/app/controllers/concerns/order_completion.rb index d3bd24a793..ca67d4a84d 100644 --- a/app/controllers/concerns/order_completion.rb +++ b/app/controllers/concerns/order_completion.rb @@ -50,9 +50,7 @@ module OrderCompletion end def order_invalid! - Alert.raise("Notice: invalid order loaded during checkout") do |payload| - payload.add_metadata :order, :order, @order - end + Alert.raise_with_record("Notice: invalid order loaded during checkout", @order) flash[:error] = t('checkout.order_not_loaded') redirect_to main_app.shop_path @@ -92,9 +90,7 @@ module OrderCompletion end def notify_failure(error = RuntimeError.new(order_processing_error)) - Alert.raise(error) do |payload| - payload.add_metadata :order, @order - end + Alert.raise_with_record(error, @order) flash[:error] = order_processing_error if flash.blank? end diff --git a/app/controllers/concerns/order_stock_check.rb b/app/controllers/concerns/order_stock_check.rb index 4d2d79fffa..911bedb124 100644 --- a/app/controllers/concerns/order_stock_check.rb +++ b/app/controllers/concerns/order_stock_check.rb @@ -20,9 +20,7 @@ module OrderStockCheck def check_order_cycle_expiry return unless current_order_cycle&.closed? - Alert.raise("Notice: order cycle closed during checkout completion") do |payload| - payload.add_metadata :order, :order, current_order - end + Alert.raise_with_record("Notice: order cycle closed during checkout completion", current_order) current_order.empty! current_order.set_order_cycle! nil diff --git a/app/jobs/backorder_job.rb b/app/jobs/backorder_job.rb index 565e637305..a6276ee95f 100644 --- a/app/jobs/backorder_job.rb +++ b/app/jobs/backorder_job.rb @@ -19,9 +19,7 @@ class BackorderJob < ApplicationJob rescue StandardError => e # Errors here shouldn't affect the checkout. So let's report them # separately: - Alert.raise(e) do |payload| - payload.add_metadata(:order, :order, order) - end + Alert.raise_with_record(e, order) end def perform(order) diff --git a/app/jobs/stock_sync_job.rb b/app/jobs/stock_sync_job.rb index a2a2fe6dd3..781afd59a9 100644 --- a/app/jobs/stock_sync_job.rb +++ b/app/jobs/stock_sync_job.rb @@ -16,9 +16,7 @@ class StockSyncJob < ApplicationJob rescue StandardError => e # Errors here shouldn't affect the shopping. So let's report them # separately: - Alert.raise(e) do |payload| - payload.add_metadata(:order, :order, order) - end + Alert.raise_with_record(e, order) end def self.sync_linked_catalogs_now(order) @@ -29,9 +27,7 @@ class StockSyncJob < ApplicationJob rescue StandardError => e # Errors here shouldn't affect the shopping. So let's report them # separately: - Alert.raise(e) do |payload| - payload.add_metadata(:order, :order, order) - end + Alert.raise_with_record(e, order) end def self.catalog_ids(order) diff --git a/app/services/alert.rb b/app/services/alert.rb index d43eae9b27..d030554298 100644 --- a/app/services/alert.rb +++ b/app/services/alert.rb @@ -28,4 +28,11 @@ class Alert block.call(payload) end end + + def self.raise_with_record(error, record, &) + metadata = { + record.class.name => record&.attributes || { record_was_nil: true } + } + self.raise(error, metadata, &) + end end diff --git a/app/services/place_proxy_order.rb b/app/services/place_proxy_order.rb index 14ad693cd5..189d4dc0ec 100644 --- a/app/services/place_proxy_order.rb +++ b/app/services/place_proxy_order.rb @@ -24,9 +24,7 @@ class PlaceProxyOrder send_placement_email rescue StandardError => e summarizer.record_and_log_error(:processing, order, e.message) - Alert.raise(e) do |payload| - payload.add_metadata :order, :order, order - end + Alert.raise_with_record(e, order) end private diff --git a/spec/services/alert_spec.rb b/spec/services/alert_spec.rb index 35db21cbf1..3c0eb123d4 100644 --- a/spec/services/alert_spec.rb +++ b/spec/services/alert_spec.rb @@ -51,6 +51,23 @@ RSpec.describe Alert do end end + it "sends ActiveRecord objects" do + order = Spree::Order.new(number: "ABC123") + + expect_any_instance_of(Bugsnag::Report).to receive(:add_metadata).with( + "Spree::Order", hash_including("number" => "ABC123") + ) + + Alert.raise_with_record("Wrong order", order) + end + + it "notifies Bugsnag when ActiveRecord object is missing" do + expect_any_instance_of(Bugsnag::Report).to receive(:add_metadata).with( + "NilClass", { record_was_nil: true } + ) + Alert.raise_with_record("Wrong order", nil) + end + it "reaches the Bugsnag service for real", :vcr do # You need to have a valid Bugsnag API key to record this test. # And after recording, you need to check the Bugsnag account for the right