Handle HttpStatus/ResponseException in THttpService (line#1867)

Motivation:

When a Thrift service fails with an `HttpStatusException` or an
`HttpResponseException`. `THttpService` wraps it with a
`TApplicationException` and sends a `200 OK` response. Instead, we could
make `THttpService` respect such exceptions and send a proper HTTP error
response.

This can be useful when a service wants to send a timeout response such
as `503 Service Unavailable`, because otherwise a client will have to
handle two different timeouts in two different ways:

- `503 Service Unavailable` (when Thrift service method failed to
  produce a response before the request times out.)
- `200 OK` with a Thrift-level timeout exception (when Thrift service
  method produces an error response that tells the request has timed out.)

Modifications:

- Make `THttpService` handle `HttpStatusException` and
  `HttpResponseException` rather than encoding it into a
  `TApplicationException`.

Result:

- Closes line#1839

thrift/src/main/java/com/linecorp/armeria/server/thrift/THttpService.java

@@ -67,6 +67,8 @@
 import com.linecorp.armeria.internal.thrift.ThriftFieldAccess;
 import com.linecorp.armeria.internal.thrift.ThriftFunction;
 import com.linecorp.armeria.server.AbstractHttpService;
+import com.linecorp.armeria.server.HttpResponseException;
+import com.linecorp.armeria.server.HttpStatusException;
 import com.linecorp.armeria.server.Service;
 import com.linecorp.armeria.server.ServiceRequestContext;
 import com.linecorp.armeria.unsafe.ByteBufHttpData;
@@ -679,6 +681,16 @@ private static void handleException(
             ServiceRequestContext ctx, RpcResponse rpcRes, CompletableFuture<HttpResponse> httpRes,
             SerializationFormat serializationFormat, int seqId, ThriftFunction func, Throwable cause) {
 
+        if (cause instanceof HttpStatusException) {
+            httpRes.complete(HttpResponse.of(((HttpStatusException) cause).httpStatus()));
+            return;
+        }
+
+        if (cause instanceof HttpResponseException) {
+            httpRes.complete(((HttpResponseException) cause).httpResponse());
+            return;
+        }
+
         final TBase<?, ?> result = func.newResult();
         final HttpData content;
         if (func.setException(result, cause)) {

thrift/src/test/java/com/linecorp/armeria/it/thrift/ThriftHttpErrorResponseTest.java

@@ -0,0 +1,89 @@
+/*
+ * Copyright 2019 LINE Corporation
+ *
+ * LINE Corporation licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package com.linecorp.armeria.it.thrift;
+
+import static com.linecorp.armeria.common.thrift.ThriftSerializationFormats.BINARY;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+
+import com.google.common.base.Ascii;
+
+import com.linecorp.armeria.client.Clients;
+import com.linecorp.armeria.client.InvalidResponseHeadersException;
+import com.linecorp.armeria.common.HttpStatus;
+import com.linecorp.armeria.server.HttpResponseException;
+import com.linecorp.armeria.server.HttpStatusException;
+import com.linecorp.armeria.server.ServerBuilder;
+import com.linecorp.armeria.server.thrift.THttpService;
+import com.linecorp.armeria.service.test.thrift.main.HelloService;
+import com.linecorp.armeria.service.test.thrift.main.HelloService.Iface;
+import com.linecorp.armeria.testing.junit.server.ServerExtension;
+
+/**
+ * Tests if Armeria decorators can alter the request/response timeout specified in Thrift call parameters.
+ */
+public class ThriftHttpErrorResponseTest {
+
+    private enum TestParam {
+        ASYNC_STATUS((HelloService.AsyncIface) (name, resultHandler) -> {
+            resultHandler.onError(HttpStatusException.of(HttpStatus.CONFLICT));
+        }),
+        ASYNC_RESPONSE((HelloService.AsyncIface) (name, resultHandler) -> {
+            resultHandler.onError(HttpResponseException.of(HttpStatus.CONFLICT));
+        }),
+        ASYNC_THROW((HelloService.AsyncIface) (name, resultHandler) -> {
+            throw HttpStatusException.of(HttpStatus.CONFLICT);
+        }),
+        SYNC_STATUS((Iface) name -> {
+            throw HttpStatusException.of(HttpStatus.CONFLICT);
+        }),
+        SYNC_RESPONSE((Iface) name -> {
+            throw HttpResponseException.of(HttpStatus.CONFLICT);
+        });
+
+        final String path;
+        final Object service;
+
+        TestParam(Object service) {
+            path = '/' + Ascii.toLowerCase(name());
+            this.service = service;
+        }
+    }
+
+    @RegisterExtension
+    static final ServerExtension server = new ServerExtension() {
+        @Override
+        protected void configure(ServerBuilder sb) throws Exception {
+            for (TestParam param : TestParam.values()) {
+                sb.service(param.path, THttpService.of(param.service));
+            }
+        }
+    };
+
+    @ParameterizedTest
+    @EnumSource(TestParam.class)
+    void test(TestParam param) throws Exception {
+        final Iface client = Clients.newClient(server.uri(BINARY, param.path), Iface.class);
+        assertThatThrownBy(() -> client.hello("foo"))
+                .isInstanceOfSatisfying(InvalidResponseHeadersException.class, cause -> {
+                    assertThat(cause.headers().status()).isEqualTo(HttpStatus.CONFLICT);
+                });
+    }
+}

thrift/src/test/java/com/linecorp/armeria/it/thrift/ThrottlingRpcServiceTest.java

@@ -24,7 +24,6 @@
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
-import org.apache.thrift.TApplicationException;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -33,6 +32,8 @@
 import org.mockito.junit.MockitoRule;
 
 import com.linecorp.armeria.client.ClientBuilder;
+import com.linecorp.armeria.client.InvalidResponseHeadersException;
+import com.linecorp.armeria.common.HttpStatus;
 import com.linecorp.armeria.server.ServerBuilder;
 import com.linecorp.armeria.server.thrift.THttpService;
 import com.linecorp.armeria.server.thrift.ThriftCallService;
@@ -83,7 +84,10 @@ public void throttle() throws Exception {
         final HelloService.Iface client = new ClientBuilder(server.uri(BINARY, "/thrift-never"))
                 .build(HelloService.Iface.class);
 
-        assertThatThrownBy(() -> client.hello("foo")).isInstanceOf(TApplicationException.class);
+        assertThatThrownBy(() -> client.hello("foo"))
+                .isInstanceOfSatisfying(InvalidResponseHeadersException.class, cause -> {
+                    assertThat(cause.headers().status()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE);
+                });
         verifyNoMoreInteractions(serviceHandler);
     }
 }