[MODEL] Prevent index methods to swallow all exceptions

Closes elastic#303

Author: karmi

elasticsearch-model/lib/elasticsearch/model/indexing.rb

@@ -187,17 +187,10 @@ def create_index!(options={})
           delete_index!(options.merge index: target_index) if options[:force]
 
           unless ( self.client.indices.exists(index: target_index) rescue false )
-            begin
-              self.client.indices.create index: target_index,
-                                         body: {
-                                           settings: self.settings.to_hash,
-                                           mappings: self.mappings.to_hash }
-            rescue Exception => e
-              unless e.class.to_s =~ /NotFound/ && options[:force]
-                STDERR.puts "[!!!] Error when creating the index: #{e.class}", "#{e.message}"
-              end
-            end
-          else
+            self.client.indices.create index: target_index,
+                                       body: {
+                                         settings: self.settings.to_hash,
+                                         mappings: self.mappings.to_hash }
           end
         end
 
@@ -217,8 +210,10 @@ def delete_index!(options={})
           begin
             self.client.indices.delete index: target_index
           rescue Exception => e
-            unless e.class.to_s =~ /NotFound/ && options[:force]
-              STDERR.puts "[!!!] Error when deleting the index: #{e.class}", "#{e.message}"
+            if e.class.to_s =~ /NotFound/ && options[:force]
+              STDERR.puts "[!!!] Index does not exist (#{e.class})"
+            else
+              raise e
             end
           end
         end
@@ -241,8 +236,10 @@ def refresh_index!(options={})
           begin
             self.client.indices.refresh index: target_index
           rescue Exception => e
-            unless e.class.to_s =~ /NotFound/ && options[:force]
-              STDERR.puts "[!!!] Error when refreshing the index: #{e.class}", "#{e.message}"
+            if e.class.to_s =~ /NotFound/ && options[:force]
+              STDERR.puts "[!!!] Index does not exist (#{e.class})"
+            else
+              raise e
             end
           end
         end

elasticsearch-model/test/unit/indexing_test.rb

@@ -12,6 +12,8 @@ def self.foo
       end
     end
 
+    class NotFound < Exception; end
+
     context "Settings class" do
       should "be convertible to hash" do
         hash     = { foo: 'bar' }
@@ -382,19 +384,40 @@ class ::DummyIndexingModelForRecreate
         end
       end
 
-      should "delete the index without raising exception" do
+      should "delete the index without raising exception when the index is not found" do
         client  = stub('client')
         indices = stub('indices')
         client.stubs(:indices).returns(indices)
 
-        indices.expects(:delete).returns({}).then.raises(Exception).at_least_once
+        indices.expects(:delete).returns({}).then.raises(NotFound).at_least_once
 
         DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
 
-        assert_nothing_raised do
-          DummyIndexingModelForRecreate.delete_index!
-          DummyIndexingModelForRecreate.delete_index!
-        end
+        assert_nothing_raised { DummyIndexingModelForRecreate.delete_index! force: true }
+      end
+
+      should "raise an exception without the force option" do
+        client  = stub('client')
+        indices = stub('indices')
+        client.stubs(:indices).returns(indices)
+
+        indices.expects(:delete).raises(NotFound)
+
+        DummyIndexingModelForRecreate.expects(:client).returns(client)
+
+        assert_raise { DummyIndexingModelForRecreate.delete_index! }
+      end
+
+      should "raise a regular exception when deleting the index" do
+        client  = stub('client')
+        indices = stub('indices')
+        client.stubs(:indices).returns(indices)
+
+        indices.expects(:delete).raises(Exception)
+
+        DummyIndexingModelForRecreate.expects(:client).returns(client)
+
+        assert_raise { DummyIndexingModelForRecreate.delete_index! force: true }
       end
 
       should "create the index with correct settings and mappings when it doesn't exist" do
@@ -430,19 +453,18 @@ class ::DummyIndexingModelForRecreate
         assert_nothing_raised { DummyIndexingModelForRecreate.create_index! }
       end
 
-      should "not raise exception during index creation" do
+      should "raise exception during index creation" do
         client  = stub('client')
         indices = stub('indices')
         client.stubs(:indices).returns(indices)
 
+        indices.expects(:delete).returns({})
         indices.expects(:exists).returns(false)
-        indices.expects(:create).raises(Exception).at_least_once
+        indices.expects(:create).raises(Exception)
 
         DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
 
-        assert_nothing_raised do
-          DummyIndexingModelForRecreate.create_index!
-        end
+        assert_raise { DummyIndexingModelForRecreate.create_index! force: true }
       end
 
       should "delete the index first with the force option" do
@@ -461,7 +483,19 @@ class ::DummyIndexingModelForRecreate
         end
       end
 
-      should "refresh the index without raising exception" do
+      should "refresh the index without raising exception with the force option" do
+        client  = stub('client')
+        indices = stub('indices')
+        client.stubs(:indices).returns(indices)
+
+        indices.expects(:refresh).returns({}).then.raises(NotFound).at_least_once
+
+        DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
+
+        assert_nothing_raised { DummyIndexingModelForRecreate.refresh_index! force: true }
+      end
+
+      should "raise a regular exception when refreshing the index" do
         client  = stub('client')
         indices = stub('indices')
         client.stubs(:indices).returns(indices)
@@ -470,10 +504,7 @@ class ::DummyIndexingModelForRecreate
 
         DummyIndexingModelForRecreate.expects(:client).returns(client).at_least_once
 
-        assert_nothing_raised do
-          DummyIndexingModelForRecreate.refresh_index!
-          DummyIndexingModelForRecreate.refresh_index!
-        end
+        assert_nothing_raised { DummyIndexingModelForRecreate.refresh_index! force: true }
       end
 
       context "with a custom index name" do