diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ce29955d12..bb867af945e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - k8s (KUBERNETES) Nat method is now deprecated and will be removed in a future release ### Breaking Changes +- Besu will now fail to start if any plugins encounter errors during initialization. To allow Besu to continue running despite plugin errors, use the `--plugin-continue-on-error` option. [#7662](https://github.com/hyperledger/besu/pull/7662) ### Additions and Improvements - Remove privacy test classes support [#7569](https://github.com/hyperledger/besu/pull/7569) diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java index 5d78f1460c4..90de0b0e952 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java @@ -503,8 +503,9 @@ public BesuPluginContextImpl providePluginContext( besuPluginContext.addService(PermissioningService.class, permissioningService); besuPluginContext.addService(PrivacyPluginService.class, new PrivacyPluginServiceImpl()); - besuPluginContext.registerPlugins( + besuPluginContext.initialize( new PluginConfiguration.Builder().pluginsDir(pluginsPath).build()); + besuPluginContext.registerPlugins(); commandLine.parseArgs(extraCLIOptions.toArray(new String[0])); // register built-in plugins diff --git a/acceptance-tests/test-plugins/src/main/java/org/hyperledger/besu/tests/acceptance/plugins/TestPicoCLIPlugin.java b/acceptance-tests/test-plugins/src/main/java/org/hyperledger/besu/tests/acceptance/plugins/TestPicoCLIPlugin.java index 0db5281537c..375fbd490ec 100644 --- a/acceptance-tests/test-plugins/src/main/java/org/hyperledger/besu/tests/acceptance/plugins/TestPicoCLIPlugin.java +++ b/acceptance-tests/test-plugins/src/main/java/org/hyperledger/besu/tests/acceptance/plugins/TestPicoCLIPlugin.java @@ -1,5 +1,5 @@ /* - * Copyright ConsenSys AG. + * Copyright contributors to Hyperledger Besu. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at @@ -32,16 +32,25 @@ public class TestPicoCLIPlugin implements BesuPlugin { private static final Logger LOG = LoggerFactory.getLogger(TestPicoCLIPlugin.class); + private static final String UNSET = "UNSET"; + private static final String FAIL_REGISTER = "FAILREGISTER"; + private static final String FAIL_BEFORE_EXTERNAL_SERVICES = "FAILBEFOREEXTERNALSERVICES"; + private static final String FAIL_START = "FAILSTART"; + private static final String FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP = + "FAILAFTEREXTERNALSERVICEPOSTMAINLOOP"; + private static final String FAIL_STOP = "FAILSTOP"; + private static final String PLUGIN_LIFECYCLE_PREFIX = "pluginLifecycle."; + @Option( names = {"--Xplugin-test-option"}, hidden = true, - defaultValue = "UNSET") + defaultValue = UNSET) String testOption = System.getProperty("testPicoCLIPlugin.testOption"); @Option( names = {"--plugin-test-stable-option"}, hidden = true, - defaultValue = "UNSET") + defaultValue = UNSET) String stableOption = ""; private String state = "uninited"; @@ -52,7 +61,7 @@ public void register(final BesuContext context) { LOG.info("Registering. Test Option is '{}'", testOption); state = "registering"; - if ("FAILREGISTER".equals(testOption)) { + if (FAIL_REGISTER.equals(testOption)) { state = "failregister"; throw new RuntimeException("I was told to fail at registration"); } @@ -66,12 +75,26 @@ public void register(final BesuContext context) { state = "registered"; } + @Override + public void beforeExternalServices() { + LOG.info("Before external services. Test Option is '{}'", testOption); + state = "beforeExternalServices"; + + if (FAIL_BEFORE_EXTERNAL_SERVICES.equals(testOption)) { + state = "failbeforeExternalServices"; + throw new RuntimeException("I was told to fail before external services"); + } + + writeSignal("beforeExternalServices"); + state = "beforeExternalServicesFinished"; + } + @Override public void start() { LOG.info("Starting. Test Option is '{}'", testOption); state = "starting"; - if ("FAILSTART".equals(testOption)) { + if (FAIL_START.equals(testOption)) { state = "failstart"; throw new RuntimeException("I was told to fail at startup"); } @@ -80,12 +103,26 @@ public void start() { state = "started"; } + @Override + public void afterExternalServicePostMainLoop() { + LOG.info("After external services post main loop. Test Option is '{}'", testOption); + state = "afterExternalServicePostMainLoop"; + + if (FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP.equals(testOption)) { + state = "failafterExternalServicePostMainLoop"; + throw new RuntimeException("I was told to fail after external services post main loop"); + } + + writeSignal("afterExternalServicePostMainLoop"); + state = "afterExternalServicePostMainLoopFinished"; + } + @Override public void stop() { LOG.info("Stopping. Test Option is '{}'", testOption); state = "stopping"; - if ("FAILSTOP".equals(testOption)) { + if (FAIL_STOP.equals(testOption)) { state = "failstop"; throw new RuntimeException("I was told to fail at stop"); } @@ -103,7 +140,7 @@ public String getState() { @SuppressWarnings("ResultOfMethodCallIgnored") private void writeSignal(final String signal) { try { - final File callbackFile = new File(callbackDir, "pluginLifecycle." + signal); + final File callbackFile = new File(callbackDir, PLUGIN_LIFECYCLE_PREFIX + signal); if (!callbackFile.getParentFile().exists()) { callbackFile.getParentFile().mkdirs(); callbackFile.getParentFile().deleteOnExit(); diff --git a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java index 7e277041776..a6d167665ff 100644 --- a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java +++ b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java @@ -1,5 +1,5 @@ /* - * Copyright ConsenSys AG. + * Copyright contributors to Hyperledger Besu. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at @@ -40,8 +40,31 @@ public class BesuPluginContextImplTest { private static final Path DEFAULT_PLUGIN_DIRECTORY = Paths.get("."); + private static final String TEST_PICO_CLI_PLUGIN = "TestPicoCLIPlugin"; + private static final String TEST_PICO_CLI_PLUGIN_TEST_OPTION = "testPicoCLIPlugin.testOption"; + private static final String FAIL_REGISTER = "FAILREGISTER"; + private static final String FAIL_START = "FAILSTART"; + private static final String FAIL_STOP = "FAILSTOP"; + private static final String FAIL_BEFORE_EXTERNAL_SERVICES = "FAILBEFOREEXTERNALSERVICES"; + private static final String FAIL_BEFORE_MAIN_LOOP = "FAILBEFOREMAINLOOP"; + private static final String FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP = + "FAILAFTEREXTERNALSERVICEPOSTMAINLOOP"; + private static final String NON_EXISTENT_PLUGIN = "NonExistentPlugin"; + private static final String REGISTERED = "registered"; + private static final String STARTED = "started"; + private static final String STOPPED = "stopped"; + private static final String FAIL_START_STATE = "failstart"; + private static final String FAIL_STOP_STATE = "failstop"; + private BesuPluginContextImpl contextImpl; + private static final PluginConfiguration DEFAULT_CONFIGURATION = + PluginConfiguration.builder() + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .externalPluginsEnabled(true) + .continueOnPluginError(true) + .build(); + @BeforeAll public static void createFakePluginDir() throws IOException { if (System.getProperty("besu.plugins.dir") == null) { @@ -53,7 +76,7 @@ public static void createFakePluginDir() throws IOException { @AfterEach public void clearTestPluginState() { - System.clearProperty("testPicoCLIPlugin.testOption"); + System.clearProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION); } @BeforeEach @@ -64,31 +87,31 @@ void setup() { @Test public void verifyEverythingGoesSmoothly() { assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty(); final Optional testPluginOptional = findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(REGISTERED); contextImpl.beforeExternalServices(); contextImpl.startPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("started"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(STARTED); contextImpl.stopPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("stopped"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(STOPPED); } @Test public void registrationErrorsHandledSmoothly() { - System.setProperty("testPicoCLIPlugin.testOption", "FAILREGISTER"); + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_REGISTER); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.beforeExternalServices(); @@ -103,11 +126,11 @@ public void registrationErrorsHandledSmoothly() { @Test public void startErrorsHandledSmoothly() { - System.setProperty("testPicoCLIPlugin.testOption", "FAILSTART"); + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_START); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -116,11 +139,11 @@ public void startErrorsHandledSmoothly() { findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(REGISTERED); contextImpl.beforeExternalServices(); contextImpl.startPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("failstart"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(FAIL_START_STATE); assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.stopPlugins(); @@ -129,11 +152,11 @@ public void startErrorsHandledSmoothly() { @Test public void stopErrorsHandledSmoothly() { - System.setProperty("testPicoCLIPlugin.testOption", "FAILSTOP"); + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_STOP); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -142,22 +165,20 @@ public void stopErrorsHandledSmoothly() { findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(REGISTERED); contextImpl.beforeExternalServices(); contextImpl.startPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("started"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(STARTED); contextImpl.stopPlugins(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("failstop"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(FAIL_STOP_STATE); } @Test public void lifecycleExceptions() throws Throwable { - final ThrowableAssert.ThrowingCallable registerPlugins = - () -> - contextImpl.registerPlugins( - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); + contextImpl.initialize(DEFAULT_CONFIGURATION); + final ThrowableAssert.ThrowingCallable registerPlugins = () -> contextImpl.registerPlugins(); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::startPlugins); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::stopPlugins); @@ -179,30 +200,27 @@ public void lifecycleExceptions() throws Throwable { @Test public void shouldRegisterAllPluginsWhenNoPluginsOption() { - final PluginConfiguration config = - PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build(); - assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(config); + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); final Optional testPluginOptional = findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); - assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); + assertThat(testPicoCLIPlugin.getState()).isEqualTo(REGISTERED); } @Test public void shouldRegisterOnlySpecifiedPluginWhenPluginsOptionIsSet() { - final PluginConfiguration config = createConfigurationForSpecificPlugin("TestPicoCLIPlugin"); - assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(config); + contextImpl.initialize(createConfigurationForSpecificPlugin(TEST_PICO_CLI_PLUGIN)); + contextImpl.registerPlugins(); final Optional requestedPlugin = findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(requestedPlugin).isPresent(); - assertThat(requestedPlugin.get().getState()).isEqualTo("registered"); + assertThat(requestedPlugin.get().getState()).isEqualTo(REGISTERED); final Optional nonRequestedPlugin = findTestPlugin(contextImpl.getRegisteredPlugins(), TestBesuEventsPlugin.class); @@ -212,9 +230,9 @@ public void shouldRegisterOnlySpecifiedPluginWhenPluginsOptionIsSet() { @Test public void shouldNotRegisterUnspecifiedPluginsWhenPluginsOptionIsSet() { - final PluginConfiguration config = createConfigurationForSpecificPlugin("TestPicoCLIPlugin"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(config); + contextImpl.initialize(createConfigurationForSpecificPlugin(TEST_PICO_CLI_PLUGIN)); + contextImpl.registerPlugins(); final Optional nonRequestedPlugin = findTestPlugin(contextImpl.getRegisteredPlugins(), TestBesuEventsPlugin.class); @@ -223,13 +241,12 @@ public void shouldNotRegisterUnspecifiedPluginsWhenPluginsOptionIsSet() { @Test void shouldThrowExceptionIfExplicitlySpecifiedPluginNotFound() { - PluginConfiguration config = createConfigurationForSpecificPlugin("NonExistentPlugin"); - + contextImpl.initialize(createConfigurationForSpecificPlugin(NON_EXISTENT_PLUGIN)); String exceptionMessage = - assertThrows(NoSuchElementException.class, () -> contextImpl.registerPlugins(config)) + assertThrows(NoSuchElementException.class, () -> contextImpl.registerPlugins()) .getMessage(); final String expectedMessage = - "The following requested plugins were not found: NonExistentPlugin"; + "The following requested plugins were not found: " + NON_EXISTENT_PLUGIN; assertThat(exceptionMessage).isEqualTo(expectedMessage); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); } @@ -241,19 +258,180 @@ void shouldNotRegisterAnyPluginsIfExternalPluginsDisabled() { .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) .externalPluginsEnabled(false) .build(); - contextImpl.registerPlugins(config); + contextImpl.initialize(config); + contextImpl.registerPlugins(); assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isTrue(); } @Test void shouldRegisterPluginsIfExternalPluginsEnabled() { + contextImpl.initialize(DEFAULT_CONFIGURATION); + contextImpl.registerPlugins(); + assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isFalse(); + } + + @Test + void shouldHaltOnRegisterErrorWhenFlagIsFalse() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_REGISTER); + PluginConfiguration config = PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) - .externalPluginsEnabled(true) + .continueOnPluginError(false) .build(); - contextImpl.registerPlugins(config); - assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isFalse(); + + contextImpl.initialize(config); + + String errorMessage = + String.format("Error registering plugin of type %s", TestPicoCLIPlugin.class.getName()); + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> contextImpl.registerPlugins()) + .withMessageContaining(errorMessage); + } + + @Test + void shouldNotHaltOnRegisterErrorWhenFlagIsTrue() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_REGISTER); + + PluginConfiguration config = + PluginConfiguration.builder() + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(true) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + } + + @Test + void shouldHaltOnBeforeExternalServicesErrorWhenFlagIsFalse() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_BEFORE_EXTERNAL_SERVICES); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(false) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + + String errorMessage = + String.format( + "Error calling `beforeExternalServices` on plugin of type %s", + TestPicoCLIPlugin.class.getName()); + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> contextImpl.beforeExternalServices()) + .withMessageContaining(errorMessage); + } + + @Test + void shouldNotHaltOnBeforeExternalServicesErrorWhenFlagIsTrue() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_BEFORE_EXTERNAL_SERVICES); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(true) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + } + + @Test + void shouldHaltOnBeforeMainLoopErrorWhenFlagIsFalse() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_START); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(false) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + + String errorMessage = + String.format("Error starting plugin of type %s", TestPicoCLIPlugin.class.getName()); + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> contextImpl.startPlugins()) + .withMessageContaining(errorMessage); + } + + @Test + void shouldNotHaltOnBeforeMainLoopErrorWhenFlagIsTrue() { + System.setProperty(TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_BEFORE_MAIN_LOOP); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(true) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + contextImpl.startPlugins(); + + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + } + + @Test + void shouldHaltOnAfterExternalServicePostMainLoopErrorWhenFlagIsFalse() { + System.setProperty( + TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(false) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + contextImpl.startPlugins(); + + String errorMessage = + String.format( + "Error calling `afterExternalServicePostMainLoop` on plugin of type %s", + TestPicoCLIPlugin.class.getName()); + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> contextImpl.afterExternalServicesMainLoop()) + .withMessageContaining(errorMessage); + } + + @Test + void shouldNotHaltOnAfterExternalServicePostMainLoopErrorWhenFlagIsTrue() { + System.setProperty( + TEST_PICO_CLI_PLUGIN_TEST_OPTION, FAIL_AFTER_EXTERNAL_SERVICE_POST_MAIN_LOOP); + + PluginConfiguration config = + PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(TEST_PICO_CLI_PLUGIN))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .continueOnPluginError(true) + .build(); + + contextImpl.initialize(config); + contextImpl.registerPlugins(); + contextImpl.beforeExternalServices(); + contextImpl.startPlugins(); + contextImpl.afterExternalServicesMainLoop(); + + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); } private PluginConfiguration createConfigurationForSpecificPlugin(final String pluginName) { diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 79879538bd5..e93518cad43 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -118,7 +118,6 @@ import org.hyperledger.besu.ethereum.core.MiningParametersMetrics; import org.hyperledger.besu.ethereum.core.PrivacyParameters; import org.hyperledger.besu.ethereum.core.VersionMetadata; -import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; @@ -1080,9 +1079,8 @@ private IExecutionStrategy createExecuteTask(final IExecutionStrategy nextStep) private IExecutionStrategy createPluginRegistrationTask(final IExecutionStrategy nextStep) { return parseResult -> { - PluginConfiguration configuration = - PluginsConfigurationOptions.fromCommandLine(parseResult.commandSpec().commandLine()); - besuPluginContext.registerPlugins(configuration); + besuPluginContext.initialize(PluginsConfigurationOptions.fromCommandLine(commandLine)); + besuPluginContext.registerPlugins(); commandLine.setExecutionStrategy(nextStep); return commandLine.execute(parseResult.originalArgs().toArray(new String[0])); }; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java b/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java index ba05f455246..8f83c71a787 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java @@ -128,6 +128,9 @@ public interface DefaultCommandValues { /** The constant DEFAULT_PLUGINS_OPTION_NAME. */ String DEFAULT_PLUGINS_OPTION_NAME = "--plugins"; + /** The constant DEFAULT_CONTINUE_ON_PLUGIN_ERROR_OPTION_NAME. */ + String DEFAULT_CONTINUE_ON_PLUGIN_ERROR_OPTION_NAME = "--plugin-continue-on-error"; + /** The constant DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME. */ String DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME = "--Xplugins-external-enabled"; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index 47df831c577..25893fff895 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -14,6 +14,7 @@ */ package org.hyperledger.besu.cli.options.stable; +import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_CONTINUE_ON_PLUGIN_ERROR_OPTION_NAME; import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME; import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_OPTION_NAME; @@ -27,7 +28,7 @@ import picocli.CommandLine; -/** The Plugins Options options. */ +/** The Plugins options. */ public class PluginsConfigurationOptions implements CLIOptions { @CommandLine.Option( @@ -44,9 +45,17 @@ public class PluginsConfigurationOptions implements CLIOptions