From 564207b0925cabae149290152983d34d1c8d617b Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 20 Sep 2016 13:55:10 +0100 Subject: [PATCH] Add Tomcat-specific failure analysis for connector start failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, when a Tomcat connector failed to start it was assumed that the failure was due to the port being in use and a PortInUseException was thrown. Unfortunately, this assumption doesn’t always hold true. For example, a Tomcat connector will also fail to start when its using SSL and the key store password is wrong. This could lead to incorrect guidance from the PortInUseFailureAnalyzer indicating that a port clash had occurred when, in fact, it was the SSL configuration that needed to be corrected. Unfortunately, Tomcat only tells us that the connector failed to start. It doesn’t provide access to the exception that would allow us to determine why it failed to start. This commit updates the embedded Tomcat container to throw a ConnectorStartFailedException in the event of a connector failing to start. A new failure analyser, ConnectorStartFailureAnalyzer, has been introduced to analyse the new exception and offer some more general guidance. Closes gh-6896 --- .../tomcat/ConnectorStartFailedException.java | 50 +++++++++++++++++++ .../TomcatEmbeddedServletContainer.java | 5 +- .../ConnectorStartFailureAnalyzer.java | 44 ++++++++++++++++ .../main/resources/META-INF/spring.factories | 1 + ...tEmbeddedServletContainerFactoryTests.java | 11 ++-- ...yEmbeddedServletContainerFactoryTests.java | 8 +++ ...tEmbeddedServletContainerFactoryTests.java | 7 +++ ...wEmbeddedServletContainerFactoryTests.java | 8 +++ 8 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/ConnectorStartFailedException.java create mode 100644 spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/ConnectorStartFailureAnalyzer.java diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/ConnectorStartFailedException.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/ConnectorStartFailedException.java new file mode 100644 index 00000000000..b88babf49d1 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/ConnectorStartFailedException.java @@ -0,0 +1,50 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.context.embedded.tomcat; + +import org.apache.catalina.connector.Connector; + +import org.springframework.boot.context.embedded.EmbeddedServletContainerException; + +/** + * A {@code ConnectorStartFailedException} is thrown when a Tomcat {@link Connector} fails + * to start, for example due to a port clash or incorrect SSL configuration. + * + * @author Andy Wilkinson + * @since 1.4.1 + */ +public class ConnectorStartFailedException extends EmbeddedServletContainerException { + + private final int port; + + /** + * Creates a new {@code ConnectorStartFailedException} for a connector that's + * configured to listen on the given {@code port}. + * + * @param port the port + */ + public ConnectorStartFailedException(int port) { + super("Connector configured to listen on port " + port + " failed to start", + null); + this.port = port; + } + + public int getPort() { + return this.port; + } + +} diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java index 210b9217710..ac82e37b270 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java @@ -36,7 +36,6 @@ import org.apache.naming.ContextBindings; import org.springframework.boot.context.embedded.EmbeddedServletContainer; import org.springframework.boot.context.embedded.EmbeddedServletContainerException; -import org.springframework.boot.context.embedded.PortInUseException; import org.springframework.util.Assert; /** @@ -185,7 +184,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer TomcatEmbeddedServletContainer.logger .info("Tomcat started on port(s): " + getPortsDescription(true)); } - catch (PortInUseException ex) { + catch (ConnectorStartFailedException ex) { stopSilently(); throw ex; } @@ -203,7 +202,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer private void checkThatConnectorsHaveStarted() { for (Connector connector : this.tomcat.getService().findConnectors()) { if (LifecycleState.FAILED.equals(connector.getState())) { - throw new PortInUseException(connector.getPort()); + throw new ConnectorStartFailedException(connector.getPort()); } } } diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/ConnectorStartFailureAnalyzer.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/ConnectorStartFailureAnalyzer.java new file mode 100644 index 00000000000..18746bdf26a --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/ConnectorStartFailureAnalyzer.java @@ -0,0 +1,44 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.diagnostics.analyzer; + +import org.springframework.boot.context.embedded.tomcat.ConnectorStartFailedException; +import org.springframework.boot.diagnostics.AbstractFailureAnalyzer; +import org.springframework.boot.diagnostics.FailureAnalysis; + +/** + * An {@link AbstractFailureAnalyzer} for {@link ConnectorStartFailedException}. + * + * @author Andy Wilkinson + */ +class ConnectorStartFailureAnalyzer + extends AbstractFailureAnalyzer { + + @Override + protected FailureAnalysis analyze(Throwable rootFailure, + ConnectorStartFailedException cause) { + return new FailureAnalysis( + "The Tomcat connector configured to listen on port " + cause.getPort() + + " failed to start. The port may already be in use or the" + + " connector may be misconfigured.", + "Verify the connector's configuration, identify and stop any process " + + "that's listening on port " + cause.getPort() + + ", or configure this application to listen on another port.", + cause); + } + +} diff --git a/spring-boot/src/main/resources/META-INF/spring.factories b/spring-boot/src/main/resources/META-INF/spring.factories index fe6431b8886..73a6988ec71 100644 --- a/spring-boot/src/main/resources/META-INF/spring.factories +++ b/spring-boot/src/main/resources/META-INF/spring.factories @@ -36,6 +36,7 @@ org.springframework.boot.diagnostics.FailureAnalyzer=\ org.springframework.boot.diagnostics.analyzer.BeanCurrentlyInCreationFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.BeanNotOfRequiredTypeFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.BindFailureAnalyzer,\ +org.springframework.boot.diagnostics.analyzer.ConnectorStartFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.NoUniqueBeanDefinitionFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.PortInUseFailureAnalyzer,\ org.springframework.boot.diagnostics.analyzer.ValidationExceptionFailureAnalyzer diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java index d5371fd78bf..71962fbb4c7 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java @@ -840,8 +840,8 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { AbstractEmbeddedServletContainerFactoryTests.this.container.start(); fail(); } - catch (PortInUseException ex) { - assertThat(ex.getPort()).isEqualTo(port); + catch (RuntimeException ex) { + handleExceptionCausedByBlockedPort(ex, port); } } @@ -864,8 +864,8 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { AbstractEmbeddedServletContainerFactoryTests.this.container.start(); fail(); } - catch (PortInUseException ex) { - assertThat(ex.getPort()).isEqualTo(port); + catch (RuntimeException ex) { + handleExceptionCausedByBlockedPort(ex, port); } } @@ -886,6 +886,9 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { protected abstract void addConnector(int port, AbstractEmbeddedServletContainerFactory factory); + protected abstract void handleExceptionCausedByBlockedPort(RuntimeException ex, + int blockedPort); + private boolean doTestCompression(int contentSize, String[] mimeTypes, String[] excludedUserAgents) throws Exception { String testContent = setUpFactoryForCompression(contentSize, mimeTypes, diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java index 9ac8844d2f2..cedeaae7846 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java @@ -45,6 +45,7 @@ import org.mockito.InOrder; import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactory; import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactoryTests; import org.springframework.boot.context.embedded.Compression; +import org.springframework.boot.context.embedded.PortInUseException; import org.springframework.boot.context.embedded.Ssl; import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.http.HttpHeaders; @@ -336,4 +337,11 @@ public class JettyEmbeddedServletContainerFactoryTests return (charsetName != null) ? Charset.forName(charsetName) : null; } + @Override + protected void handleExceptionCausedByBlockedPort(RuntimeException ex, + int blockedPort) { + assertThat(ex).isInstanceOf(PortInUseException.class); + assertThat(((PortInUseException) ex).getPort()).isEqualTo(blockedPort); + } + } diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java index c102462d0a4..80289374a44 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java @@ -498,4 +498,11 @@ public class TomcatEmbeddedServletContainerFactoryTests return ((TomcatEmbeddedServletContainer) this.container).getTomcat(); } + @Override + protected void handleExceptionCausedByBlockedPort(RuntimeException ex, + int blockedPort) { + assertThat(ex).isInstanceOf(ConnectorStartFailedException.class); + assertThat(((ConnectorStartFailedException) ex).getPort()).isEqualTo(blockedPort); + } + } diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactoryTests.java index 2fff72cea40..b3ad0fe8b26 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactoryTests.java @@ -41,6 +41,7 @@ import org.springframework.boot.context.embedded.AbstractEmbeddedServletContaine import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactoryTests; import org.springframework.boot.context.embedded.ExampleServlet; import org.springframework.boot.context.embedded.MimeMappings.Mapping; +import org.springframework.boot.context.embedded.PortInUseException; import org.springframework.boot.web.servlet.ErrorPage; import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.http.HttpStatus; @@ -291,4 +292,11 @@ public class UndertowEmbeddedServletContainerFactoryTests return (charsetName != null) ? Charset.forName(charsetName) : null; } + @Override + protected void handleExceptionCausedByBlockedPort(RuntimeException ex, + int blockedPort) { + assertThat(ex).isInstanceOf(PortInUseException.class); + assertThat(((PortInUseException) ex).getPort()).isEqualTo(blockedPort); + } + }