From 7219383b851f9fb08b098bc99be9242895f2b587 Mon Sep 17 00:00:00 2001 From: Mike Smithson Date: Fri, 21 Feb 2020 18:29:18 -0500 Subject: [PATCH 1/3] Issue #20151 Improve error message when creating an image with no Docker Daemon available. Translating IOException to DockerException for a more meaningful error message. formatting according to project formatting standards removing local change --- .../platform/docker/HttpClientHttp.java | 22 ++++++++----------- .../platform/docker/HttpClientHttpTests.java | 9 ++++---- 2 files changed, 13 insertions(+), 18 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 011ae9dee740..e1ecf6017fcd 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 @@ -36,7 +36,6 @@ 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; @@ -136,19 +135,16 @@ private Response execute(HttpUriRequest request) throws IOException { 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); - } - catch (Exception ex) { - } + Errors errors = SharedObjectMapper.get().readValue(entity.getContent(), Errors.class); + throw new DockerException(request.getURI(), statusCode, statusLine.getReasonPhrase(), errors); } - EntityUtils.consume(entity); - throw new DockerException(request.getURI(), statusCode, statusLine.getReasonPhrase(), errors); + if (statusCode == 500) { + throw new DockerException(request.getURI(), statusCode, statusLine.getReasonPhrase(), null); + } + + return new HttpClientResponse(response); } /** @@ -175,7 +171,7 @@ public long getContentLength() { } @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 50463611c905..2fe8719a7faf 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.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; @@ -132,7 +131,7 @@ void postWithContentShouldExecuteHttpPost() throws Exception { 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 +151,7 @@ void putWithContentShouldExecuteHttpPut() throws Exception { 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 +170,7 @@ void deleteShouldExecuteHttpDelete() throws IOException { } @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,7 +178,7 @@ void executeWhenResposeIsIn400RangeShouldThrowDockerException() throws ClientPro } @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()); From 0a9064a9bd11010b32b85cd007f05a2f02cf9790 Mon Sep 17 00:00:00 2001 From: Mike Smithson Date: Fri, 21 Feb 2020 20:58:37 -0500 Subject: [PATCH 2/3] adding author info --- .../boot/buildpack/platform/docker/HttpClientHttp.java | 1 + .../boot/buildpack/platform/docker/HttpClientHttpTests.java | 1 + 2 files changed, 2 insertions(+) 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 e1ecf6017fcd..8158309fe34e 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 @@ -45,6 +45,7 @@ * {@link Http} implementation backed by a {@link HttpClient}. * * @author Phillip Webb + * @author Mike Smithson */ class HttpClientHttp implements Http { 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 2fe8719a7faf..7261e38cd085 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 @@ -53,6 +53,7 @@ * Tests for {@link HttpClientHttp}. * * @author Phillip Webb + * @author Mike Smithson */ class HttpClientHttpTests { From 3f3fb62b73a700a358bbcaf5ab9eca8b8182f06c Mon Sep 17 00:00:00 2001 From: Mike Smithson Date: Sat, 22 Feb 2020 22:37:25 -0500 Subject: [PATCH 3/3] Issue #20151 Improve error messaging when no Docker daemon available. Rethrowing IOException as DockerException when executing an Http request and adding test to verify behavior. --- .../platform/docker/HttpClientHttp.java | 50 +++++++++++-------- .../platform/docker/HttpClientHttpTests.java | 8 +++ 2 files changed, 36 insertions(+), 22 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 8158309fe34e..1446f603878e 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 @@ 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; @@ -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 @@ public Response get(URI uri) throws IOException { * 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 @@ public Response post(URI uri) throws IOException { * @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 @@ public Response post(URI uri, String contentType, IOConsumer write * @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,33 +114,41 @@ public Response put(URI uri, String contentType, IOConsumer writer * 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 >= 400 && statusCode < 500) { - Errors errors = SharedObjectMapper.get().readValue(entity.getContent(), Errors.class); - throw new DockerException(request.getURI(), statusCode, statusLine.getReasonPhrase(), errors); + 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); + } + if (statusCode == 500) { + throw new DockerException(request.getURI(), statusCode, statusLine.getReasonPhrase(), null); + } } - if (statusCode == 500) { - throw new DockerException(request.getURI(), statusCode, statusLine.getReasonPhrase(), null); + 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); 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 7261e38cd085..7ea9d2d6a14c 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 @@ -185,6 +185,14 @@ void executeWhenResposeIsIn500RangeShouldThrowDockerException() { .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);