From 951f3f76afb08eb21642d10ba73a3c49ffb9862c Mon Sep 17 00:00:00 2001 From: Igor Dvorzhak Date: Fri, 13 Mar 2020 15:25:54 -0700 Subject: [PATCH 1/2] fix: include request URL into HttpResponseException message --- .../client/http/HttpResponseException.java | 7 + .../http/HttpResponseExceptionTest.java | 140 +++++++++++------- 2 files changed, 94 insertions(+), 53 deletions(-) diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java b/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java index 6e5349e18..27f45228f 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java @@ -284,6 +284,13 @@ public static StringBuilder computeMessageBuffer(HttpResponse response) { } builder.append(statusMessage); } + HttpRequest request = response.getRequest(); + if (request != null) { + if (builder.length() > 0) { + builder.append('\n'); + } + builder.append("Request URL: ").append(request.getUrl()); + } return builder; } } diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java index 4d651aecb..b81f49a0a 100644 --- a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java @@ -14,12 +14,15 @@ package com.google.api.client.http; +import static com.google.api.client.testing.http.HttpTesting.SIMPLE_GENERIC_URL; +import static com.google.api.client.util.StringUtils.LINE_SEPARATOR; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + import com.google.api.client.http.HttpResponseException.Builder; -import com.google.api.client.testing.http.HttpTesting; import com.google.api.client.testing.http.MockHttpTransport; import com.google.api.client.testing.http.MockLowLevelHttpRequest; import com.google.api.client.testing.http.MockLowLevelHttpResponse; -import com.google.api.client.util.StringUtils; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -27,6 +30,7 @@ import java.io.ObjectOutput; import java.io.ObjectOutputStream; import junit.framework.TestCase; +import org.junit.function.ThrowingRunnable; /** * Tests {@link HttpResponseException}. @@ -37,12 +41,11 @@ public class HttpResponseExceptionTest extends TestCase { public void testConstructor() throws Exception { HttpTransport transport = new MockHttpTransport(); - HttpRequest request = - transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); + HttpRequest request = transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); HttpResponse response = request.execute(); HttpHeaders headers = response.getHeaders(); HttpResponseException e = new HttpResponseException(response); - assertEquals("200", e.getMessage()); + assertThat(e).hasMessageThat().isEqualTo("200\nRequest URL: " + SIMPLE_GENERIC_URL); assertNull(e.getContent()); assertEquals(200, e.getStatusCode()); assertNull(e.getStatusMessage()); @@ -83,8 +86,7 @@ public LowLevelHttpResponse execute() throws IOException { }; } }; - HttpRequest request = - transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); + HttpRequest request = transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); HttpResponse response = request.execute(); HttpResponseException e = new HttpResponseException(response); assertEquals("OK", e.getStatusMessage()); @@ -105,14 +107,18 @@ public LowLevelHttpResponse execute() throws IOException { }; } }; - HttpRequest request = - transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); - try { - request.execute(); - fail(); - } catch (HttpResponseException e) { - assertEquals("", e.getMessage()); - } + final HttpRequest request = + transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); + HttpResponseException responseException = + assertThrows( + HttpResponseException.class, + new ThrowingRunnable() { + @Override + public void run() throws Throwable { + request.execute(); + } + }); + assertThat(responseException).hasMessageThat().isEqualTo("Request URL: " + SIMPLE_GENERIC_URL); } public void testConstructor_messageButNoStatusCode() throws Exception { @@ -131,14 +137,20 @@ public LowLevelHttpResponse execute() throws IOException { }; } }; - HttpRequest request = - transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); - try { - request.execute(); - fail(); - } catch (HttpResponseException e) { - assertEquals("Foo", e.getMessage()); - } + final HttpRequest request = + transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); + HttpResponseException responseException = + assertThrows( + HttpResponseException.class, + new ThrowingRunnable() { + @Override + public void run() throws Throwable { + request.execute(); + } + }); + assertThat(responseException) + .hasMessageThat() + .isEqualTo("Foo\nRequest URL: " + SIMPLE_GENERIC_URL); } public void testComputeMessage() throws Exception { @@ -156,10 +168,10 @@ public LowLevelHttpResponse execute() throws IOException { }; } }; - HttpRequest request = - transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); + HttpRequest request = transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); HttpResponse response = request.execute(); - assertEquals("200 Foo", HttpResponseException.computeMessageBuffer(response).toString()); + assertThat(HttpResponseException.computeMessageBuffer(response).toString()) + .isEqualTo("200 Foo\nRequest URL: " + SIMPLE_GENERIC_URL); } public void testThrown() throws Exception { @@ -179,15 +191,25 @@ public LowLevelHttpResponse execute() throws IOException { }; } }; - HttpRequest request = - transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); - try { - request.execute(); - fail(); - } catch (HttpResponseException e) { - assertEquals( - "404 Not Found" + StringUtils.LINE_SEPARATOR + "Unable to find resource", e.getMessage()); - } + final HttpRequest request = + transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); + HttpResponseException responseException = + assertThrows( + HttpResponseException.class, + new ThrowingRunnable() { + @Override + public void run() throws Throwable { + request.execute(); + } + }); + + assertThat(responseException) + .hasMessageThat() + .isEqualTo( + "404 Not Found\nRequest URL: " + + SIMPLE_GENERIC_URL + + LINE_SEPARATOR + + "Unable to find resource"); } public void testInvalidCharset() throws Exception { @@ -208,14 +230,21 @@ public LowLevelHttpResponse execute() throws IOException { }; } }; - HttpRequest request = - transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); - try { - request.execute(); - fail(); - } catch (HttpResponseException e) { - assertEquals("404 Not Found", e.getMessage()); - } + final HttpRequest request = + transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); + HttpResponseException responseException = + assertThrows( + HttpResponseException.class, + new ThrowingRunnable() { + @Override + public void run() throws Throwable { + request.execute(); + } + }); + + assertThat(responseException) + .hasMessageThat() + .isEqualTo("404 Not Found\nRequest URL: " + SIMPLE_GENERIC_URL); } public void testUnsupportedCharset() throws Exception { @@ -236,20 +265,25 @@ public LowLevelHttpResponse execute() throws IOException { }; } }; - HttpRequest request = - transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); - try { - request.execute(); - fail(); - } catch (HttpResponseException e) { - assertEquals("404 Not Found", e.getMessage()); - } + final HttpRequest request = + transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); + HttpResponseException responseException = + assertThrows( + HttpResponseException.class, + new ThrowingRunnable() { + @Override + public void run() throws Throwable { + request.execute(); + } + }); + assertThat(responseException) + .hasMessageThat() + .isEqualTo("404 Not Found\nRequest URL: " + SIMPLE_GENERIC_URL); } public void testSerialization() throws Exception { HttpTransport transport = new MockHttpTransport(); - HttpRequest request = - transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); + HttpRequest request = transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); HttpResponse response = request.execute(); HttpResponseException e = new HttpResponseException(response); ByteArrayOutputStream out = new ByteArrayOutputStream(); From e02d10c9bf2657af9cb89ab550888e5da1eea88a Mon Sep 17 00:00:00 2001 From: Igor Dvorzhak Date: Sun, 15 Mar 2020 15:12:01 -0700 Subject: [PATCH 2/2] Add request method to the exception message + review fixes --- .../client/http/HttpResponseException.java | 6 ++- .../http/HttpResponseExceptionTest.java | 38 +++++++++---------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java b/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java index 27f45228f..a9d80a4f5 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java @@ -289,7 +289,11 @@ public static StringBuilder computeMessageBuffer(HttpResponse response) { if (builder.length() > 0) { builder.append('\n'); } - builder.append("Request URL: ").append(request.getUrl()); + String requestMethod = request.getRequestMethod(); + if (requestMethod != null) { + builder.append(requestMethod).append(' '); + } + builder.append(request.getUrl()); } return builder; } diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java index b81f49a0a..9066e9d70 100644 --- a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java @@ -44,12 +44,12 @@ public void testConstructor() throws Exception { HttpRequest request = transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); HttpResponse response = request.execute(); HttpHeaders headers = response.getHeaders(); - HttpResponseException e = new HttpResponseException(response); - assertThat(e).hasMessageThat().isEqualTo("200\nRequest URL: " + SIMPLE_GENERIC_URL); - assertNull(e.getContent()); - assertEquals(200, e.getStatusCode()); - assertNull(e.getStatusMessage()); - assertTrue(headers == e.getHeaders()); + HttpResponseException responseException = new HttpResponseException(response); + assertThat(responseException).hasMessageThat().isEqualTo("200\nGET " + SIMPLE_GENERIC_URL); + assertNull(responseException.getContent()); + assertEquals(200, responseException.getStatusCode()); + assertNull(responseException.getStatusMessage()); + assertTrue(headers == responseException.getHeaders()); } public void testBuilder() throws Exception { @@ -88,8 +88,8 @@ public LowLevelHttpResponse execute() throws IOException { }; HttpRequest request = transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); HttpResponse response = request.execute(); - HttpResponseException e = new HttpResponseException(response); - assertEquals("OK", e.getStatusMessage()); + HttpResponseException responseException = new HttpResponseException(response); + assertEquals("OK", responseException.getStatusMessage()); } public void testConstructor_noStatusCode() throws Exception { @@ -118,7 +118,7 @@ public void run() throws Throwable { request.execute(); } }); - assertThat(responseException).hasMessageThat().isEqualTo("Request URL: " + SIMPLE_GENERIC_URL); + assertThat(responseException).hasMessageThat().isEqualTo("GET " + SIMPLE_GENERIC_URL); } public void testConstructor_messageButNoStatusCode() throws Exception { @@ -148,9 +148,7 @@ public void run() throws Throwable { request.execute(); } }); - assertThat(responseException) - .hasMessageThat() - .isEqualTo("Foo\nRequest URL: " + SIMPLE_GENERIC_URL); + assertThat(responseException).hasMessageThat().isEqualTo("Foo\nGET " + SIMPLE_GENERIC_URL); } public void testComputeMessage() throws Exception { @@ -171,7 +169,7 @@ public LowLevelHttpResponse execute() throws IOException { HttpRequest request = transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); HttpResponse response = request.execute(); assertThat(HttpResponseException.computeMessageBuffer(response).toString()) - .isEqualTo("200 Foo\nRequest URL: " + SIMPLE_GENERIC_URL); + .isEqualTo("200 Foo\nGET " + SIMPLE_GENERIC_URL); } public void testThrown() throws Exception { @@ -206,7 +204,7 @@ public void run() throws Throwable { assertThat(responseException) .hasMessageThat() .isEqualTo( - "404 Not Found\nRequest URL: " + "404 Not Found\nGET " + SIMPLE_GENERIC_URL + LINE_SEPARATOR + "Unable to find resource"); @@ -244,7 +242,7 @@ public void run() throws Throwable { assertThat(responseException) .hasMessageThat() - .isEqualTo("404 Not Found\nRequest URL: " + SIMPLE_GENERIC_URL); + .isEqualTo("404 Not Found\nGET " + SIMPLE_GENERIC_URL); } public void testUnsupportedCharset() throws Exception { @@ -278,22 +276,22 @@ public void run() throws Throwable { }); assertThat(responseException) .hasMessageThat() - .isEqualTo("404 Not Found\nRequest URL: " + SIMPLE_GENERIC_URL); + .isEqualTo("404 Not Found\nGET " + SIMPLE_GENERIC_URL); } public void testSerialization() throws Exception { HttpTransport transport = new MockHttpTransport(); HttpRequest request = transport.createRequestFactory().buildGetRequest(SIMPLE_GENERIC_URL); HttpResponse response = request.execute(); - HttpResponseException e = new HttpResponseException(response); + HttpResponseException responseException = new HttpResponseException(response); ByteArrayOutputStream out = new ByteArrayOutputStream(); ObjectOutput s = new ObjectOutputStream(out); - s.writeObject(e); + s.writeObject(responseException); ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray()); ObjectInputStream objectInput = new ObjectInputStream(in); HttpResponseException e2 = (HttpResponseException) objectInput.readObject(); - assertEquals(e.getMessage(), e2.getMessage()); - assertEquals(e.getStatusCode(), e2.getStatusCode()); + assertEquals(responseException.getMessage(), e2.getMessage()); + assertEquals(responseException.getStatusCode(), e2.getStatusCode()); assertNull(e2.getHeaders()); } }