Skip to content

Commit

Permalink
fix: include request method and URL into HttpResponseException message (
Browse files Browse the repository at this point in the history 
#1002)

* fix: include request URL into HttpResponseException message
* Add request method to the exception message + review fixes
  • Loading branch information
@medb
medb authored Mar 16, 2020
1 parent 027c227 commit 15111a1
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 64 deletions.
11 changes: 11 additions & 0 deletions google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
160 changes: 96 additions & 64 deletions google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,23 @@

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;
import java.io.ObjectInputStream;
import java.io.ObjectOutput;
import java.io.ObjectOutputStream;
import junit.framework.TestCase;
import org.junit.function.ThrowingRunnable;

/**
* Tests {@link HttpResponseException}.
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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());
}
}

0 comments on commit 15111a1

Please sign in to comment.