From 1c170b35ea5c888f55da791f061f94de69c0d31e Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 12 Feb 2016 11:52:41 +0000 Subject: [PATCH] Improve handling of connection failures in remote debug tunnel Previously, if an application had been started without remote debugging enabled, an attempt to connect to it via RemoteSpringApplication and the HTTP tunnel would result in the application being hammered by connection attempts for 30 seconds. This commit updates the tunnel server to respond with Service Unavailable (503) when a connection attempt is made and the JVM does not have remote debugging enabled. When the client receives a 503 response, it now logs a warning message describing the possible problem before closing the connection. The client has also been updated to provide improved diagnostics when a connection to the tunnel server cannot be established, for example because the remote URL is incorrect, or the remote application isn't running. Lastly, the client has been updated so that it continues to accept connections when a connection to the server is closed. This allows the user to correct a problem with the remote application, such as restarting it with remote debugging enabled, without having to also restart the process that's running RemoteSpringApplication. Closes gh-5021 --- .../tunnel/client/HttpTunnelConnection.java | 18 ++++++++++-- .../devtools/tunnel/client/TunnelClient.java | 7 ++++- .../tunnel/server/HttpTunnelServer.java | 10 ++++++- .../RemoteDebugNotRunningException.java | 26 +++++++++++++++++ .../server/RemoteDebugPortProvider.java | 8 +++-- .../test/MockClientHttpRequestFactory.java | 19 +++++++++--- .../client/HttpTunnelConnectionTests.java | 29 ++++++++++++++++++- 7 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/RemoteDebugNotRunningException.java diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/HttpTunnelConnection.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/HttpTunnelConnection.java index 219af782f66..9516bf980e0 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/HttpTunnelConnection.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/HttpTunnelConnection.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * 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. @@ -18,6 +18,7 @@ package org.springframework.boot.devtools.tunnel.client; import java.io.Closeable; import java.io.IOException; +import java.net.ConnectException; import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; @@ -46,6 +47,7 @@ import org.springframework.util.Assert; * * @author Phillip Webb * @author Rob Winch + * @author Andy Wilkinson * @since 1.3.0 * @see TunnelClient * @see org.springframework.boot.devtools.tunnel.server.HttpTunnelServer @@ -157,7 +159,13 @@ public class HttpTunnelConnection implements TunnelConnection { sendAndReceive(payload); } catch (IOException ex) { - logger.trace("Unexpected connection error", ex); + if (ex instanceof ConnectException) { + logger.warn("Failed to connect to remote application at " + + HttpTunnelConnection.this.uri); + } + else { + logger.trace("Unexpected connection error", ex); + } closeQuietly(); } } @@ -188,6 +196,12 @@ public class HttpTunnelConnection implements TunnelConnection { close(); return; } + if (response.getStatusCode() == HttpStatus.SERVICE_UNAVAILABLE) { + logger.warn("Remote application responded with service unavailable. Did " + + "you forget to start it with remote debugging enabled?"); + close(); + return; + } if (response.getStatusCode() == HttpStatus.OK) { HttpTunnelPayload payload = HttpTunnelPayload.get(response); if (payload != null) { diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/TunnelClient.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/TunnelClient.java index 2991a284afa..37cf021e0e0 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/TunnelClient.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/client/TunnelClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * 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. @@ -21,6 +21,7 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.ServerSocket; import java.nio.ByteBuffer; +import java.nio.channels.AsynchronousCloseException; import java.nio.channels.ServerSocketChannel; import java.nio.channels.SocketChannel; import java.nio.channels.WritableByteChannel; @@ -36,6 +37,7 @@ import org.springframework.util.Assert; * specified port for local clients to connect to. * * @author Phillip Webb + * @author Andy Wilkinson * @since 1.3.0 */ public class TunnelClient implements SmartInitializingSingleton { @@ -143,6 +145,9 @@ public class TunnelClient implements SmartInitializingSingleton { try { handleConnection(socket); } + catch (AsynchronousCloseException ex) { + // Connection has been closed. Keep the server running + } finally { socket.close(); } diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/HttpTunnelServer.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/HttpTunnelServer.java index c1bfae52a5f..85d4ef237b7 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/HttpTunnelServer.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/HttpTunnelServer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * 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. @@ -88,6 +88,10 @@ import org.springframework.util.Assert; * 410 (Gone) * The target server has disconnected. * + * + * 503 (Service Unavailable) + * The target server is unavailable + * * *

* Requests and responses that contain payloads include a {@code x-seq} header that @@ -96,6 +100,7 @@ import org.springframework.util.Assert; * {@code 1}. * * @author Phillip Webb + * @author Andy Wilkinson * @since 1.3.0 * @see org.springframework.boot.devtools.tunnel.client.HttpTunnelConnection */ @@ -153,6 +158,9 @@ public class HttpTunnelServer { catch (ConnectException ex) { httpConnection.respond(HttpStatus.GONE); } + catch (RemoteDebugNotRunningException ex) { + httpConnection.respond(HttpStatus.SERVICE_UNAVAILABLE); + } } /** diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/RemoteDebugNotRunningException.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/RemoteDebugNotRunningException.java new file mode 100644 index 00000000000..2c2af1d0308 --- /dev/null +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/RemoteDebugNotRunningException.java @@ -0,0 +1,26 @@ +/* + * 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.devtools.tunnel.server; + +/** + * Exception thrown to indicate that remote debug is not running. + * + * @author Andy Wilkinson + */ +class RemoteDebugNotRunningException extends RuntimeException { + +} diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/RemoteDebugPortProvider.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/RemoteDebugPortProvider.java index 6f525c4588e..9223da08d4f 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/RemoteDebugPortProvider.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/tunnel/server/RemoteDebugPortProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * 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. @@ -20,12 +20,12 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.boot.lang.UsesUnsafeJava; -import org.springframework.util.Assert; /** * {@link PortProvider} that provides the port being used by the Java remote debugging. * * @author Phillip Webb + * @author Andy Wilkinson */ public class RemoteDebugPortProvider implements PortProvider { @@ -35,7 +35,9 @@ public class RemoteDebugPortProvider implements PortProvider { @Override public int getPort() { - Assert.state(isRemoteDebugRunning(), "Remote debug is not running"); + if (!isRemoteDebugRunning()) { + throw new RemoteDebugNotRunningException(); + } return getRemoteDebugPort(); } diff --git a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/test/MockClientHttpRequestFactory.java b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/test/MockClientHttpRequestFactory.java index 9f518b2839f..3d10375c609 100644 --- a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/test/MockClientHttpRequestFactory.java +++ b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/test/MockClientHttpRequestFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * 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. @@ -37,12 +37,13 @@ import org.springframework.mock.http.client.MockClientHttpResponse; * Mock {@link ClientHttpRequestFactory}. * * @author Phillip Webb + * @author Andy Wilkinson */ public class MockClientHttpRequestFactory implements ClientHttpRequestFactory { private AtomicLong seq = new AtomicLong(); - private Deque responses = new ArrayDeque(); + private Deque responses = new ArrayDeque(); private List executedRequests = new ArrayList(); @@ -58,6 +59,12 @@ public class MockClientHttpRequestFactory implements ClientHttpRequestFactory { } } + public void willRespond(IOException... response) { + for (IOException exception : response) { + this.responses.addLast(exception); + } + } + public void willRespond(String... response) { for (String payload : response) { this.responses.add(new Response(0, payload.getBytes(), HttpStatus.OK)); @@ -81,11 +88,15 @@ public class MockClientHttpRequestFactory implements ClientHttpRequestFactory { @Override protected ClientHttpResponse executeInternal() throws IOException { MockClientHttpRequestFactory.this.executedRequests.add(this); - Response response = MockClientHttpRequestFactory.this.responses.pollFirst(); + Object response = MockClientHttpRequestFactory.this.responses.pollFirst(); + if (response instanceof IOException) { + throw (IOException) response; + } if (response == null) { response = new Response(0, null, HttpStatus.GONE); } - return response.asHttpResponse(MockClientHttpRequestFactory.this.seq); + return ((Response) response) + .asHttpResponse(MockClientHttpRequestFactory.this.seq); } } diff --git a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/tunnel/client/HttpTunnelConnectionTests.java b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/tunnel/client/HttpTunnelConnectionTests.java index c8f0f2b8d13..568928cda11 100644 --- a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/tunnel/client/HttpTunnelConnectionTests.java +++ b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/tunnel/client/HttpTunnelConnectionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * 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. @@ -19,6 +19,7 @@ package org.springframework.boot.devtools.tunnel.client; import java.io.ByteArrayOutputStream; import java.io.Closeable; import java.io.IOException; +import java.net.ConnectException; import java.nio.ByteBuffer; import java.nio.channels.Channels; import java.nio.channels.WritableByteChannel; @@ -33,11 +34,14 @@ import org.mockito.MockitoAnnotations; import org.springframework.boot.devtools.test.MockClientHttpRequestFactory; import org.springframework.boot.devtools.tunnel.client.HttpTunnelConnection.TunnelChannel; +import org.springframework.boot.test.OutputCapture; import org.springframework.http.HttpStatus; import org.springframework.util.SocketUtils; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -48,12 +52,16 @@ import static org.mockito.Mockito.verify; * * @author Phillip Webb * @author Rob Winch + * @author Andy Wilkinson */ public class HttpTunnelConnectionTests { @Rule public ExpectedException thrown = ExpectedException.none(); + @Rule + public OutputCapture outputCapture = new OutputCapture(); + private int port = SocketUtils.findAvailableTcpPort(); private String url; @@ -144,6 +152,25 @@ public class HttpTunnelConnectionTests { assertThat(this.requestFactory.getExecutedRequests().size(), greaterThan(10)); } + @Test + public void serviceUnavailableResponseLogsWarningAndClosesTunnel() throws Exception { + this.requestFactory.willRespond(HttpStatus.SERVICE_UNAVAILABLE); + TunnelChannel tunnel = openTunnel(true); + assertThat(tunnel.isOpen(), is(false)); + this.outputCapture.expect(containsString( + "Did you forget to start it with remote debugging enabled?")); + } + + @Test + public void connectFailureLogsWarning() throws Exception { + this.requestFactory.willRespond(new ConnectException()); + TunnelChannel tunnel = openTunnel(true); + assertThat(tunnel.isOpen(), is(false)); + this.outputCapture.expect(containsString( + "Failed to connect to remote application at http://localhost:" + + this.port)); + } + private void write(TunnelChannel channel, String string) throws IOException { channel.write(ByteBuffer.wrap(string.getBytes())); }