Make ClientRequestContext.authority() and host() return non-null

brave/brave6/src/main/java/com/linecorp/armeria/client/brave/ArmeriaHttpClientParser.java

@@ -16,7 +16,7 @@
 
 package com.linecorp.armeria.client.brave;
 
-import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.linecorp.armeria.client.ObjectUtil.firstNonNullException;
 
 import java.net.InetSocketAddress;
 
@@ -72,7 +72,7 @@ public void parse(brave.http.HttpRequest request, TraceContext context, SpanCust
             return;
         }
 
-        span.tag(SpanTags.TAG_HTTP_HOST, firstNonNull(ctx.authority(), "UNKNOWN"))
+        span.tag(SpanTags.TAG_HTTP_HOST, firstNonNullException(ctx::authority, "UNKNOWN"))
             .tag(SpanTags.TAG_HTTP_URL, ctx.uri().toString());
     }
 

core/src/main/java/com/linecorp/armeria/client/ClientRequestContext.java

@@ -290,14 +290,12 @@ ClientRequestContext newDerivedContext(RequestId id, @Nullable HttpRequest req,
      *     <li>{@link Endpoint#authority()}.</li>
      * </ol>
      */
-    @Nullable
     @UnstableApi
     String authority();
 
     /**
      * Returns the host part of {@link #authority()}, without a port number.
      */
-    @Nullable
     @UnstableApi
     String host();
 

core/src/main/java/com/linecorp/armeria/client/ClientRequestContextWrapper.java

@@ -70,13 +70,11 @@ public String fragment() {
         return unwrap().fragment();
     }
 
-    @Nullable
     @Override
     public String authority() {
         return unwrap().authority();
     }
 
-    @Nullable
     @Override
     public String host() {
         return unwrap().host();

core/src/main/java/com/linecorp/armeria/client/ObjectUtil.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright 2024 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.client;
+
+import java.util.function.Supplier;
+
+/**
+ * The utility class for Object.
+ */
+public final class ObjectUtil {
+
+    /**
+     * Returns a non-null value. If an unexpected exception occurs while retrieving the first value,
+     * or the first value is null, this function will return the second value.
+     * Otherwise, it returns the first value.
+     */
+    public static <T> T firstNonNullException(Supplier<T> firstSupplier, T second) {
+        try {
+            if (firstSupplier != null) {
+                final T first = firstSupplier.get();
+                if (first != null) {
+                    return first;
+                }
+            }
+        } catch (Exception e) {
+            // Ignore
+        }
+
+        if (second != null) {
+            return second;
+        }
+
+        throw new NullPointerException("Both parameters are null.");
+    }
+
+    private ObjectUtil() {
+    }
+}

core/src/main/java/com/linecorp/armeria/client/observation/DefaultHttpClientObservationConvention.java

@@ -16,7 +16,7 @@
 
 package com.linecorp.armeria.client.observation;
 
-import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.linecorp.armeria.client.ObjectUtil.firstNonNullException;
 
 import java.net.InetSocketAddress;
 
@@ -106,7 +106,7 @@ public KeyValues getHighCardinalityKeyValues(ClientObservationContext context) {
         }
         final ImmutableList.Builder<KeyValue> builder = ImmutableList.builderWithExpectedSize(expectedSize);
         builder.add(HighCardinalityKeys.HTTP_PATH.withValue(ctx.path()),
-                    HighCardinalityKeys.HTTP_HOST.withValue(firstNonNull(ctx.authority(), "UNKNOWN")),
+                    HighCardinalityKeys.HTTP_HOST.withValue(firstNonNullException(ctx::authority, "UNKNOWN")),
                     HighCardinalityKeys.HTTP_URL.withValue(ctx.uri().toString()));
         addIfNotNull(addressRemote, builder);
         addIfNotNull(addressLocal, builder);

core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java

@@ -483,7 +483,7 @@ private void failEarly(Throwable cause) {
 
     // TODO(ikhoon): Consider moving the logic for filling authority to `HttpClientDelegate.exceute()`.
     private void autoFillSchemeAuthorityAndOrigin() {
-        final String authority = authority();
+        final String authority = authorityOrNull();
         if (authority != null && endpoint != null && endpoint.isIpAddrOnly()) {
             // The connection will be established with the IP address but `host` set to the `Endpoint`
             // could be used for SNI. It would make users send HTTPS requests with CSLB or configure a reverse
@@ -750,9 +750,19 @@ public String fragment() {
         return requestTarget().fragment();
     }
 
-    @Nullable
     @Override
     public String authority() {
+        final String authority = authorityOrNull();
+        if (authority == null) {
+            throw new IllegalStateException(
+                    "ClientRequestContext may be in the process of initialization." +
+                    "In this case, host() or authority() could be null");
+        }
+        return authority;
+    }
+
+    @Nullable
+    private String authorityOrNull() {
         final HttpHeaders additionalRequestHeaders = this.additionalRequestHeaders;
         String authority = additionalRequestHeaders.get(HttpHeaderNames.AUTHORITY);
         if (authority == null) {
@@ -774,6 +784,7 @@ public String authority() {
         if (authority == null) {
             authority = internalRequestHeaders.get(HttpHeaderNames.HOST);
         }
+
         return authority;
     }
 
@@ -794,20 +805,16 @@ private String origin() {
         return origin;
     }
 
-    @Nullable
     @Override
     public String host() {
         final String authority = authority();
-        if (authority == null) {
-            return null;
-        }
         return SchemeAndAuthority.of(null, authority).host();
     }
 
     @Override
     public URI uri() {
         final String scheme = getScheme(sessionProtocol());
-        final String authority = authority();
+        final String authority = authorityOrNull();
         final String path = path();
         final String query = query();
         final String fragment = fragment();

core/src/test/java/com/linecorp/armeria/client/ClientRequestContextTest.java

@@ -18,6 +18,8 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
 import java.util.function.Function;
 import java.util.stream.Stream;
 
@@ -29,6 +31,7 @@
 import org.junit.jupiter.params.provider.ArgumentsSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
+import com.linecorp.armeria.common.AggregatedHttpResponse;
 import com.linecorp.armeria.common.HttpMethod;
 import com.linecorp.armeria.common.HttpRequest;
 import com.linecorp.armeria.common.RequestContext;
@@ -269,6 +272,31 @@ void hasOwnAttr() {
         }
     }
 
+    @Test
+    void callAuthorityShouldBeThrownDuringPartiallyInit() {
+        final CountDownLatch countDownLatch = new CountDownLatch(1);
+        final WebClient client = WebClient
+                .builder("http://localhost:8080")
+                .contextCustomizer(ctx -> {
+                    countDownLatch.countDown();
+                    assertThatThrownBy(ctx::host)
+                            .isInstanceOf(IllegalStateException.class)
+                            .hasMessage(
+                                    "ClientRequestContext may be in the process of initialization." +
+                                    "In this case, host() or authority() could be null");
+                    assertThatThrownBy(ctx::authority)
+                            .isInstanceOf(IllegalStateException.class)
+                            .hasMessage(
+                                    "ClientRequestContext may be in the process of initialization." +
+                                    "In this case, host() or authority() could be null"
+                            );
+                    }).build();
+
+        final CompletableFuture<AggregatedHttpResponse> aggregate = client.get("/").aggregate();
+        assertThat(countDownLatch.getCount()).isEqualTo(0);
+        aggregate.cancel(true);
+    }
+
     @ParameterizedTest
     @ValueSource(strings = {"%", "http:///", "http://foo.com/bar"})
     void updateRequestWithInvalidPath(String path) {

core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java

@@ -221,7 +221,8 @@ void testAuthorityOverridden() {
                 HttpMethod.POST, "/foo",
                 HttpHeaderNames.SCHEME, "http"));
         final DefaultClientRequestContext ctx = newContext(ClientOptions.of(), request1);
-        assertThat(ctx.authority()).isNull();
+
+        assertThatThrownBy(() -> ctx.authority()).isInstanceOf(IllegalStateException.class);
         assertThat(ctx.uri().toString()).isEqualTo("http:/foo");
         assertThat(ctx.uri()).hasScheme("http").hasAuthority(null).hasPath("/foo");