diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java index 5fcfbbc1cae..a61c471cd8b 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java @@ -68,6 +68,7 @@ import org.eclipse.jetty.xml.XmlConfiguration; public class DeploymentManager extends ContainerLifeCycle { private static final Logger LOG = Log.getLogger(DeploymentManager.class); + private List onStartupErrors; /** * Represents a single tracked app within the deployment manager. @@ -237,6 +238,14 @@ public class DeploymentManager extends ContainerLifeCycle { startAppProvider(provider); } + + if (onStartupErrors != null) + { + RuntimeException mex = new RuntimeException("Failed to successfully start App Providers (See log for details)"); + onStartupErrors.forEach((cause) -> mex.addSuppressed(cause)); + throw mex; + } + super.doStart(); } @@ -507,6 +516,7 @@ public class DeploymentManager extends ContainerLifeCycle catch (Throwable t) { LOG.warn("Unable to reach node goal: " + nodeName,t); + LOG.info("state() = {}", getState()); // migrate to FAILED node Node failed = _lifecycle.getNodeByName(AppLifeCycle.FAILED); appentry.setLifeCycleNode(failed); @@ -519,9 +529,23 @@ public class DeploymentManager extends ContainerLifeCycle // The runBindings failed for 'failed' node LOG.ignore(ignore); } + + if (isStarting()) + { + addOnStartupError(t); + } } } + private synchronized void addOnStartupError(Throwable cause) + { + if(onStartupErrors == null) + { + onStartupErrors = new ArrayList(); + } + onStartupErrors.add(cause); + } + /** * Move an {@link App} through the {@link AppLifeCycle} to the desired {@link Node}, executing each lifecycle step * in the process to reach the desired state. diff --git a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/BadAppDeployTest.java b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/BadAppDeployTest.java new file mode 100644 index 00000000000..8ab3663b82f --- /dev/null +++ b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/BadAppDeployTest.java @@ -0,0 +1,181 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.deploy; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import javax.servlet.ServletException; + +import org.eclipse.jetty.deploy.providers.WebAppProvider; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.handler.ContextHandlerCollection; +import org.eclipse.jetty.server.handler.DefaultHandler; +import org.eclipse.jetty.server.handler.HandlerCollection; +import org.eclipse.jetty.toolchain.test.FS; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; +import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.StacklessLogging; +import org.eclipse.jetty.webapp.WebAppContext; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import static java.time.Duration.ofSeconds; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@ExtendWith(WorkDirExtension.class) +public class BadAppDeployTest +{ + public WorkDir workDir; + private Server server; + + @AfterEach + public void stopServer() throws Exception + { + if (server != null) + { + server.stop(); + } + } + + @Test + public void testBadApp_XmlOrder() throws Exception + { + /* Non-working Bean Order as reported in Issue #3620 + It is important that this Order be maintained for an accurate test case. + ### BEAN: QueuedThreadPool[qtp1327763628]@4f2410ac{STOPPED,8<=0<=200,i=0,r=-1,q=0}[NO_TRY] + ### BEAN: ServerConnector@16f65612{HTTP/1.1,[http/1.1]}{0.0.0.0:8080} + ### BEAN: HandlerCollection@5f150435{STOPPED} + ### BEAN: DeploymentManager@1c53fd30{STOPPED} + */ + + /* Working Bean Order + ### BEAN: QueuedThreadPool[qtp1530388690]@5b37e0d2{STOPPED,8<=0<=200,i=0,r=-1,q=0}[NO_TRY] + ### BEAN: ServerConnector@5e265ba4{HTTP/1.1,[http/1.1]}{0.0.0.0:8080} + ### BEAN: DeploymentManager@3419866c{STOPPED} + ### BEAN: HandlerCollection@63e31ee{STOPPED} + */ + + server = new Server(); + ServerConnector connector = new ServerConnector(server); + connector.setPort(0); + server.addConnector(connector); + + ContextHandlerCollection contexts = new ContextHandlerCollection(); + HandlerCollection handlers = new HandlerCollection(); + handlers.addHandler(contexts); + handlers.addHandler(new DefaultHandler()); + server.setHandler(handlers); // this should be done before addBean(deploymentManager) + + DeploymentManager deploymentManager = new DeploymentManager(); + deploymentManager.setContexts(contexts); + WebAppProvider webAppProvider = new WebAppProvider(); + deploymentManager.addAppProvider(webAppProvider); + + Path webappsDir = workDir.getEmptyPathDir().resolve("webapps").toAbsolutePath(); + + FS.ensureDirExists(webappsDir); + + copyTestResource("webapps/badapp/badapp.war", webappsDir.resolve("badapp.war")); + copyTestResource("webapps/badapp/badapp.xml", webappsDir.resolve("badapp.xml")); + + webAppProvider.setMonitoredDirName(webappsDir.toString()); + webAppProvider.setScanInterval(1); + + server.addBean(deploymentManager); // this should be done after setHandler(handlers) + + assertTimeoutPreemptively(ofSeconds(10), () -> { + + try (StacklessLogging ignore = new StacklessLogging(Log.getLogger(WebAppContext.class), + Log.getLogger(DeploymentManager.class), + Log.getLogger("org.eclipse.jetty.server.handler.ContextHandler.badapp"))) + { + RuntimeException cause = assertThrows(RuntimeException.class, () -> server.start()); + assertThat(cause.getMessage(), containsString("Failed to successfully start App Providers")); + assertTrue(server.isFailed(), "Server should be in failed state"); + } + }); + } + + @Test + public void testBadApp_EmbeddedOrder() throws Exception + { + /* Working Bean Order + ### BEAN: QueuedThreadPool[qtp1530388690]@5b37e0d2{STOPPED,8<=0<=200,i=0,r=-1,q=0}[NO_TRY] + ### BEAN: ServerConnector@5e265ba4{HTTP/1.1,[http/1.1]}{0.0.0.0:8080} + ### BEAN: DeploymentManager@3419866c{STOPPED} + ### BEAN: HandlerCollection@63e31ee{STOPPED} + */ + + server = new Server(); + ServerConnector connector = new ServerConnector(server); + connector.setPort(0); + server.addConnector(connector); + + ContextHandlerCollection contexts = new ContextHandlerCollection(); + + DeploymentManager deploymentManager = new DeploymentManager(); + deploymentManager.setContexts(contexts); + WebAppProvider webAppProvider = new WebAppProvider(); + deploymentManager.addAppProvider(webAppProvider); + + Path webappsDir = workDir.getEmptyPathDir().resolve("webapps").toAbsolutePath(); + + FS.ensureDirExists(webappsDir); + + copyTestResource("webapps/badapp/badapp.war", webappsDir.resolve("badapp.war")); + copyTestResource("webapps/badapp/badapp.xml", webappsDir.resolve("badapp.xml")); + + webAppProvider.setMonitoredDirName(webappsDir.toString()); + webAppProvider.setScanInterval(1); + + server.addBean(deploymentManager); // this should be done before setHandler(handlers) + + HandlerCollection handlers = new HandlerCollection(); + handlers.addHandler(contexts); + handlers.addHandler(new DefaultHandler()); + server.setHandler(handlers); // this should be done after addBean(deploymentManager) + + assertTimeoutPreemptively(ofSeconds(10), () -> { + + try (StacklessLogging ignore = new StacklessLogging(Log.getLogger(WebAppContext.class), + Log.getLogger(DeploymentManager.class), + Log.getLogger("org.eclipse.jetty.server.handler.ContextHandler.badapp"))) + { + ServletException cause = assertThrows(ServletException.class, () -> server.start()); + assertThat(cause.getMessage(), containsString("intentionally")); + assertTrue(server.isFailed(), "Server should be in failed state"); + } + }); + } + + private void copyTestResource(String testResourceFile, Path webappsFile) throws IOException + { + Path srcFile = MavenTestingUtils.getTestResourcePathFile(testResourceFile); + Files.copy(srcFile, webappsFile); + } +} diff --git a/jetty-deploy/src/test/resources/webapps/badapp/badapp.war b/jetty-deploy/src/test/resources/webapps/badapp/badapp.war new file mode 100644 index 00000000000..3fc1a60d5fe Binary files /dev/null and b/jetty-deploy/src/test/resources/webapps/badapp/badapp.war differ diff --git a/jetty-deploy/src/test/resources/webapps/badapp/badapp.xml b/jetty-deploy/src/test/resources/webapps/badapp/badapp.xml new file mode 100644 index 00000000000..d085d7b9cec --- /dev/null +++ b/jetty-deploy/src/test/resources/webapps/badapp/badapp.xml @@ -0,0 +1,8 @@ + + + + + /badapp + /badapp.war + true + diff --git a/tests/test-distribution/pom.xml b/tests/test-distribution/pom.xml index fb9106462ae..47dad9d2a39 100644 --- a/tests/test-distribution/pom.xml +++ b/tests/test-distribution/pom.xml @@ -78,7 +78,6 @@ org.eclipse.jetty.toolchain jetty-test-helper - test org.eclipse.jetty diff --git a/tests/test-distribution/src/main/java/org/eclipse/jetty/tests/distribution/DistributionTester.java b/tests/test-distribution/src/main/java/org/eclipse/jetty/tests/distribution/DistributionTester.java index 44a2a9edda9..9faf5b73010 100644 --- a/tests/test-distribution/src/main/java/org/eclipse/jetty/tests/distribution/DistributionTester.java +++ b/tests/test-distribution/src/main/java/org/eclipse/jetty/tests/distribution/DistributionTester.java @@ -62,6 +62,8 @@ import org.eclipse.aether.spi.connector.transport.TransporterFactory; import org.eclipse.aether.transfer.AbstractTransferListener; import org.eclipse.aether.transport.file.FileTransporterFactory; import org.eclipse.aether.transport.http.HttpTransporterFactory; +import org.eclipse.jetty.toolchain.test.FS; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -166,6 +168,21 @@ public class DistributionTester } } + /** + * Installs content from {@code src/test/resources/} into {@code ${jetty.base}/} + * + * @param testResourcePath the location of the source file in {@code src/test/resources} + * @param baseResourcePath the location of the destination file in {@code ${jetty.base}} + * @throws IOException if unable to copy file + */ + public void installBaseResource(String testResourcePath, String baseResourcePath) throws IOException + { + Path srcFile = MavenTestingUtils.getTestResourcePath(testResourcePath); + Path destFile = config.jettyBase.resolve(baseResourcePath); + + Files.copy(srcFile, destFile); + } + /** * Installs in {@code ${jetty.base}/webapps} the given war file under the given context path. * @@ -176,7 +193,7 @@ public class DistributionTester public void installWarFile(File warFile, String context) throws IOException { //webapps - Path webapps = Paths.get(config.jettyBase.toString(), "webapps", context); + Path webapps = config.jettyBase.resolve("webapps").resolve(context); if (!Files.exists(webapps)) Files.createDirectories(webapps); unzip(warFile, webapps.toFile()); @@ -213,7 +230,9 @@ public class DistributionTester if (config.jettyBase == null) { - config.jettyBase = Files.createTempDirectory("jetty_base_"); + Path bases = MavenTestingUtils.getTargetTestingPath("bases"); + FS.ensureDirExists(bases); + config.jettyBase = Files.createTempDirectory(bases, "jetty_base_"); } else { @@ -225,12 +244,12 @@ public class DistributionTester private String getJavaExecutable() { String[] javaExecutables = new String[]{"java", "java.exe"}; - File javaHomeDir = new File(System.getProperty("java.home")); + Path javaBinDir = Paths.get(System.getProperty("java.home")).resolve("bin"); for (String javaExecutable : javaExecutables) { - File javaFile = new File(javaHomeDir, "bin" + File.separator + javaExecutable); - if (javaFile.exists() && javaFile.isFile()) - return javaFile.getAbsolutePath(); + Path javaFile = javaBinDir.resolve(javaExecutable); + if (Files.exists(javaFile) && Files.isRegularFile(javaFile)) + return javaFile.toAbsolutePath().toString(); } return "java"; } diff --git a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/BadStartupTest.java b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/BadStartupTest.java new file mode 100644 index 00000000000..8a1283b75c2 --- /dev/null +++ b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/BadStartupTest.java @@ -0,0 +1,64 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.tests.distribution; + +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests where we expect the server to not start properly. + * Presumably due to bad configuration, or bad webapps. + */ +public class BadStartupTest +{ + @Test + public void testThrowOnUnavailable_BadApp() throws Exception + { + String jettyVersion = System.getProperty("jettyVersion"); + DistributionTester distribution = DistributionTester.Builder.newInstance() + .jettyVersion(jettyVersion) + .mavenLocalRepository(System.getProperty("mavenRepoPath")) + .build(); + + try (DistributionTester.Run run1 = distribution.start("--add-to-start=http,deploy")) + { + assertTrue(run1.awaitFor(5, TimeUnit.SECONDS)); + assertThat(run1.getExitValue(), is(0)); + + // Setup webapps directory + distribution.installBaseResource("badapp/badapp.war", + "webapps/badapp.war"); + distribution.installBaseResource("badapp/badapp.xml", + "webapps/badapp.xml"); + + int port = distribution.freePort(); + try (DistributionTester.Run run2 = distribution.start("jetty.http.port=" + port)) + { + assertTrue(run2.awaitFor(5, TimeUnit.SECONDS), "Should have exited"); + assertThat("Should have gotten a non-zero exit code", run2.getExitValue(), not(is(0))); + } + } + } +} diff --git a/tests/test-distribution/src/test/resources/badapp/badapp.war b/tests/test-distribution/src/test/resources/badapp/badapp.war new file mode 100644 index 00000000000..3fc1a60d5fe Binary files /dev/null and b/tests/test-distribution/src/test/resources/badapp/badapp.war differ diff --git a/tests/test-distribution/src/test/resources/badapp/badapp.xml b/tests/test-distribution/src/test/resources/badapp/badapp.xml new file mode 100644 index 00000000000..d085d7b9cec --- /dev/null +++ b/tests/test-distribution/src/test/resources/badapp/badapp.xml @@ -0,0 +1,8 @@ + + + + + /badapp + /badapp.war + true +