Skip to content

Commit

Permalink
Improve Docker API 5xx error messages
Browse files Browse the repository at this point in the history
This commit improves the error messages returned by the Spring Boot
build plugins when a 5xx status code is returned from the Docker
API while attempting to build an image. If the error response has
contents containing a JSON structure with a "message" key, the value
associated with that key will be included in the exception message
and in the build plugin output error.

Fixes gh-21515
  • Loading branch information
scottfrederick committed Jun 3, 2020
1 parent 1486ce5 commit 2925326
Show file tree
Hide file tree
Showing 7 changed files with 202 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.net.URI;

import org.springframework.util.Assert;
import org.springframework.util.StringUtils;

/**
* Exception thrown when a call to the Docker API fails.
Expand All @@ -35,11 +36,15 @@ public class DockerEngineException extends RuntimeException {

private final Errors errors;

DockerEngineException(String host, URI uri, int statusCode, String reasonPhrase, Errors errors) {
super(buildMessage(host, uri, statusCode, reasonPhrase, errors));
private final Message responseMessage;

DockerEngineException(String host, URI uri, int statusCode, String reasonPhrase, Errors errors,
Message responseMessage) {
super(buildMessage(host, uri, statusCode, reasonPhrase, errors, responseMessage));
this.statusCode = statusCode;
this.reasonPhrase = reasonPhrase;
this.errors = errors;
this.responseMessage = responseMessage;
}

/**
Expand All @@ -51,32 +56,45 @@ public int getStatusCode() {
}

/**
* Return the reason phrase returned by the Docker API error.
* Return the reason phrase returned by the Docker API.
* @return the reasonPhrase
*/
public String getReasonPhrase() {
return this.reasonPhrase;
}

/**
* Return the Errors from the body of the Docker API error, or {@code null} if the
* error JSON could not be read.
* Return the errors from the body of the Docker API response, or {@code null} if the
* errors JSON could not be read.
* @return the errors or {@code null}
*/
public Errors getErrors() {
return this.errors;
}

private static String buildMessage(String host, URI uri, int statusCode, String reasonPhrase, Errors errors) {
Assert.notNull(host, "host must not be null");
/**
* Return the message from the body of the Docker API response, or {@code null} if the
* message JSON could not be read.
* @return the message or {@code null}
*/
public Message getResponseMessage() {
return this.responseMessage;
}

private static String buildMessage(String host, URI uri, int statusCode, String reasonPhrase, Errors errors,
Message responseMessage) {
Assert.notNull(host, "Host must not be null");
Assert.notNull(uri, "URI must not be null");
StringBuilder message = new StringBuilder(
"Docker API call to '" + host + uri + "' failed with status code " + statusCode);
if (reasonPhrase != null && !reasonPhrase.isEmpty()) {
message.append(" \"" + reasonPhrase + "\"");
if (!StringUtils.isEmpty(reasonPhrase)) {
message.append(" \"").append(reasonPhrase).append("\"");
}
if (responseMessage != null && !StringUtils.isEmpty(responseMessage.getMessage())) {
message.append(" and message \"").append(responseMessage.getMessage()).append("\"");
}
if (errors != null && !errors.isEmpty()) {
message.append(" " + errors);
message.append(" ").append(errors);
}
return message.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,9 @@ private Response execute(HttpUriRequest request) {
HttpEntity entity = response.getEntity();
if (statusCode >= 400 && statusCode <= 500) {
Errors errors = (statusCode != 500) ? getErrorsFromResponse(entity) : null;
Message message = getMessageFromResponse(entity);
throw new DockerEngineException(this.host.toHostString(), request.getURI(), statusCode,
statusLine.getReasonPhrase(), errors);
statusLine.getReasonPhrase(), errors, message);
}
return new HttpClientResponse(response);
}
Expand All @@ -150,6 +151,16 @@ private Errors getErrorsFromResponse(HttpEntity entity) {
}
}

private Message getMessageFromResponse(HttpEntity entity) {
try {
return (entity.getContent() != null)
? SharedObjectMapper.get().readValue(entity.getContent(), Message.class) : null;
}
catch (IOException ex) {
return null;
}
}

HttpHost getHost() {
return this.host;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* 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 com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;

/**
* A message returned from the Docker API.
*
* @author Scott Frederick
* @since 2.3.1
*/
public class Message {

private final String message;

@JsonCreator
Message(@JsonProperty("message") String message) {
this.message = message;
}

/**
* Return the message contained in the response.
* @return the message
*/
public String getMessage() {
return this.message;
}

@Override
public String toString() {
return this.message;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -49,55 +49,78 @@ class DockerEngineExceptionTests {

private static final Errors ERRORS = new Errors(Collections.singletonList(new Errors.Error("code", "message")));

private static final Message NO_MESSAGE = new Message(null);

private static final Message MESSAGE = new Message("response message");

@Test
void createWhenHostIsNullThrowsException() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new DockerEngineException(null, null, 404, null, NO_ERRORS))
.withMessage("host must not be null");
.isThrownBy(() -> new DockerEngineException(null, null, 404, null, NO_ERRORS, NO_MESSAGE))
.withMessage("Host must not be null");
}

@Test
void createWhenUriIsNullThrowsException() {
assertThatIllegalArgumentException()
.isThrownBy(() -> new DockerEngineException(HOST, null, 404, null, NO_ERRORS))
.isThrownBy(() -> new DockerEngineException(HOST, null, 404, null, NO_ERRORS, NO_MESSAGE))
.withMessage("URI must not be null");
}

@Test
void create() {
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, "missing", ERRORS);
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, "missing", ERRORS, MESSAGE);
assertThat(exception.getMessage()).isEqualTo(
"Docker API call to 'docker://localhost/example' failed with status code 404 \"missing\" [code: message]");
"Docker API call to 'docker://localhost/example' failed with status code 404 \"missing\" and message \"response message\" [code: message]");
assertThat(exception.getStatusCode()).isEqualTo(404);
assertThat(exception.getReasonPhrase()).isEqualTo("missing");
assertThat(exception.getErrors()).isSameAs(ERRORS);
assertThat(exception.getResponseMessage()).isSameAs(MESSAGE);
}

@Test
void createWhenReasonPhraseIsNull() {
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, null, ERRORS);
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, null, ERRORS, MESSAGE);
assertThat(exception.getMessage()).isEqualTo(
"Docker API call to 'docker://localhost/example' failed with status code 404 [code: message]");
"Docker API call to 'docker://localhost/example' failed with status code 404 and message \"response message\" [code: message]");
assertThat(exception.getStatusCode()).isEqualTo(404);
assertThat(exception.getReasonPhrase()).isNull();
assertThat(exception.getErrors()).isSameAs(ERRORS);
assertThat(exception.getResponseMessage()).isSameAs(MESSAGE);
}

@Test
void createWhenErrorsIsNull() {
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, "missing", null);
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, "missing", null, MESSAGE);
assertThat(exception.getMessage()).isEqualTo(
"Docker API call to 'docker://localhost/example' failed with status code 404 \"missing\" and message \"response message\"");
assertThat(exception.getErrors()).isNull();
}

@Test
void createWhenErrorsIsEmpty() {
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, "missing", NO_ERRORS);
assertThat(exception.getMessage())
.isEqualTo("Docker API call to 'docker://localhost/example' failed with status code 404 \"missing\"");
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, "missing", NO_ERRORS, MESSAGE);
assertThat(exception.getMessage()).isEqualTo(
"Docker API call to 'docker://localhost/example' failed with status code 404 \"missing\" and message \"response message\"");
assertThat(exception.getStatusCode()).isEqualTo(404);
assertThat(exception.getReasonPhrase()).isEqualTo("missing");
assertThat(exception.getErrors()).isSameAs(NO_ERRORS);
}

@Test
void createWhenMessageIsNull() {
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, "missing", ERRORS, null);
assertThat(exception.getMessage()).isEqualTo(
"Docker API call to 'docker://localhost/example' failed with status code 404 \"missing\" [code: message]");
assertThat(exception.getResponseMessage()).isNull();
}

@Test
void createWhenMessageIsEmpty() {
DockerEngineException exception = new DockerEngineException(HOST, URI, 404, "missing", ERRORS, NO_MESSAGE);
assertThat(exception.getMessage()).isEqualTo(
"Docker API call to 'docker://localhost/example' failed with status code 404 \"missing\" [code: message]");
assertThat(exception.getResponseMessage()).isSameAs(NO_MESSAGE);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,42 @@ void executeWhenResponseIsIn400RangeShouldThrowDockerException() throws IOExcept
given(this.entity.getContent()).willReturn(getClass().getResourceAsStream("errors.json"));
given(this.statusLine.getStatusCode()).willReturn(404);
assertThatExceptionOfType(DockerEngineException.class).isThrownBy(() -> this.http.get(this.uri))
.satisfies((ex) -> assertThat(ex.getErrors()).hasSize(2));
.satisfies((ex) -> {
assertThat(ex.getErrors()).hasSize(2);
assertThat(ex.getResponseMessage()).isNull();
});
}

@Test
void executeWhenResponseIsIn500RangeShouldThrowDockerException() {
void executeWhenResponseIsIn500RangeWithNoContentShouldThrowDockerException() {
given(this.statusLine.getStatusCode()).willReturn(500);
assertThatExceptionOfType(DockerEngineException.class).isThrownBy(() -> this.http.get(this.uri))
.satisfies((ex) -> assertThat(ex.getErrors()).isNull());
.satisfies((ex) -> {
assertThat(ex.getErrors()).isNull();
assertThat(ex.getResponseMessage()).isNull();
});
}

@Test
void executeWhenResponseIsIn500RangeWithMessageShouldThrowDockerException() throws IOException {
given(this.entity.getContent()).willReturn(getClass().getResourceAsStream("message.json"));
given(this.statusLine.getStatusCode()).willReturn(500);
assertThatExceptionOfType(DockerEngineException.class).isThrownBy(() -> this.http.get(this.uri))
.satisfies((ex) -> {
assertThat(ex.getErrors()).isNull();
assertThat(ex.getResponseMessage().getMessage()).contains("test message");
});
}

@Test
void executeWhenResponseIsIn500RangeWithOtherContentShouldThrowDockerException() throws IOException {
given(this.entity.getContent()).willReturn(this.content);
given(this.statusLine.getStatusCode()).willReturn(500);
assertThatExceptionOfType(DockerEngineException.class).isThrownBy(() -> this.http.get(this.uri))
.satisfies((ex) -> {
assertThat(ex.getErrors()).isNull();
assertThat(ex.getResponseMessage()).isNull();
});
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* 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.junit.jupiter.api.Test;

import org.springframework.boot.buildpack.platform.json.AbstractJsonTests;

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

/**
* Tests for {@link Message}.
*
* @author Scott Frederick
*/
class MessageTests extends AbstractJsonTests {

@Test
void readValueDeserializesJson() throws Exception {
Message message = getObjectMapper().readValue(getContent("message.json"), Message.class);
assertThat(message.getMessage()).isEqualTo("test message");
}

@Test
void toStringHasErrorDetails() throws Exception {
Message errors = getObjectMapper().readValue(getContent("message.json"), Message.class);
assertThat(errors.toString()).isEqualTo("test message");
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"message": "test message"
}

0 comments on commit 2925326

Please sign in to comment.