From 9dcc683dc03428a03c68cc63fd6c9994c10c77f7 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 17 Sep 2018 10:50:24 +1000 Subject: [PATCH] Notify Bugsnag on sign-up errors This may lead to more error reports than we want to see. A not existing email address may cause Bugsnag to be notified. If this happens, we can rescue form these specific errors and only report the rest. --- app/controllers/user_registrations_controller.rb | 5 ++++- lib/open_food_network/error_logger.rb | 13 +++++++++++++ .../user_registrations_controller_spec.rb | 1 + spec/lib/open_food_network/error_logger_spec.rb | 14 ++++++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 lib/open_food_network/error_logger.rb create mode 100644 spec/lib/open_food_network/error_logger_spec.rb diff --git a/app/controllers/user_registrations_controller.rb b/app/controllers/user_registrations_controller.rb index 4827a9a0b0..654fe1791e 100644 --- a/app/controllers/user_registrations_controller.rb +++ b/app/controllers/user_registrations_controller.rb @@ -1,3 +1,5 @@ +require 'open_food_network/error_logger' + class UserRegistrationsController < Spree::UserRegistrationsController before_filter :set_checkout_redirect, only: :create @@ -21,7 +23,8 @@ class UserRegistrationsController < Spree::UserRegistrationsController else render_error(@user.errors) end - rescue StandardError + rescue StandardError => error + OpenFoodNetwork::ErrorLogger.notify(error) render_error(message: I18n.t('devise.user_registrations.spree_user.unknown_error')) end diff --git a/lib/open_food_network/error_logger.rb b/lib/open_food_network/error_logger.rb new file mode 100644 index 0000000000..a1199c9e68 --- /dev/null +++ b/lib/open_food_network/error_logger.rb @@ -0,0 +1,13 @@ +# Our error logging API currently wraps Bugsnag. +# It makes us more flexible if we wanted to replace Bugsnag or change logging +# behaviour. +module OpenFoodNetwork + module ErrorLogger + # Tries to escalate the error to a developer. + # 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) + end + end +end diff --git a/spec/controllers/user_registrations_controller_spec.rb b/spec/controllers/user_registrations_controller_spec.rb index 505a1846e2..cb9d278774 100644 --- a/spec/controllers/user_registrations_controller_spec.rb +++ b/spec/controllers/user_registrations_controller_spec.rb @@ -31,6 +31,7 @@ describe UserRegistrationsController, type: :controller do it "returns error when emailing fails" do allow(Spree::UserMailer).to receive(:confirmation_instructions).and_raise("Some error") + expect(OpenFoodNetwork::ErrorLogger).to receive(:notify) xhr :post, :create, spree_user: user_params, use_route: :spree diff --git a/spec/lib/open_food_network/error_logger_spec.rb b/spec/lib/open_food_network/error_logger_spec.rb new file mode 100644 index 0000000000..f1cd326fa2 --- /dev/null +++ b/spec/lib/open_food_network/error_logger_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' +require 'open_food_network/error_logger' + +module OpenFoodNetwork + describe ErrorLogger do + let(:error) { StandardError.new("Test") } + + it "notifies Bugsnag" do + expect(Bugsnag).to receive(:notify).with(error) + + ErrorLogger.notify(error) + end + end +end