Skip to content

Commit

Permalink
Provide better error message if Docker is not running
Browse files Browse the repository at this point in the history
Previously, if the Spring Boot build plugins got a connection error
when attempting to communicate with a Docker daemon (for example,
when the daemon isn't running), the error message made it appear that
the daemon returned an HTTP error code. This commit makes a connection
error distinct from an HTTP error response code to make it easier for
the user to diagnose the root cause of the problem.

Fixes gh-21554
  • Loading branch information
scottfrederick committed May 29, 2020
1 parent d3ef6f2 commit 7722394
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2012-2020 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
*
* https://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.buildpack.platform.docker.transport;

import org.springframework.util.Assert;

/**
* Exception thrown when connection to the Docker daemon fails.
*
* @author Scott Frederick
* @since 2.3.0
*/
public class DockerConnectionException extends RuntimeException {

private static final String JNA_EXCEPTION_CLASS_NAME = "com.sun.jna.LastErrorException";

public DockerConnectionException(String host, Exception cause) {
super(buildMessage(host, cause), cause);
}

private static String buildMessage(String host, Exception cause) {
Assert.notNull(host, "host must not be null");
Assert.notNull(cause, "cause must not be null");
StringBuilder message = new StringBuilder("Connection to the Docker daemon at '" + host + "' failed");

This comment has been minimized.

Copy link
@charboubmustapha

charboubmustapha May 29, 2020

probably use the append of StringBuilder instead of String concatenation

This comment has been minimized.

Copy link
@philwebb

philwebb May 29, 2020

Member

There's not much benefit. It would make the code harder to read and since this code only runs when there's an error there's no performance problem with things as they are.

This comment has been minimized.

Copy link
@RUBenGAMArrarodRiguEZ-ToMtOm

RUBenGAMArrarodRiguEZ-ToMtOm Aug 3, 2020

Agree and this looks fine to me.
@charboubmustapha Since Java 7 (almost 10 years ago) A compiler might decide the use of a String concatenation + using an StringBuffer as replacement.
https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.18.1

String causeMessage = getCauseMessage(cause);
if (causeMessage != null && !causeMessage.isEmpty()) {
message.append(" with error \"").append(causeMessage).append("\"");
}
message.append("; ensure the Docker daemon is running and accessible");
return message.toString();
}

private static String getCauseMessage(Exception cause) {
if (cause.getCause() != null && cause.getCause().getClass().getName().equals(JNA_EXCEPTION_CLASS_NAME)) {
return cause.getCause().getMessage();
}

return cause.getMessage();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import org.springframework.util.Assert;

/**
* Exception throw when the Docker API fails.
* Exception thrown when a call to the Docker API fails.
*
* @author Phillip Webb
* @author Scott Frederick
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private Response execute(HttpUriRequest request) {
return new HttpClientResponse(response);
}
catch (IOException ex) {
throw new DockerEngineException(this.host.toHostString(), request.getURI(), 500, ex.getMessage(), null);
throw new DockerConnectionException(this.host.toHostString(), ex);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright 2012-2020 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
*
* https://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.buildpack.platform.docker.transport;

import java.io.IOException;

import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;

/**
* Tests for {@link DockerEngineException}.
*
* @author Scott Frederick
*/
class DockerConnectionExceptionTests {

private static final String HOST = "docker://localhost/";

@Test
void createWhenHostIsNullThrowsException() {
assertThatIllegalArgumentException().isThrownBy(() -> new DockerConnectionException(null, null))
.withMessage("host must not be null");
}

@Test
void createWhenCauseIsNullThrowsException() {
assertThatIllegalArgumentException().isThrownBy(() -> new DockerConnectionException(HOST, null))
.withMessage("cause must not be null");
}

@Test
void createWithIOException() {
DockerConnectionException exception = new DockerConnectionException(HOST, new IOException("error"));
assertThat(exception.getMessage())
.contains("Connection to the Docker daemon at 'docker://localhost/' failed with error \"error\"");
}

@Test
void createWithLastErrorException() {
DockerConnectionException exception = new DockerConnectionException(HOST,
new IOException(new com.sun.jna.LastErrorException("root cause")));
assertThat(exception.getMessage())
.contains("Connection to the Docker daemon at 'docker://localhost/' failed with error \"root cause\"");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,8 @@ void executeWhenResponseIsIn500RangeShouldThrowDockerException() {
void executeWhenClientThrowsIOExceptionRethrowsAsDockerException() throws IOException {
given(this.client.execute(any(HttpHost.class), any(HttpRequest.class)))
.willThrow(new IOException("test IO exception"));
assertThatExceptionOfType(DockerEngineException.class).isThrownBy(() -> this.http.get(this.uri))
.satisfies((ex) -> assertThat(ex.getErrors()).isNull()).satisfies(DockerEngineException::getStatusCode)
.withMessageContaining("500")
.satisfies((ex) -> assertThat(ex.getReasonPhrase()).contains("test IO exception"));
assertThatExceptionOfType(DockerConnectionException.class).isThrownBy(() -> this.http.get(this.uri))
.satisfies((ex) -> assertThat(ex.getMessage()).contains("test IO exception"));
}

private String writeToString(HttpEntity entity) throws IOException {
Expand Down

0 comments on commit 7722394

Please sign in to comment.