From 15111a1001d6f72cb92cd2d76aaed6f1229bc14a Mon Sep 17 00:00:00 2001 From: Igor Dvorzhak Date: Mon, 16 Mar 2020 03:07:48 -0700 Subject: [PATCH] fix: include request method and URL into HttpResponseException message (#1002) * fix: include request URL into HttpResponseException message * Add request method to the exception message + review fixes --- .../client/http/HttpResponseException.java | 11 ++ .../http/HttpResponseExceptionTest.java | 160 +++++++++++------- 2 files changed, 107 insertions(+), 64 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..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 @@ -284,6 +284,17 @@ public static StringBuilder computeMessageBuffer(HttpResponse response) { } builder.append(statusMessage); } + HttpRequest request = response.getRequest(); + if (request != null) { + if (builder.length() > 0) { + builder.append('\n'); + } + 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 4d651aecb..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 @@ -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,16 +41,15 @@ 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()); - 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 { @@ -83,11 +86,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(); - HttpResponseException e = new HttpResponseException(response); - assertEquals("OK", e.getStatusMessage()); + HttpResponseException responseException = new HttpResponseException(response); + assertEquals("OK", responseException.getStatusMessage()); } public void testConstructor_noStatusCode() throws Exception { @@ -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("GET " + SIMPLE_GENERIC_URL); } public void testConstructor_messageButNoStatusCode() throws Exception { @@ -131,14 +137,18 @@ 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\nGET " + SIMPLE_GENERIC_URL); } public void testComputeMessage() throws Exception { @@ -156,10 +166,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\nGET " + SIMPLE_GENERIC_URL); } public void testThrown() throws Exception { @@ -179,15 +189,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\nGET " + + SIMPLE_GENERIC_URL + + LINE_SEPARATOR + + "Unable to find resource"); } public void testInvalidCharset() throws Exception { @@ -208,14 +228,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\nGET " + SIMPLE_GENERIC_URL); } public void testUnsupportedCharset() throws Exception { @@ -236,30 +263,35 @@ 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\nGET " + 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); + 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()); } }