Skip to content

Commit 3bff49b

Browse files
Remove status message support from HTTP client
Messages are not in HTTP/2 and are poorly supported in servlets.
1 parent ff25545 commit 3bff49b

File tree

16 files changed

+17
-112
lines changed

16 files changed

+17
-112
lines changed

event/src/main/java/com/facebook/airlift/event/client/HttpEventClient.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -169,13 +169,12 @@ public Void handle(Request request, Response response)
169169
try {
170170
InputStream inputStream = response.getInputStream();
171171
String responseBody = CharStreams.toString(new InputStreamReader(inputStream));
172-
log.debug("Posting event to %s failed: status_code=%d status_line=%s body=%s", request.getUri(), statusCode, response.getStatusMessage(), responseBody);
172+
log.debug("Posting event to %s failed: status_code=%d body=%s", request.getUri(), statusCode, responseBody);
173173
}
174174
catch (IOException bodyError) {
175-
log.debug("Posting event to %s failed: status_code=%d status_line=%s error=%s",
175+
log.debug("Posting event to %s failed: status_code=%d error=%s",
176176
request.getUri(),
177177
statusCode,
178-
response.getStatusMessage(),
179178
bodyError.getMessage());
180179
}
181180
return null;

http-client/src/main/java/com/facebook/airlift/http/client/FullJsonResponseHandler.java

+4-13
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ public JsonResponse<T> handle(Request request, Response response)
6464
byte[] bytes = readResponseBytes(response);
6565
String contentType = response.getHeader(CONTENT_TYPE);
6666
if ((contentType == null) || !MediaType.parse(contentType).is(MEDIA_TYPE_JSON)) {
67-
return new JsonResponse<>(response.getStatusCode(), response.getStatusMessage(), response.getHeaders(), bytes);
67+
return new JsonResponse<>(response.getStatusCode(), response.getHeaders(), bytes);
6868
}
69-
return new JsonResponse<>(response.getStatusCode(), response.getStatusMessage(), response.getHeaders(), jsonCodec, bytes);
69+
return new JsonResponse<>(response.getStatusCode(), response.getHeaders(), jsonCodec, bytes);
7070
}
7171

7272
private static byte[] readResponseBytes(Response response)
@@ -82,18 +82,16 @@ private static byte[] readResponseBytes(Response response)
8282
public static class JsonResponse<T>
8383
{
8484
private final int statusCode;
85-
private final String statusMessage;
8685
private final ListMultimap<HeaderName, String> headers;
8786
private final boolean hasValue;
8887
private final byte[] jsonBytes;
8988
private final byte[] responseBytes;
9089
private final T value;
9190
private final IllegalArgumentException exception;
9291

93-
public JsonResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers, byte[] responseBytes)
92+
public JsonResponse(int statusCode, ListMultimap<HeaderName, String> headers, byte[] responseBytes)
9493
{
9594
this.statusCode = statusCode;
96-
this.statusMessage = statusMessage;
9795
this.headers = ImmutableListMultimap.copyOf(headers);
9896

9997
this.hasValue = false;
@@ -104,10 +102,9 @@ public JsonResponse(int statusCode, String statusMessage, ListMultimap<HeaderNam
104102
}
105103

106104
@SuppressWarnings("ThrowableInstanceNeverThrown")
107-
public JsonResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers, JsonCodec<T> jsonCodec, byte[] jsonBytes)
105+
public JsonResponse(int statusCode, ListMultimap<HeaderName, String> headers, JsonCodec<T> jsonCodec, byte[] jsonBytes)
108106
{
109107
this.statusCode = statusCode;
110-
this.statusMessage = statusMessage;
111108
this.headers = ImmutableListMultimap.copyOf(headers);
112109

113110
this.jsonBytes = requireNonNull(jsonBytes, "jsonBytes is null");
@@ -131,11 +128,6 @@ public int getStatusCode()
131128
return statusCode;
132129
}
133130

134-
public String getStatusMessage()
135-
{
136-
return statusMessage;
137-
}
138-
139131
@Nullable
140132
public String getHeader(String name)
141133
{
@@ -201,7 +193,6 @@ public String toString()
201193
{
202194
return toStringHelper(this)
203195
.add("statusCode", statusCode)
204-
.add("statusMessage", statusMessage)
205196
.add("headers", headers)
206197
.add("hasValue", hasValue)
207198
.add("value", value)

http-client/src/main/java/com/facebook/airlift/http/client/JsonResponseHandler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public T handle(Request request, Response response)
6868
{
6969
if (!successfulResponseCodes.contains(response.getStatusCode())) {
7070
throw new UnexpectedResponseException(
71-
String.format("Expected response code to be %s, but was %d: %s", successfulResponseCodes, response.getStatusCode(), response.getStatusMessage()),
71+
String.format("Expected response code to be %s, but was %d", successfulResponseCodes, response.getStatusCode()),
7272
request,
7373
response);
7474
}

http-client/src/main/java/com/facebook/airlift/http/client/Response.java

-2
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ public interface Response
2929
{
3030
int getStatusCode();
3131

32-
String getStatusMessage();
33-
3432
@Nullable
3533
default String getHeader(String name)
3634
{

http-client/src/main/java/com/facebook/airlift/http/client/StatusResponseHandler.java

+2-9
Original file line numberDiff line numberDiff line change
@@ -48,19 +48,17 @@ public StatusResponse handleException(Request request, Exception exception)
4848
@Override
4949
public StatusResponse handle(Request request, Response response)
5050
{
51-
return new StatusResponse(response.getStatusCode(), response.getStatusMessage(), response.getHeaders());
51+
return new StatusResponse(response.getStatusCode(), response.getHeaders());
5252
}
5353

5454
public static class StatusResponse
5555
{
5656
private final int statusCode;
57-
private final String statusMessage;
5857
private final ListMultimap<HeaderName, String> headers;
5958

60-
public StatusResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers)
59+
public StatusResponse(int statusCode, ListMultimap<HeaderName, String> headers)
6160
{
6261
this.statusCode = statusCode;
63-
this.statusMessage = statusMessage;
6462
this.headers = ImmutableListMultimap.copyOf(headers);
6563
}
6664

@@ -69,11 +67,6 @@ public int getStatusCode()
6967
return statusCode;
7068
}
7169

72-
public String getStatusMessage()
73-
{
74-
return statusMessage;
75-
}
76-
7770
@Nullable
7871
public String getHeader(String name)
7972
{

http-client/src/main/java/com/facebook/airlift/http/client/StringResponseHandler.java

+1-10
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,12 @@ public StringResponse handle(Request request, Response response)
6161
MediaType mediaType = MediaType.parse(contentType);
6262
return new StringResponse(
6363
response.getStatusCode(),
64-
response.getStatusMessage(),
6564
response.getHeaders(),
6665
new String(ByteStreams.toByteArray(response.getInputStream()), mediaType.charset().or(UTF_8)));
6766
}
6867

6968
return new StringResponse(
7069
response.getStatusCode(),
71-
response.getStatusMessage(),
7270
response.getHeaders(),
7371
new String(ByteStreams.toByteArray(response.getInputStream()), UTF_8));
7472
}
@@ -80,14 +78,12 @@ public StringResponse handle(Request request, Response response)
8078
public static class StringResponse
8179
{
8280
private final int statusCode;
83-
private final String statusMessage;
8481
private final ListMultimap<HeaderName, String> headers;
8582
private final String body;
8683

87-
public StringResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers, String body)
84+
public StringResponse(int statusCode, ListMultimap<HeaderName, String> headers, String body)
8885
{
8986
this.statusCode = statusCode;
90-
this.statusMessage = statusMessage;
9187
this.headers = ImmutableListMultimap.copyOf(headers);
9288
this.body = body;
9389
}
@@ -97,11 +93,6 @@ public int getStatusCode()
9793
return statusCode;
9894
}
9995

100-
public String getStatusMessage()
101-
{
102-
return statusMessage;
103-
}
104-
10596
public String getBody()
10697
{
10798
return body;

http-client/src/main/java/com/facebook/airlift/http/client/UnexpectedResponseException.java

+2-12
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,13 @@ public class UnexpectedResponseException
3131
{
3232
private final Request request;
3333
private final int statusCode;
34-
private final String statusMessage;
3534
private final ListMultimap<HeaderName, String> headers;
3635

3736
public UnexpectedResponseException(Request request, Response response)
3837
{
39-
this(String.format("%d: %s", response.getStatusCode(), response.getStatusMessage()),
38+
this(String.format("%d", response.getStatusCode()),
4039
request,
4140
response.getStatusCode(),
42-
response.getStatusMessage(),
4341
ImmutableListMultimap.copyOf(response.getHeaders()));
4442
}
4543

@@ -48,16 +46,14 @@ public UnexpectedResponseException(String message, Request request, Response res
4846
this(message,
4947
request,
5048
response.getStatusCode(),
51-
response.getStatusMessage(),
5249
ImmutableListMultimap.copyOf(response.getHeaders()));
5350
}
5451

55-
public UnexpectedResponseException(String message, Request request, int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers)
52+
public UnexpectedResponseException(String message, Request request, int statusCode, ListMultimap<HeaderName, String> headers)
5653
{
5754
super(message);
5855
this.request = request;
5956
this.statusCode = statusCode;
60-
this.statusMessage = statusMessage;
6157
this.headers = ImmutableListMultimap.copyOf(headers);
6258
}
6359

@@ -66,11 +62,6 @@ public int getStatusCode()
6662
return statusCode;
6763
}
6864

69-
public String getStatusMessage()
70-
{
71-
return statusMessage;
72-
}
73-
7465
@Nullable
7566
public String getHeader(String name)
7667
{
@@ -94,7 +85,6 @@ public String toString()
9485
return toStringHelper(this)
9586
.add("request", request)
9687
.add("statusCode", statusCode)
97-
.add("statusMessage", statusMessage)
9888
.add("headers", headers)
9989
.toString();
10090
}

http-client/src/main/java/com/facebook/airlift/http/client/jetty/JettyResponse.java

-7
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,6 @@ public int getStatusCode()
3131
return response.getStatus();
3232
}
3333

34-
@Override
35-
public String getStatusMessage()
36-
{
37-
return response.getReason();
38-
}
39-
4034
@Override
4135
public ListMultimap<HeaderName, String> getHeaders()
4236
{
@@ -60,7 +54,6 @@ public String toString()
6054
{
6155
return toStringHelper(this)
6256
.add("statusCode", getStatusCode())
63-
.add("statusMessage", getStatusMessage())
6457
.add("headers", getHeaders())
6558
.toString();
6659
}

http-client/src/main/java/com/facebook/airlift/http/client/testing/TestingResponse.java

-7
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,6 @@ public int getStatusCode()
4343
return status.code();
4444
}
4545

46-
@Override
47-
public String getStatusMessage()
48-
{
49-
return status.reason();
50-
}
51-
5246
@Override
5347
public ListMultimap<HeaderName, String> getHeaders()
5448
{
@@ -73,7 +67,6 @@ public String toString()
7367
{
7468
return toStringHelper(this)
7569
.add("statusCode", getStatusCode())
76-
.add("statusMessage", getStatusMessage())
7770
.add("headers", getHeaders())
7871
.toString();
7972
}

http-client/src/main/java/com/facebook/airlift/http/client/thrift/ThriftResponse.java

-10
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,22 @@
88

99
import java.util.List;
1010

11-
import static java.util.Objects.requireNonNull;
12-
1311
public class ThriftResponse<T>
1412
{
1513
private final int statusCode;
16-
private final String statusMessage;
1714
private final String errorMessage;
1815
private final ListMultimap<HeaderName, String> headers;
1916
private final T value;
2017
private final IllegalArgumentException exception;
2118

2219
ThriftResponse(
2320
int statusCode,
24-
String statusMessage,
2521
String errorMessage,
2622
ListMultimap<HeaderName, String> headers,
2723
T value,
2824
IllegalArgumentException exception)
2925
{
3026
this.statusCode = statusCode;
31-
this.statusMessage = requireNonNull(statusMessage, "statusMessage is null");
3227
this.errorMessage = errorMessage;
3328
this.headers = headers != null ? ImmutableListMultimap.copyOf(headers) : null;
3429
this.value = value;
@@ -40,11 +35,6 @@ public int getStatusCode()
4035
return statusCode;
4136
}
4237

43-
public String getStatusMessage()
44-
{
45-
return statusMessage;
46-
}
47-
4838
public String getErrorMessage()
4939
{
5040
return errorMessage;

http-client/src/main/java/com/facebook/airlift/http/client/thrift/ThriftResponseHandler.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public ThriftResponse<T> handle(Request request, Response response)
6969
catch (Exception e) {
7070
exception = new IllegalArgumentException("Unable to create " + thriftCodec.getType() + " from THRIFT response", e);
7171
}
72-
return new ThriftResponse<>(response.getStatusCode(), response.getStatusMessage(), null, response.getHeaders(), value, exception);
72+
return new ThriftResponse<>(response.getStatusCode(), null, response.getHeaders(), value, exception);
7373
}
7474

7575
private ThriftResponse<T> createErrorResponse(Response response)
@@ -83,7 +83,7 @@ public InputStream openStream() throws IOException
8383
};
8484
try {
8585
String errorMessage = byteSource.asCharSource(StandardCharsets.UTF_8).read();
86-
return new ThriftResponse<>(response.getStatusCode(), response.getStatusMessage(), errorMessage, response.getHeaders(), null, null);
86+
return new ThriftResponse<>(response.getStatusCode(), errorMessage, response.getHeaders(), null, null);
8787
}
8888
catch (IOException e) {
8989
throw new UncheckedIOException(e);

http-client/src/test/java/com/facebook/airlift/http/client/AbstractHttpClientTest.java

-21
Original file line numberDiff line numberDiff line change
@@ -621,27 +621,6 @@ public void testResponseStatusCode()
621621
assertEquals(statusCode, 543);
622622
}
623623

624-
@Test
625-
public void testResponseStatusMessage()
626-
throws Exception
627-
{
628-
servlet.setResponseStatusMessage("message");
629-
630-
Request request = prepareGet()
631-
.setUri(baseURI)
632-
.build();
633-
634-
String statusMessage = executeRequest(request, createStatusResponseHandler()).getStatusMessage();
635-
636-
if (createClientConfig().isHttp2Enabled()) {
637-
// reason phrases are not supported in HTTP/2
638-
assertNull(statusMessage);
639-
}
640-
else {
641-
assertEquals(statusMessage, "message");
642-
}
643-
}
644-
645624
@Test
646625
public void testRequestHeaders()
647626
throws Exception

http-client/src/test/java/com/facebook/airlift/http/client/EchoServlet.java

+2-12
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ public final class EchoServlet
4242
private byte[] requestBytes;
4343

4444
private int responseStatusCode = 200;
45-
private String responseStatusMessage;
4645
private final ListMultimap<String, String> responseHeaders = ArrayListMultimap.create();
4746
private String responseBody;
4847

@@ -63,12 +62,8 @@ protected void service(HttpServletRequest request, HttpServletResponse response)
6362

6463
requestBytes = ByteStreams.toByteArray(request.getInputStream());
6564

66-
if (responseStatusMessage != null) {
67-
response.sendError(responseStatusCode, responseStatusMessage);
68-
}
69-
else {
70-
response.setStatus(responseStatusCode);
71-
}
65+
response.setStatus(responseStatusCode);
66+
7267
for (Map.Entry<String, String> entry : responseHeaders.entries()) {
7368
response.addHeader(entry.getKey(), entry.getValue());
7469
}
@@ -126,11 +121,6 @@ public void setResponseStatusCode(int responseStatusCode)
126121
this.responseStatusCode = responseStatusCode;
127122
}
128123

129-
public void setResponseStatusMessage(String responseStatusMessage)
130-
{
131-
this.responseStatusMessage = responseStatusMessage;
132-
}
133-
134124
public void addResponseHeader(String name, String value)
135125
{
136126
this.responseHeaders.put(name, value);

0 commit comments

Comments
 (0)