From 6f095d6fec31a3032ae1aa4afbd43f297e9394f1 Mon Sep 17 00:00:00 2001 From: Mike Smithson Date: Fri, 21 Feb 2020 18:29:18 -0500 Subject: [PATCH] Improve error message from image building Translate IOException to DockerException for a more meaningful error message when the Docker daemon is not available. Fixes gh-20151 --- .../platform/docker/HttpClientHttp.java | 59 ++++++++++--------- .../platform/docker/HttpClientHttpTests.java | 18 ++++-- 2 files changed, 44 insertions(+), 33 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/HttpClientHttp.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/HttpClientHttp.java index 011ae9dee74..1446f603878 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/HttpClientHttp.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/HttpClientHttp.java @@ -19,6 +19,8 @@ package org.springframework.boot.buildpack.platform.docker; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.PrintWriter; +import java.io.StringWriter; import java.net.URI; import org.apache.http.HttpEntity; @@ -36,7 +38,6 @@ import org.apache.http.entity.AbstractHttpEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.HttpClients; -import org.apache.http.util.EntityUtils; import org.springframework.boot.buildpack.platform.io.Content; import org.springframework.boot.buildpack.platform.io.IOConsumer; @@ -46,6 +47,7 @@ import org.springframework.boot.buildpack.platform.json.SharedObjectMapper; * {@link Http} implementation backed by a {@link HttpClient}. * * @author Phillip Webb + * @author Mike Smithson */ class HttpClientHttp implements Http { @@ -66,10 +68,9 @@ class HttpClientHttp implements Http { * Perform a HTTP GET operation. * @param uri the destination URI * @return the operation response - * @throws IOException on IO error */ @Override - public Response get(URI uri) throws IOException { + public Response get(URI uri) { return execute(new HttpGet(uri)); } @@ -77,10 +78,9 @@ class HttpClientHttp implements Http { * Perform a HTTP POST operation. * @param uri the destination URI * @return the operation response - * @throws IOException on IO error */ @Override - public Response post(URI uri) throws IOException { + public Response post(URI uri) { return execute(new HttpPost(uri)); } @@ -90,11 +90,10 @@ class HttpClientHttp implements Http { * @param contentType the content type to write * @param writer a content writer * @return the operation response - * @throws IOException on IO error */ @Override - public Response post(URI uri, String contentType, IOConsumer writer) throws IOException { + public Response post(URI uri, String contentType, IOConsumer writer) { return execute(new HttpPost(uri), contentType, writer); } @@ -104,11 +103,10 @@ class HttpClientHttp implements Http { * @param contentType the content type to write * @param writer a content writer * @return the operation response - * @throws IOException on IO error */ @Override - public Response put(URI uri, String contentType, IOConsumer writer) throws IOException { + public Response put(URI uri, String contentType, IOConsumer writer) { return execute(new HttpPut(uri), contentType, writer); } @@ -116,39 +114,44 @@ class HttpClientHttp implements Http { * Perform a HTTP DELETE operation. * @param uri the destination URI * @return the operation response - * @throws IOException on IO error */ @Override - public Response delete(URI uri) throws IOException { + public Response delete(URI uri) { return execute(new HttpDelete(uri)); } private Response execute(HttpEntityEnclosingRequestBase request, String contentType, - IOConsumer writer) throws IOException { + IOConsumer writer) { request.setHeader(HttpHeaders.CONTENT_TYPE, contentType); request.setEntity(new WritableHttpEntity(writer)); return execute(request); } - private Response execute(HttpUriRequest request) throws IOException { - CloseableHttpResponse response = this.client.execute(request); - StatusLine statusLine = response.getStatusLine(); - int statusCode = statusLine.getStatusCode(); - HttpEntity entity = response.getEntity(); - if (statusCode >= 200 && statusCode < 300) { - return new HttpClientResponse(response); - } - Errors errors = null; - if (statusCode >= 400 && statusCode < 500) { - try { - errors = SharedObjectMapper.get().readValue(entity.getContent(), Errors.class); + private Response execute(HttpUriRequest request) { + CloseableHttpResponse response; + try { + response = this.client.execute(request); + StatusLine statusLine = response.getStatusLine(); + int statusCode = statusLine.getStatusCode(); + HttpEntity entity = response.getEntity(); + + if (statusCode >= 400 && statusCode < 500) { + Errors errors = SharedObjectMapper.get().readValue(entity.getContent(), Errors.class); + throw new DockerException(request.getURI(), statusCode, statusLine.getReasonPhrase(), errors); } - catch (Exception ex) { + if (statusCode == 500) { + throw new DockerException(request.getURI(), statusCode, statusLine.getReasonPhrase(), null); } } - EntityUtils.consume(entity); - throw new DockerException(request.getURI(), statusCode, statusLine.getReasonPhrase(), errors); + catch (IOException ioe) { + StringWriter stringWriter = new StringWriter(); + PrintWriter printWriter = new PrintWriter(stringWriter); + ioe.printStackTrace(printWriter); + throw new DockerException(request.getURI(), 500, stringWriter.toString(), null); + } + + return new HttpClientResponse(response); } /** @@ -175,7 +178,7 @@ class HttpClientHttp implements Http { } @Override - public InputStream getContent() throws IOException, UnsupportedOperationException { + public InputStream getContent() throws UnsupportedOperationException { throw new UnsupportedOperationException(); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/HttpClientHttpTests.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/HttpClientHttpTests.java index 50463611c90..7ea9d2d6a14 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/HttpClientHttpTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/HttpClientHttpTests.java @@ -26,7 +26,6 @@ import org.apache.http.HttpEntity; import org.apache.http.HttpEntityEnclosingRequest; import org.apache.http.HttpHeaders; import org.apache.http.StatusLine; -import org.apache.http.client.ClientProtocolException; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpDelete; import org.apache.http.client.methods.HttpGet; @@ -54,6 +53,7 @@ import static org.mockito.Mockito.verify; * Tests for {@link HttpClientHttp}. * * @author Phillip Webb + * @author Mike Smithson */ class HttpClientHttpTests { @@ -132,7 +132,7 @@ class HttpClientHttpTests { assertThat(entity.isRepeatable()).isFalse(); assertThat(entity.getContentLength()).isEqualTo(-1); assertThat(entity.isStreaming()).isTrue(); - assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> entity.getContent()); + assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(entity::getContent); assertThat(writeToString(entity)).isEqualTo("test"); assertThat(response.getContent()).isSameAs(this.content); } @@ -152,7 +152,7 @@ class HttpClientHttpTests { assertThat(entity.isRepeatable()).isFalse(); assertThat(entity.getContentLength()).isEqualTo(-1); assertThat(entity.isStreaming()).isTrue(); - assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> entity.getContent()); + assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(entity::getContent); assertThat(writeToString(entity)).isEqualTo("test"); assertThat(response.getContent()).isSameAs(this.content); } @@ -171,7 +171,7 @@ class HttpClientHttpTests { } @Test - void executeWhenResposeIsIn400RangeShouldThrowDockerException() throws ClientProtocolException, IOException { + void executeWhenResposeIsIn400RangeShouldThrowDockerException() throws IOException { given(this.entity.getContent()).willReturn(getClass().getResourceAsStream("errors.json")); given(this.statusLine.getStatusCode()).willReturn(404); assertThatExceptionOfType(DockerException.class).isThrownBy(() -> this.http.get(this.uri)) @@ -179,12 +179,20 @@ class HttpClientHttpTests { } @Test - void executeWhenResposeIsIn500RangeShouldThrowDockerException() throws ClientProtocolException, IOException { + void executeWhenResposeIsIn500RangeShouldThrowDockerException() { given(this.statusLine.getStatusCode()).willReturn(500); assertThatExceptionOfType(DockerException.class).isThrownBy(() -> this.http.get(this.uri)) .satisfies((ex) -> assertThat(ex.getErrors()).isNull()); } + @Test + void executeWhenClientExecutesRequestThrowsIOExceptionRethrowsAsDockerException() throws IOException { + given(this.client.execute(any())).willThrow(IOException.class); + assertThatExceptionOfType(DockerException.class).isThrownBy(() -> this.http.get(this.uri)) + .satisfies((ex) -> assertThat(ex.getErrors()).isNull()).satisfies(DockerException::getStatusCode) + .withMessageContaining("500").satisfies((ex) -> assertThat(ex.getReasonPhrase())).isNotNull(); + } + private String writeToString(HttpEntity entity) throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); entity.writeTo(out);