BulkUpdateRequest#approve!: don't swallow exceptions.

Rescue `AliasAndImplicationImporter::Error` instead of `Exception`.

app/logical/alias_and_implication_importer.rb

@@ -1,4 +1,5 @@
 class AliasAndImplicationImporter
+  class Error < RuntimeError; end
   attr_accessor :text, :commands, :forum_id, :rename_aliased_pages, :skip_secondary_validations
 
   def initialize(text, forum_id, rename_aliased_pages = "0", skip_secondary_validations = true)
@@ -50,7 +51,7 @@ class AliasAndImplicationImporter
         # do nothing
 
       else
-        raise "Unparseable line: #{line}"
+        raise Error, "Unparseable line: #{line}"
       end
     end
   end
@@ -61,20 +62,20 @@ class AliasAndImplicationImporter
       when :create_alias
         tag_alias = TagAlias.new(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2], :skip_secondary_validations => skip_secondary_validations)
         unless tag_alias.valid?
-          raise "Error: #{tag_alias.errors.full_messages.join("; ")} (create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name})"
+          raise Error, "Error: #{tag_alias.errors.full_messages.join("; ")} (create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name})"
         end
 
       when :create_implication
         tag_implication = TagImplication.new(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2], :skip_secondary_validations => skip_secondary_validations)
         unless tag_implication.valid?
-          raise "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
+          raise Error, "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
         end
 
       when :remove_alias, :remove_implication, :mass_update, :change_category
         # okay
 
       else
-        raise "Unknown token: #{token[0]}"
+        raise Error, "Unknown token: #{token[0]}"
       end
     end
   end
@@ -88,7 +89,7 @@ private
         when :create_alias
           tag_alias = TagAlias.create(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2], :skip_secondary_validations => skip_secondary_validations)
           unless tag_alias.valid?
-            raise "Error: #{tag_alias.errors.full_messages.join("; ")} (create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name})"
+            raise Error, "Error: #{tag_alias.errors.full_messages.join("; ")} (create alias #{tag_alias.antecedent_name} -> #{tag_alias.consequent_name})"
           end
           tag_alias.rename_wiki_and_artist if rename_aliased_pages?
           tag_alias.approve!(approver: approver, update_topic: false)
@@ -96,18 +97,18 @@ private
         when :create_implication
           tag_implication = TagImplication.create(:forum_topic_id => forum_id, :status => "pending", :antecedent_name => token[1], :consequent_name => token[2], :skip_secondary_validations => skip_secondary_validations)
           unless tag_implication.valid?
-            raise "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
+            raise Error, "Error: #{tag_implication.errors.full_messages.join("; ")} (create implication #{tag_implication.antecedent_name} -> #{tag_implication.consequent_name})"
           end
           tag_implication.approve!(approver: approver, update_topic: false)
 
         when :remove_alias
           tag_alias = TagAlias.where("antecedent_name = ?", token[1]).first
-          raise "Alias for #{token[1]} not found" if tag_alias.nil?
+          raise Error, "Alias for #{token[1]} not found" if tag_alias.nil?
           tag_alias.destroy
 
         when :remove_implication
           tag_implication = TagImplication.where("antecedent_name = ? and consequent_name = ?", token[1], token[2]).first
-          raise "Implication for #{token[1]} not found" if tag_implication.nil?
+          raise Error, "Implication for #{token[1]} not found" if tag_implication.nil?
           tag_implication.destroy
 
         when :mass_update
@@ -119,7 +120,7 @@ private
           tag.save
 
         else
-          raise "Unknown token: #{token[0]}"
+          raise Error, "Unknown token: #{token[0]}"
         end
       end
     end

app/models/bulk_update_request.rb

@@ -96,7 +96,7 @@ class BulkUpdateRequest < ApplicationRecord
         forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has been approved by @#{approver.name}.", "APPROVED")
       end
 
-    rescue Exception => x
+    rescue AliasAndImplicationImporter::Error => x
       self.approver = approver
       CurrentUser.scoped(approver) do
         forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has failed: #{x.to_s}", "FAILED")

test/unit/bulk_update_request_test.rb

@@ -113,7 +113,7 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase
       end
 
       should "handle errors gracefully" do
-        @req.stubs(:update).raises(RuntimeError.new("blah"))
+        @req.stubs(:update).raises(AliasAndImplicationImporter::Error.new("blah"))
         assert_difference("ForumPost.count", 1) do
           @req.approve!(@admin)
         end