always_raise_on_error -> version_error_behavior

CHANGELOG.md

@@ -11,8 +11,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 ### Added
 
-- [#1422](https://github.com/paper-trail-gem/paper_trail/pull/1450) - Add `always_raise_on_error`
-  config option to consistently surface errors when creating `Version` records.
+- [#1422](https://github.com/paper-trail-gem/paper_trail/pull/1450) - Add `version_error_behavior` config
+  config option to control error handling when creating/updating/deleting `Version` records.
 
 ### Fixed
 

README.md

@@ -289,7 +289,7 @@ Global configuration options affect all threads.
 - object_changes_adapter
 - serializer
 - version_limit
-- always_raise_on_error
+- version_error_behavior
 
 Syntax example: (options described in detail later)
 

lib/paper_trail/config.rb

@@ -15,7 +15,7 @@ class Config
       :serializer,
       :version_limit,
       :has_paper_trail_defaults,
-      :always_raise_on_error
+      :version_error_behavior
     )
 
     def initialize
@@ -26,6 +26,7 @@ def initialize
       # Variables which affect all threads, whose access is *not* synchronized.
       @serializer = PaperTrail::Serializers::YAML
       @has_paper_trail_defaults = {}
+      @version_error_behavior = :legacy
     end
 
     # Indicates whether PaperTrail is on or off. Default: true.

lib/paper_trail/record_trail.rb

@@ -64,6 +64,8 @@ def record_create
         # of versions_assoc.build?, the association cache is unaware. So, we
         # invalidate the `versions` association cache with `reset`.
         versions.reset
+      rescue StandardError => e
+        handle_version_errors e, version, :create
       end
     end
 
@@ -291,10 +293,20 @@ def log_version_errors(version, action)
     # Centralized handler for version errors
     # @api private
     def handle_version_errors(e, version, action)
-      if PaperTrail.config.always_raise_on_error
-        raise e
-      else
+      case PaperTrail.config.version_error_behavior
+      when :legacy
+        # legacy behavior was to raise on create and log on update/delete
+        if action == :create
+          raise e
+        else
+          log_version_errors(version, action)
+        end
+      when :log
         log_version_errors(version, action)
+      when :exception
+        raise e
+      when :silent
+        # noop
       end
     end
 

spec/paper_trail/config_spec.rb

@@ -17,89 +17,222 @@ module PaperTrail
       end
     end
 
-    describe ".always_raise_on_error", versioning: true do
-      context "when true" do
+    describe ".version_error_behavior", versioning: true do
+      let(:logger) { instance_double(Logger) }
+
+      before do
+        allow(logger).to receive(:warn)
+        allow(logger).to receive(:debug?).and_return(false)
+
+        ActiveRecord::Base.logger = logger
+      end
+
+      after do
+        ActiveRecord::Base.logger = nil
+      end
+
+      context "when :legacy" do
         context "when version cannot be created" do
-          before { PaperTrail.config.always_raise_on_error = true }
+          before { PaperTrail.config.version_error_behavior = :legacy }
 
-          after { PaperTrail.config.always_raise_on_error = false }
+          it "raises an error but does not log on create" do
+            expect {
+              Comment.create!(content: "Henry")
+            }.to raise_error(ActiveRecord::RecordInvalid)
+
+            expect(logger).not_to have_received(:warn)
+          end
+
+          it "logs but does not raise error on update" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
 
-          it "raises an error on create" do
+            expect {
+              comment.update!(content: "Brad")
+            }.not_to raise_error
+
+            expect(logger).to have_received(:warn)
+          end
+
+          it "does not raise error or log on update_columns" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.update_columns(content: "Brad")
+            }.not_to raise_error
+
+            expect(logger).not_to have_received(:warn)
+          end
+
+          it "logs but does not raise error on destroy" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.destroy!
+            }.not_to raise_error
+
+            expect(logger).to have_received(:warn)
+          end
+        end
+      end
+
+      context "when exception" do
+        context "when version cannot be created" do
+          before { PaperTrail.config.version_error_behavior = :exception }
+
+          after { PaperTrail.config.version_error_behavior = :legacy }
+
+          it "raises an error but does not log on create" do
             expect {
               Comment.create!(content: "Henry")
             }.to raise_error(ActiveRecord::RecordInvalid)
+
+            expect(logger).not_to have_received(:warn)
           end
 
-          it "raises an error on update" do
+          it "raises an error but does not on update" do
             comment = PaperTrail.request(whodunnit: "Foo") do
               Comment.create!(content: "Henry")
             end
 
             expect {
               comment.update!(content: "Brad")
             }.to raise_error(ActiveRecord::RecordInvalid)
+
+            expect(logger).not_to have_received(:warn)
           end
 
-          it "raises an error on update_columns" do
+          it "does not raise an error or log on update_columns" do
             comment = PaperTrail.request(whodunnit: "Foo") do
               Comment.create!(content: "Henry")
             end
 
             expect {
               comment.update_columns(content: "Brad")
             }.not_to raise_error
+
+            expect(logger).not_to have_received(:warn)
           end
 
-          it "raises an error on destroy" do
+          it "raises an error but does not log on destroy" do
             comment = PaperTrail.request(whodunnit: "Foo") do
               Comment.create!(content: "Henry")
             end
 
             expect {
               comment.destroy!
             }.to raise_error(ActiveRecord::RecordInvalid)
+
+            expect(logger).not_to have_received(:warn)
           end
         end
       end
 
-      context "when false" do
+      context "when log" do
         context "when version cannot be created" do
-          before { PaperTrail.config.always_raise_on_error = false }
+          before { PaperTrail.config.version_error_behavior = :log }
+
+          after { PaperTrail.config.version_error_behavior = :legacy }
 
-          it "raises an error on create" do
+          it "logs and does not raise an error on create" do
             expect {
               Comment.create!(content: "Henry")
-            }.to raise_error(ActiveRecord::RecordInvalid)
+            }.not_to raise_error
+
+            expect(logger).to have_received(:warn)
           end
 
-          it "does not raise an error on update" do
+          it "logs and does not raise an error on update" do
             comment = PaperTrail.request(whodunnit: "Foo") do
               Comment.create!(content: "Henry")
             end
 
             expect {
               comment.update!(content: "Brad")
             }.not_to raise_error
+
+            expect(logger).to have_received(:warn)
           end
 
-          it "does not raise an error on update_columns" do
+          it "does not raise an error or log on update_columns" do
             comment = PaperTrail.request(whodunnit: "Foo") do
               Comment.create!(content: "Henry")
             end
 
             expect {
               comment.update_columns(content: "Brad")
             }.not_to raise_error
+
+            expect(logger).not_to have_received(:warn)
           end
 
-          it "does not raises an error on destroy" do
+          it "logs and does not raise an error on destroy" do
             comment = PaperTrail.request(whodunnit: "Foo") do
               Comment.create!(content: "Henry")
             end
 
             expect {
               comment.destroy!
             }.not_to raise_error
+
+            expect(logger).to have_received(:warn)
+          end
+        end
+      end
+
+      context "when silent" do
+        context "when version cannot be created" do
+          before { PaperTrail.config.version_error_behavior = :silent }
+
+          after { PaperTrail.config.version_error_behavior = :legacy }
+
+          it "does not log or raise an error on create" do
+            expect {
+              Comment.create!(content: "Henry")
+            }.not_to raise_error
+
+            expect(logger).not_to have_received(:warn)
+          end
+
+          it "does not log or raise an error on update" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.update!(content: "Brad")
+            }.not_to raise_error
+
+            expect(logger).not_to have_received(:warn)
+          end
+
+          it "does not log or raise an error on update_columns" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.update_columns(content: "Brad")
+            }.not_to raise_error
+
+            expect(logger).not_to have_received(:warn)
+          end
+
+          it "does not log or raise an error on destroy" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.destroy!
+            }.not_to raise_error
+
+            expect(logger).not_to have_received(:warn)
           end
         end
       end