Add Spree::Api::ErrorHandler

api/app/controllers/spree/api/v2/base_controller.rb

@@ -10,15 +10,15 @@ class BaseController < ActionController::API
         rescue_from ActiveRecord::RecordNotFound, with: :record_not_found
         rescue_from CanCan::AccessDenied, with: :access_denied
         rescue_from Spree::Core::GatewayError, with: :gateway_error
+        rescue_from ActionController::ParameterMissing, with: :error_during_processing
+        rescue_from ArgumentError, with: :error_during_processing
 
         def content_type
           Spree::Api::Config[:api_v2_content_type]
         end
 
         def render_serialized_payload(status = 200)
           render json: yield, status: status, content_type: content_type
-        rescue ArgumentError => exception
-          render_error_payload(exception.message, 400)
         end
 
         def render_error_payload(error, status = 422)
@@ -99,6 +99,16 @@ def access_denied(exception)
         def gateway_error(exception)
           render_error_payload(exception.message)
         end
+
+        def error_during_processing(exception)
+          result = error_handler.call(exception: exception, opts: { user: spree_current_user })
+
+          render_error_payload(result.value[:message], 400)
+        end
+
+        def error_handler
+          Spree::Api::Dependencies.error_handler.constantize
+        end
       end
     end
   end

api/app/models/spree/api_dependencies.rb

@@ -16,7 +16,7 @@ class ApiDependencies
       :storefront_cart_update_service, :storefront_cart_estimate_shipping_rates_service, :storefront_estimated_shipment_serializer,
       :storefront_store_serializer, :storefront_address_serializer, :storefront_order_serializer,
       :storefront_account_create_address_service, :storefront_account_update_address_service, :storefront_address_finder,
-      :storefront_collection_sorter
+      :storefront_collection_sorter, :error_handler
     ].freeze
 
     attr_accessor *INJECTION_POINTS
@@ -86,6 +86,8 @@ def set_storefront_defaults
       @storefront_find_by_variant_finder = Spree::Dependencies.line_item_by_variant_finder
       @storefront_products_finder = Spree::Dependencies.products_finder
       @storefront_taxon_finder = Spree::Dependencies.taxon_finder
+
+      @error_handler = 'Spree::Api::ErrorHandler'
     end
   end
 end

api/app/services/spree/api/error_handler.rb

@@ -0,0 +1,40 @@
+module Spree
+  module Api
+    class ErrorHandler
+      prepend ::Spree::ServiceModule::Base
+
+      def call(exception:, opts: {})
+        run :format_message
+        run :log_error
+        run :report_error
+      end
+
+      protected
+
+      def format_message(exception:, opts:)
+        message = if exception.respond_to?(:original_message)
+                    exception.original_message
+                  else
+                    exception.message
+                  end
+
+        success(exception: exception, message: message, opts: opts)
+      end
+
+      def log_error(exception:, message:, opts:)
+        Rails.logger.error message
+        Rails.logger.error "User ID: #{opts[:user]&.id}" if opts[:user]
+        Rails.logger.error exception.backtrace.join("\n")
+
+        success(exception: exception, message: message, opts: opts)
+      end
+
+      def report_error(exception:, message:, opts:)
+        # overwrite this method in your application to support different error handlers
+        # eg. Sentry, Rollbar, etc
+
+        success(exception: exception, message: message)
+      end
+    end
+  end
+end

api/spec/controllers/spree/api/v2/base_controller_spec.rb

@@ -88,4 +88,37 @@ def default_resource_includes
       end
     end
   end
+
+  describe '#error_during_processing' do
+
+    controller(described_class) do
+      def index
+        render plain: { 'products' => [] }.to_json
+      end
+    end
+
+    before do
+      @routes = ActionDispatch::Routing::RouteSet.new.tap do |r|
+        r.draw { get 'index', to: 'spree/api/v2/base#index' }
+      end
+    end
+
+    let!(:user) { create :user }
+    let(:exception) { ArgumentError.new('foo') }
+    let(:result_class) { Struct.new(:value) }
+    let(:result) { result_class.new({message: 'foo'}) }
+
+    it 'handles ArgumentError exceptions' do
+      expect(subject).to receive(:index).and_raise(exception)
+      expect(subject).to receive(:spree_current_user).and_return(user)
+      expect_next_instance_of(::Spree::Api::ErrorHandler) do |instance|
+        expect(instance).to receive(:call).with(
+          exception: exception,
+          opts: { user: user }
+        ).and_return(result)
+      end
+      get :index, params: { token: 'exception-message' }
+      expect(json_response).to eql('error' => 'foo')
+    end
+  end
 end

api/spec/services/spree/api/error_handler_spec.rb

@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+module Spree
+  describe Api::ErrorHandler do
+    class CustomException < StandardError
+      def backtrace
+        ['a', 'b']
+      end
+    end
+
+    let(:message) { 'error' }
+    let(:exception) { CustomException.new(message) }
+    let(:user) { create :user }
+
+    subject { described_class.call(exception: exception, opts: { user: user }) }
+    let(:result) { subject }
+
+    it 'returns result with error' do
+      expect(Rails.logger).to receive(:error).with(message)
+      expect(Rails.logger).to receive(:error).with("User ID: #{user.id}")
+      expect(Rails.logger).to receive(:error).with(exception.backtrace.join("\n"))
+
+      expect(result.value).to eq({ exception: exception, message: message })
+    end
+  end
+end

api/spec/spec_helper.rb

@@ -35,13 +35,15 @@
 require 'spree/testing_support/factories'
 require 'spree/testing_support/preferences'
 require 'spree/testing_support/image_helpers'
+require 'spree/testing_support/next_instance_of'
 
 require 'spree/api/testing_support/caching'
 require 'spree/api/testing_support/helpers'
 require 'spree/api/testing_support/setup'
 require 'spree/api/testing_support/v2/base'
 require 'spree/api/testing_support/v2/current_order'
 
+
 RSpec.configure do |config|
   config.backtrace_exclusion_patterns = [/gems\/activesupport/, /gems\/actionpack/, /gems\/rspec/]
   config.color = true

core/app/models/spree/app_dependencies.rb

@@ -13,7 +13,7 @@ class AppDependencies
       :completed_order_finder, :order_sorter, :cart_compare_line_items_service, :collection_paginator, :products_sorter,
       :products_finder, :taxon_finder, :line_item_by_variant_finder, :cart_estimate_shipping_rates_service,
       :account_create_address_service, :account_update_address_service, :address_finder,
-      :collection_sorter
+      :collection_sorter, :error_handler
     ].freeze
 
     attr_accessor *INJECTION_POINTS
@@ -66,6 +66,8 @@ def set_default_services
       # account
       @account_create_address_service = 'Spree::Account::Addresses::Create'
       @account_update_address_service = 'Spree::Account::Addresses::Update'
+
+      @error_handler = 'Spree::ErrorReporter'
     end
 
     def set_default_finders

core/lib/spree/testing_support/next_instance_of.rb

@@ -0,0 +1,38 @@
+# Helper services with prepended ServiceModule
+# https://gitlab.com/gitlab-org/gitlab-foss/-/issues/33587
+#
+# ex:
+# expect_next_instance_of(service_class) do |instance|
+#   expect(instance).to receive(:call)
+# end
+
+module NextInstanceOf
+  def expect_next_instance_of(klass, *new_args)
+    stub_new(expect(klass), *new_args) do |expectation|
+      yield(expectation)
+    end
+  end
+
+  def allow_next_instance_of(klass, *new_args)
+    stub_new(allow(klass), *new_args) do |allowance|
+      yield(allowance)
+    end
+  end
+
+  private
+
+  def stub_new(target, *new_args)
+    receive_new = receive(:new)
+    receive_new.with(*new_args) if new_args.any?
+
+    target.to receive_new.and_wrap_original do |method, *original_args|
+      method.call(*original_args).tap do |instance|
+        yield(instance)
+      end
+    end
+  end
+end
+
+RSpec.configure do |config|
+  config.include NextInstanceOf
+end