Repository: googleapis/google-http-java-client
Pull request #1002 (2020-03-14 00:49:33)

fix: include request method and URL into HttpResponseException message
This will allow to simplify debugging of failed requests, especially requests that are issued by [`MediaHttpUploader`](https://github.com/googleapis/google-api-java-client/blob/master/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpUploader.java) (not created directly by library clients).

For example, it's useful to have an access to `upload_id` for failed resumable upload requests.


Patch:
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 6e5349e1..a9d80a4f 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 class HttpResponseException extends IOException {
       }
       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;
   }
 }


Test patch:
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 4d651aec..9066e9d7 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.ObjectInputStream;
 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 class HttpResponseExceptionTest extends TestCase {
             };
           }
         };
-    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 class HttpResponseExceptionTest extends TestCase {
             };
           }
         };
-    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 class HttpResponseExceptionTest extends TestCase {
             };
           }
         };
-    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 class HttpResponseExceptionTest extends TestCase {
             };
           }
         };
-    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 class HttpResponseExceptionTest extends TestCase {
             };
           }
         };
-    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 class HttpResponseExceptionTest extends TestCase {
             };
           }
         };
-    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 class HttpResponseExceptionTest extends TestCase {
             };
           }
         };
-    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());
   }
 }