diff --git a/app/controllers/admin/connected_apps_controller.rb b/app/controllers/admin/connected_apps_controller.rb index a5308d4984..a89743079e 100644 --- a/app/controllers/admin/connected_apps_controller.rb +++ b/app/controllers/admin/connected_apps_controller.rb @@ -77,7 +77,7 @@ module Admin def log_and_notify_exception(exception) Rails.logger.error exception.inspect - Bugsnag.notify(exception) + Alert.raise(exception) end def vine_params_empty? diff --git a/app/controllers/api/v0/base_controller.rb b/app/controllers/api/v0/base_controller.rb index 40bc17a06a..f3e44a29b2 100644 --- a/app/controllers/api/v0/base_controller.rb +++ b/app/controllers/api/v0/base_controller.rb @@ -66,7 +66,7 @@ module Api end def error_during_processing(exception) - Bugsnag.notify(exception) + Alert.raise(exception) render(json: { exception: exception.message }, status: :unprocessable_entity) && return diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index 2cf59888fb..f1e5e992fb 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -52,7 +52,7 @@ module Api end def error_during_processing(exception) - Bugsnag.notify(exception) + Alert.raise(exception) if Rails.env.development? || Rails.env.test? render status: :unprocessable_entity, diff --git a/app/controllers/concerns/order_completion.rb b/app/controllers/concerns/order_completion.rb index d98faf149b..d3bd24a793 100644 --- a/app/controllers/concerns/order_completion.rb +++ b/app/controllers/concerns/order_completion.rb @@ -50,7 +50,7 @@ module OrderCompletion end def order_invalid! - Bugsnag.notify("Notice: invalid order loaded during checkout") do |payload| + Alert.raise("Notice: invalid order loaded during checkout") do |payload| payload.add_metadata :order, :order, @order end @@ -92,7 +92,7 @@ module OrderCompletion end def notify_failure(error = RuntimeError.new(order_processing_error)) - Bugsnag.notify(error) do |payload| + Alert.raise(error) do |payload| payload.add_metadata :order, @order end flash[:error] = order_processing_error if flash.blank? diff --git a/app/controllers/concerns/order_stock_check.rb b/app/controllers/concerns/order_stock_check.rb index 7b94876533..4d2d79fffa 100644 --- a/app/controllers/concerns/order_stock_check.rb +++ b/app/controllers/concerns/order_stock_check.rb @@ -20,7 +20,7 @@ module OrderStockCheck def check_order_cycle_expiry return unless current_order_cycle&.closed? - Bugsnag.notify("Notice: order cycle closed during checkout completion") do |payload| + Alert.raise("Notice: order cycle closed during checkout completion") do |payload| payload.add_metadata :order, :order, current_order end current_order.empty! diff --git a/app/controllers/spree/admin/payments_controller.rb b/app/controllers/spree/admin/payments_controller.rb index 0afaa5227c..f2709a3f85 100644 --- a/app/controllers/spree/admin/payments_controller.rb +++ b/app/controllers/spree/admin/payments_controller.rb @@ -60,7 +60,7 @@ module Spree end rescue StandardError => e logger.error e.message - Bugsnag.notify(e) + Alert.raise(e) flash[:error] = e.message ensure redirect_to request.referer diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 788d019dce..e00fce21c0 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -213,7 +213,7 @@ module Spree end def notify_bugsnag(error, product, variant) - Bugsnag.notify(error) do |report| + Alert.raise(error) do |report| report.add_metadata(:product, { product: product.attributes, variant: variant.attributes }) report.add_metadata(:product, :product_error, product.errors.first) unless product.valid? diff --git a/app/jobs/backorder_job.rb b/app/jobs/backorder_job.rb index 73e3f51eba..565e637305 100644 --- a/app/jobs/backorder_job.rb +++ b/app/jobs/backorder_job.rb @@ -19,7 +19,7 @@ class BackorderJob < ApplicationJob rescue StandardError => e # Errors here shouldn't affect the checkout. So let's report them # separately: - Bugsnag.notify(e) do |payload| + Alert.raise(e) do |payload| payload.add_metadata(:order, :order, order) end end diff --git a/app/jobs/report_job.rb b/app/jobs/report_job.rb index 0367ddd259..e99eccae00 100644 --- a/app/jobs/report_job.rb +++ b/app/jobs/report_job.rb @@ -22,7 +22,7 @@ class ReportJob < ApplicationJob broadcast_result(channel, format, blob) if channel rescue StandardError => e - Bugsnag.notify(e) do |payload| + Alert.raise(e) do |payload| payload.add_metadata :report, { report_class:, user:, params:, format: } diff --git a/app/jobs/stock_sync_job.rb b/app/jobs/stock_sync_job.rb index 009bdb5b5e..a2a2fe6dd3 100644 --- a/app/jobs/stock_sync_job.rb +++ b/app/jobs/stock_sync_job.rb @@ -16,7 +16,7 @@ class StockSyncJob < ApplicationJob rescue StandardError => e # Errors here shouldn't affect the shopping. So let's report them # separately: - Bugsnag.notify(e) do |payload| + Alert.raise(e) do |payload| payload.add_metadata(:order, :order, order) end end @@ -29,7 +29,7 @@ class StockSyncJob < ApplicationJob rescue StandardError => e # Errors here shouldn't affect the shopping. So let's report them # separately: - Bugsnag.notify(e) do |payload| + Alert.raise(e) do |payload| payload.add_metadata(:order, :order, order) end end diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb index 2182671049..da5d388db9 100644 --- a/app/jobs/subscription_confirm_job.rb +++ b/app/jobs/subscription_confirm_job.rb @@ -55,7 +55,7 @@ class SubscriptionConfirmJob < ApplicationJob if order.errors.any? send_failed_payment_email(order) else - Bugsnag.notify(e) do |payload| + Alert.raise(e) do |payload| payload.add_metadata :order, :order, order end send_failed_payment_email(order, e.message) @@ -108,7 +108,7 @@ class SubscriptionConfirmJob < ApplicationJob record_and_log_error(:failed_payment, order, error_message) SubscriptionMailer.failed_payment_email(order).deliver_now rescue StandardError => e - Bugsnag.notify(e) do |payload| + Alert.raise(e) do |payload| payload.add_metadata :subscription_data, { order:, error_message: } end end diff --git a/app/models/calculator/default_tax.rb b/app/models/calculator/default_tax.rb index b6c8b1d3c2..c1cb7a4bcd 100644 --- a/app/models/calculator/default_tax.rb +++ b/app/models/calculator/default_tax.rb @@ -28,7 +28,7 @@ module Calculator # In theory it should never be called any more after this has been deployed. # If the message below doesn't show up in Bugsnag, we can safely delete this method and all # the related methods below it. - Bugsnag.notify("Calculator::DefaultTax was called with legacy tax calculations") + Alert.raise("Calculator::DefaultTax was called with legacy tax calculations") calculator = OpenFoodNetwork::EnterpriseFeeCalculator.new(order.distributor, order.order_cycle) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index dfee85fef6..15d7fb33f2 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -481,7 +481,7 @@ class Enterprise < ApplicationRecord image_variant_url_for(image.variant(name)) rescue StandardError => e - Bugsnag.notify "Enterprise ##{id} #{image.try(:name)} error: #{e.message}" + Alert.raise "Enterprise ##{id} #{image.try(:name)} error: #{e.message}" Rails.logger.error(e.message) nil diff --git a/app/models/spree/image.rb b/app/models/spree/image.rb index 45859a6b45..350b1c5b4c 100644 --- a/app/models/spree/image.rb +++ b/app/models/spree/image.rb @@ -34,7 +34,7 @@ module Spree image_variant_url_for(variant(size)) rescue StandardError => e - Bugsnag.notify "Product ##{viewable_id} Image ##{id} error: #{e.message}" + Alert.raise "Product ##{viewable_id} Image ##{id} error: #{e.message}" Rails.logger.error(e.message) self.class.default_image_url(size) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 1e167f8e92..d5c3a00579 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -531,7 +531,7 @@ module Spree # because an outdated shipping fee is not as bad as a lost payment. # And the shipping fee is already up-to-date when this error occurs. # https://github.com/openfoodfoundation/openfoodnetwork/issues/3924 - Bugsnag.notify(e) do |report| + Alert.raise(e) do |report| report.add_metadata(:order, attributes) report.add_metadata(:shipment, shipment.attributes) report.add_metadata(:shipment_in_db, Spree::Shipment.find_by(id: shipment.id).attributes) diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb index 588fe91985..239a6d9072 100644 --- a/app/models/spree/tax_rate.rb +++ b/app/models/spree/tax_rate.rb @@ -106,7 +106,7 @@ module Spree calculator.compute(item) else # Tax refund should not be possible with the way our production server are configured - Bugsnag.notify( + Alert.raise( "Notice: Tax refund should not be possible, please check the default zone and " \ "the tax rate zone configuration" ) do |payload| diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index fefd41ac0d..26f22cdddf 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -293,7 +293,7 @@ module Spree end def ensure_unit_value - Bugsnag.notify("Trying to set unit_value to NaN") if unit_value&.nan? + Alert.raise("Trying to set unit_value to NaN") if unit_value&.nan? return unless (variant_unit == "items" && unit_value.nil?) || unit_value&.nan? self.unit_value = 1.0 diff --git a/app/models/stripe_account.rb b/app/models/stripe_account.rb index c0b507f316..260a29270e 100644 --- a/app/models/stripe_account.rb +++ b/app/models/stripe_account.rb @@ -15,7 +15,7 @@ class StripeAccount < ApplicationRecord destroy && Stripe::OAuth.deauthorize(stripe_user_id:) rescue Stripe::OAuth::OAuthError => e - Bugsnag.notify( + Alert.raise( e, stripe_account: stripe_user_id, enterprise_id: diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb index 79a28508f2..34bd6ac984 100644 --- a/app/models/variant_override.rb +++ b/app/models/variant_override.rb @@ -48,8 +48,8 @@ class VariantOverride < ApplicationRecord def move_stock!(quantity) unless stock_overridden? - Bugsnag.notify RuntimeError.new "Attempting to move stock of a VariantOverride " \ - "without a count_on_hand specified." + Alert.raise RuntimeError.new "Attempting to move stock of a VariantOverride " \ + "without a count_on_hand specified." return end @@ -73,8 +73,8 @@ class VariantOverride < ApplicationRecord self.attributes = { on_demand: false, count_on_hand: default_stock } save else - Bugsnag.notify RuntimeError.new "Attempting to reset stock level for a variant " \ - "with no default stock level." + Alert.raise RuntimeError.new "Attempting to reset stock level for a variant " \ + "with no default stock level." end end self diff --git a/app/services/place_proxy_order.rb b/app/services/place_proxy_order.rb index c6f942d3b0..14ad693cd5 100644 --- a/app/services/place_proxy_order.rb +++ b/app/services/place_proxy_order.rb @@ -24,7 +24,7 @@ class PlaceProxyOrder send_placement_email rescue StandardError => e summarizer.record_and_log_error(:processing, order, e.message) - Bugsnag.notify(e) do |payload| + Alert.raise(e) do |payload| payload.add_metadata :order, :order, order end end @@ -56,7 +56,7 @@ class PlaceProxyOrder true rescue StandardError => e - Bugsnag.notify(e) do |payload| + Alert.raise(e) do |payload| payload.add_metadata(:proxy_order, { subscription:, proxy_order: }) end false diff --git a/app/services/sets/product_set.rb b/app/services/sets/product_set.rb index c6a4521ab7..156f73f875 100644 --- a/app/services/sets/product_set.rb +++ b/app/services/sets/product_set.rb @@ -145,7 +145,7 @@ module Sets end def notify_bugsnag(error, product, variant, variant_attributes) - Bugsnag.notify(error) do |report| + Alert.raise(error) do |report| report.add_metadata( :product_set, { product: product.attributes, variant_attributes:, variant: variant.attributes } ) diff --git a/lib/open_food_network/error_logger.rb b/lib/open_food_network/error_logger.rb index 51ea1e715e..f028fde39b 100644 --- a/lib/open_food_network/error_logger.rb +++ b/lib/open_food_network/error_logger.rb @@ -9,7 +9,7 @@ module OpenFoodNetwork # If Bugsnag is configured, it will notify it. It would be nice to implement # some kind of fallback. def self.notify(error) - Bugsnag.notify(error) + Alert.raise(error) end end end diff --git a/spec/services/alert_spec.rb b/spec/services/alert_spec.rb index c38b0034ff..35db21cbf1 100644 --- a/spec/services/alert_spec.rb +++ b/spec/services/alert_spec.rb @@ -3,6 +3,24 @@ require 'spec_helper' RSpec.describe Alert do + around do |example| + original_config = nil + Bugsnag.configure do |config| + original_config = config.dup + config.api_key ||= "dummy-key" + config.notify_release_stages = ["test"] + config.delivery_method = :synchronous + end + + example.run + + Bugsnag.configure do |config| + config.api_key ||= original_config.api_key + config.notify_release_stages = original_config.notify_release_stages + config.delivery_method = original_config.delivery_method + end + end + it "notifies Bugsnag" do expect(Bugsnag).to receive(:notify).with("hey") @@ -23,26 +41,23 @@ RSpec.describe Alert do ) end + it "is compatible with Bugsnag API" do + expect_any_instance_of(Bugsnag::Report).to receive(:add_metadata).with( + :order, { number: "ABC123" } + ) + + Alert.raise("hey") do |payload| + payload.add_metadata(:order, { number: "ABC123" }) + end + 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 # data. - - original_config = nil - Bugsnag.configure do |config| - original_config = config.dup - config.notify_release_stages = ["test"] - config.delivery_method = :synchronous - end - Alert.raise( "Testing Bugsnag from RSpec", { RSpec: { file: __FILE__ }, env: { BUGSNAG: ENV.fetch("BUGSNAG", nil) } } ) - - Bugsnag.configure do |config| - config.notify_release_stages = original_config.notify_release_stages - config.delivery_method = original_config.delivery_method - end end end