Don't be lenient in PluginService#processModule(Module)

We today catch the exception and move on - this is no good, we should just fail
all the way if something can't be loaded or processed

core/src/main/java/org/elasticsearch/plugins/PluginsService.java

@@ -40,6 +40,7 @@ import org.elasticsearch.common.settings.Settings;
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.URL;
 import java.net.URLClassLoader;
@@ -192,8 +193,12 @@ public class PluginsService extends AbstractComponent {
                     if (reference.moduleClass.isAssignableFrom(module.getClass())) {
                         try {
                             reference.onModuleMethod.invoke(plugin.v2(), module);
+                        } catch (IllegalAccessException | InvocationTargetException e) {
+                            logger.warn("plugin {}, failed to invoke custom onModule method", e, plugin.v2().name());
+                            throw new ElasticsearchException("failed to invoke onModule", e);
                         } catch (Exception e) {
                             logger.warn("plugin {}, failed to invoke custom onModule method", e, plugin.v2().name());
+                            throw e;
                         }
                     }
                 }

core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java

@@ -19,6 +19,8 @@
 
 package org.elasticsearch.plugins;
 
+import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.common.inject.AbstractModule;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.index.IndexModule;
@@ -56,6 +58,28 @@ public class PluginsServiceTests extends ESTestCase {
         }
     }
 
+    public static class FailOnModule extends Plugin {
+        @Override
+        public String name() {
+            return "fail-on-module";
+        }
+        @Override
+        public String description() {
+            return "fails in onModule";
+        }
+
+        public void onModule(BrokenModule brokenModule) {
+            throw new IllegalStateException("boom");
+        }
+    }
+
+    public static class BrokenModule extends AbstractModule {
+
+        @Override
+        protected void configure() {
+        }
+    }
+
     static PluginsService newPluginsService(Settings settings, Class<? extends Plugin>... classpathPlugins) {
         return new PluginsService(settings, new Environment(settings).pluginsFile(), Arrays.asList(classpathPlugins));
     }
@@ -86,4 +110,17 @@ public class PluginsServiceTests extends ESTestCase {
             assertTrue(msg, msg.contains("plugin [additional-settings2]"));
         }
     }
+
+    public void testOnModuleExceptionsArePropergated() {
+        Settings settings = Settings.builder()
+                .put("path.home", createTempDir()).build();
+        PluginsService service = newPluginsService(settings, FailOnModule.class);
+        try {
+            service.processModule(new BrokenModule());
+            fail("boom");
+        } catch (ElasticsearchException ex) {
+            assertEquals("failed to invoke onModule", ex.getMessage());
+            assertEquals("boom", ex.getCause().getCause().getMessage());
+        }
+    }
 }