Remove status message support from HTTP client

Messages are not in HTTP/2 and are poorly supported in servlets.
prestodb · Jul 31, 2024 · 63c13d6 · 63c13d6
1 parent ff25545
commit 63c13d6
Show file tree

Hide file tree

Showing 16 changed files with 17 additions and 112 deletions.
diff --git a/event/src/main/java/com/facebook/airlift/event/client/HttpEventClient.java b/event/src/main/java/com/facebook/airlift/event/client/HttpEventClient.java
@@ -169,13 +169,12 @@ public Void handle(Request request, Response response)
             try {
                 InputStream inputStream = response.getInputStream();
                 String responseBody = CharStreams.toString(new InputStreamReader(inputStream));
-                log.debug("Posting event to %s failed: status_code=%d status_line=%s body=%s", request.getUri(), statusCode, response.getStatusMessage(), responseBody);
+                log.debug("Posting event to %s failed: status_code=%d body=%s", request.getUri(), statusCode, responseBody);
             }
             catch (IOException bodyError) {
-                log.debug("Posting event to %s failed: status_code=%d status_line=%s error=%s",
+                log.debug("Posting event to %s failed: status_code=%d error=%s",
                         request.getUri(),
                         statusCode,
-                        response.getStatusMessage(),
                         bodyError.getMessage());
             }
             return null;

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/FullJsonResponseHandler.java b/http-client/src/main/java/com/facebook/airlift/http/client/FullJsonResponseHandler.java
@@ -64,9 +64,9 @@ public JsonResponse<T> handle(Request request, Response response)
         byte[] bytes = readResponseBytes(response);
         String contentType = response.getHeader(CONTENT_TYPE);
         if ((contentType == null) || !MediaType.parse(contentType).is(MEDIA_TYPE_JSON)) {
-            return new JsonResponse<>(response.getStatusCode(), response.getStatusMessage(), response.getHeaders(), bytes);
+            return new JsonResponse<>(response.getStatusCode(), response.getHeaders(), bytes);
         }
-        return new JsonResponse<>(response.getStatusCode(), response.getStatusMessage(), response.getHeaders(), jsonCodec, bytes);
+        return new JsonResponse<>(response.getStatusCode(), response.getHeaders(), jsonCodec, bytes);
     }
 
     private static byte[] readResponseBytes(Response response)
@@ -82,18 +82,16 @@ private static byte[] readResponseBytes(Response response)
     public static class JsonResponse<T>
     {
         private final int statusCode;
-        private final String statusMessage;
         private final ListMultimap<HeaderName, String> headers;
         private final boolean hasValue;
         private final byte[] jsonBytes;
         private final byte[] responseBytes;
         private final T value;
         private final IllegalArgumentException exception;
 
-        public JsonResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers, byte[] responseBytes)
+        public JsonResponse(int statusCode, ListMultimap<HeaderName, String> headers, byte[] responseBytes)
         {
             this.statusCode = statusCode;
-            this.statusMessage = statusMessage;
             this.headers = ImmutableListMultimap.copyOf(headers);
 
             this.hasValue = false;
@@ -104,10 +102,9 @@ public JsonResponse(int statusCode, String statusMessage, ListMultimap<HeaderNam
         }
 
         @SuppressWarnings("ThrowableInstanceNeverThrown")
-        public JsonResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers, JsonCodec<T> jsonCodec, byte[] jsonBytes)
+        public JsonResponse(int statusCode, ListMultimap<HeaderName, String> headers, JsonCodec<T> jsonCodec, byte[] jsonBytes)
         {
             this.statusCode = statusCode;
-            this.statusMessage = statusMessage;
             this.headers = ImmutableListMultimap.copyOf(headers);
 
             this.jsonBytes = requireNonNull(jsonBytes, "jsonBytes is null");
@@ -131,11 +128,6 @@ public int getStatusCode()
             return statusCode;
         }
 
-        public String getStatusMessage()
-        {
-            return statusMessage;
-        }
-
         @Nullable
         public String getHeader(String name)
         {
@@ -201,7 +193,6 @@ public String toString()
         {
             return toStringHelper(this)
                     .add("statusCode", statusCode)
-                    .add("statusMessage", statusMessage)
                     .add("headers", headers)
                     .add("hasValue", hasValue)
                     .add("value", value)

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/JsonResponseHandler.java b/http-client/src/main/java/com/facebook/airlift/http/client/JsonResponseHandler.java
@@ -68,7 +68,7 @@ public T handle(Request request, Response response)
     {
         if (!successfulResponseCodes.contains(response.getStatusCode())) {
             throw new UnexpectedResponseException(
-                    String.format("Expected response code to be %s, but was %d: %s", successfulResponseCodes, response.getStatusCode(), response.getStatusMessage()),
+                    String.format("Expected response code to be %s, but was %d", successfulResponseCodes, response.getStatusCode()),
                     request,
                     response);
         }

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/Response.java b/http-client/src/main/java/com/facebook/airlift/http/client/Response.java
@@ -29,8 +29,6 @@ public interface Response
 {
     int getStatusCode();
 
-    String getStatusMessage();
-
     @Nullable
     default String getHeader(String name)
     {

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/StatusResponseHandler.java b/http-client/src/main/java/com/facebook/airlift/http/client/StatusResponseHandler.java
@@ -48,19 +48,17 @@ public StatusResponse handleException(Request request, Exception exception)
     @Override
     public StatusResponse handle(Request request, Response response)
     {
-        return new StatusResponse(response.getStatusCode(), response.getStatusMessage(), response.getHeaders());
+        return new StatusResponse(response.getStatusCode(), response.getHeaders());
     }
 
     public static class StatusResponse
     {
         private final int statusCode;
-        private final String statusMessage;
         private final ListMultimap<HeaderName, String> headers;
 
-        public StatusResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers)
+        public StatusResponse(int statusCode, ListMultimap<HeaderName, String> headers)
         {
             this.statusCode = statusCode;
-            this.statusMessage = statusMessage;
             this.headers = ImmutableListMultimap.copyOf(headers);
         }
 
@@ -69,11 +67,6 @@ public int getStatusCode()
             return statusCode;
         }
 
-        public String getStatusMessage()
-        {
-            return statusMessage;
-        }
-
         @Nullable
         public String getHeader(String name)
         {

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/StringResponseHandler.java b/http-client/src/main/java/com/facebook/airlift/http/client/StringResponseHandler.java
@@ -61,14 +61,12 @@ public StringResponse handle(Request request, Response response)
                 MediaType mediaType = MediaType.parse(contentType);
                 return new StringResponse(
                         response.getStatusCode(),
-                        response.getStatusMessage(),
                         response.getHeaders(),
                         new String(ByteStreams.toByteArray(response.getInputStream()), mediaType.charset().or(UTF_8)));
             }
 
             return new StringResponse(
                     response.getStatusCode(),
-                    response.getStatusMessage(),
                     response.getHeaders(),
                     new String(ByteStreams.toByteArray(response.getInputStream()), UTF_8));
         }
@@ -80,14 +78,12 @@ public StringResponse handle(Request request, Response response)
     public static class StringResponse
     {
         private final int statusCode;
-        private final String statusMessage;
         private final ListMultimap<HeaderName, String> headers;
         private final String body;
 
-        public StringResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers, String body)
+        public StringResponse(int statusCode, ListMultimap<HeaderName, String> headers, String body)
         {
             this.statusCode = statusCode;
-            this.statusMessage = statusMessage;
             this.headers = ImmutableListMultimap.copyOf(headers);
             this.body = body;
         }
@@ -97,11 +93,6 @@ public int getStatusCode()
             return statusCode;
         }
 
-        public String getStatusMessage()
-        {
-            return statusMessage;
-        }
-
         public String getBody()
         {
             return body;

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/UnexpectedResponseException.java b/http-client/src/main/java/com/facebook/airlift/http/client/UnexpectedResponseException.java
@@ -31,15 +31,13 @@ public class UnexpectedResponseException
 {
     private final Request request;
     private final int statusCode;
-    private final String statusMessage;
     private final ListMultimap<HeaderName, String> headers;
 
     public UnexpectedResponseException(Request request, Response response)
     {
-        this(String.format("%d: %s", response.getStatusCode(), response.getStatusMessage()),
+        this(String.format("%d", response.getStatusCode()),
                 request,
                 response.getStatusCode(),
-                response.getStatusMessage(),
                 ImmutableListMultimap.copyOf(response.getHeaders()));
     }
 
@@ -48,16 +46,14 @@ public UnexpectedResponseException(String message, Request request, Response res
         this(message,
                 request,
                 response.getStatusCode(),
-                response.getStatusMessage(),
                 ImmutableListMultimap.copyOf(response.getHeaders()));
     }
 
-    public UnexpectedResponseException(String message, Request request, int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers)
+    public UnexpectedResponseException(String message, Request request, int statusCode, ListMultimap<HeaderName, String> headers)
     {
         super(message);
         this.request = request;
         this.statusCode = statusCode;
-        this.statusMessage = statusMessage;
         this.headers = ImmutableListMultimap.copyOf(headers);
     }
 
@@ -66,11 +62,6 @@ public int getStatusCode()
         return statusCode;
     }
 
-    public String getStatusMessage()
-    {
-        return statusMessage;
-    }
-
     @Nullable
     public String getHeader(String name)
     {
@@ -94,7 +85,6 @@ public String toString()
         return toStringHelper(this)
                 .add("request", request)
                 .add("statusCode", statusCode)
-                .add("statusMessage", statusMessage)
                 .add("headers", headers)
                 .toString();
     }

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/jetty/JettyResponse.java b/http-client/src/main/java/com/facebook/airlift/http/client/jetty/JettyResponse.java
@@ -31,12 +31,6 @@ public int getStatusCode()
         return response.getStatus();
     }
 
-    @Override
-    public String getStatusMessage()
-    {
-        return response.getReason();
-    }
-
     @Override
     public ListMultimap<HeaderName, String> getHeaders()
     {
@@ -60,7 +54,6 @@ public String toString()
     {
         return toStringHelper(this)
                 .add("statusCode", getStatusCode())
-                .add("statusMessage", getStatusMessage())
                 .add("headers", getHeaders())
                 .toString();
     }

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/testing/TestingResponse.java b/http-client/src/main/java/com/facebook/airlift/http/client/testing/TestingResponse.java
@@ -43,12 +43,6 @@ public int getStatusCode()
         return status.code();
     }
 
-    @Override
-    public String getStatusMessage()
-    {
-        return status.reason();
-    }
-
     @Override
     public ListMultimap<HeaderName, String> getHeaders()
     {
@@ -73,7 +67,6 @@ public String toString()
     {
         return toStringHelper(this)
                 .add("statusCode", getStatusCode())
-                .add("statusMessage", getStatusMessage())
                 .add("headers", getHeaders())
                 .toString();
     }

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/thrift/ThriftResponse.java b/http-client/src/main/java/com/facebook/airlift/http/client/thrift/ThriftResponse.java
@@ -8,27 +8,22 @@
 
 import java.util.List;
 
-import static java.util.Objects.requireNonNull;
-
 public class ThriftResponse<T>
 {
     private final int statusCode;
-    private final String statusMessage;
     private final String errorMessage;
     private final ListMultimap<HeaderName, String> headers;
     private final T value;
     private final IllegalArgumentException exception;
 
     ThriftResponse(
             int statusCode,
-            String statusMessage,
             String errorMessage,
             ListMultimap<HeaderName, String> headers,
             T value,
             IllegalArgumentException exception)
     {
         this.statusCode = statusCode;
-        this.statusMessage = requireNonNull(statusMessage, "statusMessage is null");
         this.errorMessage = errorMessage;
         this.headers = headers != null ? ImmutableListMultimap.copyOf(headers) : null;
         this.value = value;
@@ -40,11 +35,6 @@ public int getStatusCode()
         return statusCode;
     }
 
-    public String getStatusMessage()
-    {
-        return statusMessage;
-    }
-
     public String getErrorMessage()
     {
         return errorMessage;

diff --git a/http-client/src/main/java/com/facebook/airlift/http/client/thrift/ThriftResponseHandler.java b/http-client/src/main/java/com/facebook/airlift/http/client/thrift/ThriftResponseHandler.java
@@ -69,7 +69,7 @@ public ThriftResponse<T> handle(Request request, Response response)
         catch (Exception e) {
             exception = new IllegalArgumentException("Unable to create " + thriftCodec.getType() + " from THRIFT response", e);
         }
-        return new ThriftResponse<>(response.getStatusCode(), response.getStatusMessage(), null, response.getHeaders(), value, exception);
+        return new ThriftResponse<>(response.getStatusCode(), null, response.getHeaders(), value, exception);
     }
 
     private ThriftResponse<T> createErrorResponse(Response response)
@@ -83,7 +83,7 @@ public InputStream openStream() throws IOException
         };
         try {
             String errorMessage = byteSource.asCharSource(StandardCharsets.UTF_8).read();
-            return new ThriftResponse<>(response.getStatusCode(), response.getStatusMessage(), errorMessage, response.getHeaders(), null, null);
+            return new ThriftResponse<>(response.getStatusCode(), errorMessage, response.getHeaders(), null, null);
         }
         catch (IOException e) {
             throw new UncheckedIOException(e);

diff --git a/http-client/src/test/java/com/facebook/airlift/http/client/AbstractHttpClientTest.java b/http-client/src/test/java/com/facebook/airlift/http/client/AbstractHttpClientTest.java
@@ -621,27 +621,6 @@ public void testResponseStatusCode()
         assertEquals(statusCode, 543);
     }
 
-    @Test
-    public void testResponseStatusMessage()
-            throws Exception
-    {
-        servlet.setResponseStatusMessage("message");
-
-        Request request = prepareGet()
-                .setUri(baseURI)
-                .build();
-
-        String statusMessage = executeRequest(request, createStatusResponseHandler()).getStatusMessage();
-
-        if (createClientConfig().isHttp2Enabled()) {
-            // reason phrases are not supported in HTTP/2
-            assertNull(statusMessage);
-        }
-        else {
-            assertEquals(statusMessage, "message");
-        }
-    }
-
     @Test
     public void testRequestHeaders()
             throws Exception

diff --git a/http-client/src/test/java/com/facebook/airlift/http/client/EchoServlet.java b/http-client/src/test/java/com/facebook/airlift/http/client/EchoServlet.java
@@ -42,7 +42,6 @@ public final class EchoServlet
     private byte[] requestBytes;
 
     private int responseStatusCode = 200;
-    private String responseStatusMessage;
     private final ListMultimap<String, String> responseHeaders = ArrayListMultimap.create();
     private String responseBody;
 
@@ -63,12 +62,8 @@ protected void service(HttpServletRequest request, HttpServletResponse response)
 
         requestBytes = ByteStreams.toByteArray(request.getInputStream());
 
-        if (responseStatusMessage != null) {
-            response.sendError(responseStatusCode, responseStatusMessage);
-        }
-        else {
-            response.setStatus(responseStatusCode);
-        }
+        response.setStatus(responseStatusCode);
+
         for (Map.Entry<String, String> entry : responseHeaders.entries()) {
             response.addHeader(entry.getKey(), entry.getValue());
         }
@@ -126,11 +121,6 @@ public void setResponseStatusCode(int responseStatusCode)
         this.responseStatusCode = responseStatusCode;
     }
 
-    public void setResponseStatusMessage(String responseStatusMessage)
-    {
-        this.responseStatusMessage = responseStatusMessage;
-    }
-
     public void addResponseHeader(String name, String value)
     {
         this.responseHeaders.put(name, value);