throw DeadlineExpiredException when deadline expiration is realized

deadlines/src/main/java/com/palantir/deadlines/Deadlines.java

@@ -56,6 +56,8 @@ private Deadlines() {}
      *
      * If no deadline state has been set for the current trace, return an empty Optional.
      *
+     * This method never throws.
+     *
      * @return the remaining deadline time for the current trace, or {@link Duration#ZERO} if the deadline
      * has expired, or {@link Optional#empty()} if no such deadline state exists.
      */
@@ -89,6 +91,9 @@ private static Optional<RemainingDeadline> getRemainingDeadlineInternal() {
      *
      * This function has no side-effects on the internal deadline state stored in a TraceLocal.
      *
+     * If at the time this method is called the selected deadline to encode has already expired, a
+     * {@link DeadlineExpiredException} will be thrown.
+     *
      * @param proposedDeadline a proposed value for the deadline; the actual value used will be the minimum of
      * this value and one already set via a previous call to {@link #parseFromRequest}, if it exists
      * @param request the request object to write the encoding to
@@ -126,9 +131,16 @@ public static <T> void encodeToRequest(
      * {@link #getRemainingDeadline()}} from threads participating in the current trace. The deadline value
      * is read from a {@link DeadlinesHttpHeaders#EXPECT_WITHIN} header on the request object.
      *
+     * The optional `internalDeadline` parameter specifices an alternative deadline to be used if it is lower
+     * than the one parsed from the request header. Callers can use this to bound the selected deadline based
+     * on specific internal logic that might take precedence over one provided in a request.
+     *
      * This function has side-effects on the internal deadline state stored in a TraceLocal; the state is
      * set (or overwritten) based on the value of the deadline parsed from request headers.
      *
+     * If at the time this method is called the selected deadline to store has already expired, a
+     * {@link DeadlineExpiredException} will be thrown.
+     *
      * @param internalDeadline if present, represents an alternative deadline that should be used if it is
      * lower than the one parsed from a request header
      * @param request the request object to read the deadline value from
@@ -170,7 +182,10 @@ private static void checkExpiration(long deadline, boolean internal) {
             // expired
             Expired_Cause cause = internal ? Expired_Cause.INTERNAL : Expired_Cause.EXTERNAL;
             metrics.expired(cause).mark();
-            // TODO(blaub): throw exception instead of return
+            if (internal) {
+                throw DeadlineExpiredException.internal();
+            }
+            throw DeadlineExpiredException.external();
         }
     }
 

deadlines/src/test/java/com/palantir/deadlines/DeadlinesTest.java

@@ -18,12 +18,15 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import com.codahale.metrics.Meter;
 import com.palantir.deadlines.DeadlineMetrics.Expired_Cause;
 import com.palantir.deadlines.Deadlines.RequestDecodingAdapter;
 import com.palantir.deadlines.Deadlines.RequestEncodingAdapter;
+import com.palantir.tracing.CloseableSpan;
 import com.palantir.tracing.CloseableTracer;
+import com.palantir.tracing.DetachedSpan;
 import com.palantir.tritium.metrics.registry.SharedTaggedMetricRegistries;
 import java.time.Duration;
 import java.util.HashMap;
@@ -306,15 +309,17 @@ public void test_encode_to_request_expiration_external_deadline() {
             long originalInternalValue = internalMeter.getCount();
 
             Map<String, String> outbound = new HashMap<>();
-            Deadlines.encodeToRequest(Duration.ofSeconds(10), outbound, DummyRequestEncoder.INSTANCE);
+            assertThatThrownBy(() ->
+                            Deadlines.encodeToRequest(Duration.ofSeconds(10), outbound, DummyRequestEncoder.INSTANCE))
+                    .isInstanceOf(DeadlineExpiredException.External.class);
 
             assertThat(externalMeter.getCount()).isGreaterThan(originalExternalValue);
             assertThat(internalMeter.getCount()).isEqualTo(originalInternalValue);
         }
     }
 
     @Test
-    public void test_encode_to_request_expiration_internal_deadline_2() {
+    public void test_encode_to_request_expiration_internal_deadline() {
         TestClock clock = new TestClock();
         Deadlines.setClock(clock);
         try (CloseableTracer tracer = CloseableTracer.startSpan("test")) {
@@ -335,13 +340,117 @@ public void test_encode_to_request_expiration_internal_deadline_2() {
             long originalInternalValue = internalMeter.getCount();
 
             Map<String, String> outbound = new HashMap<>();
-            Deadlines.encodeToRequest(Duration.ofSeconds(10), outbound, DummyRequestEncoder.INSTANCE);
+            assertThatThrownBy(() ->
+                            Deadlines.encodeToRequest(Duration.ofSeconds(10), outbound, DummyRequestEncoder.INSTANCE))
+                    .isInstanceOf(DeadlineExpiredException.Internal.class);
 
             assertThat(internalMeter.getCount()).isGreaterThan(originalInternalValue);
             assertThat(externalMeter.getCount()).isEqualTo(originalExternalValue);
         }
     }
 
+    @Test
+    public void test_rpc_expiration_1hop_external() {
+        TestClock clock = new TestClock();
+        Deadlines.setClock(clock);
+
+        Map<String, String> request = new HashMap<>();
+        Duration clientDeadline = Duration.ofSeconds(1);
+        Deadlines.encodeToRequest(clientDeadline, request, DummyRequestEncoder.INSTANCE);
+
+        DetachedSpan span = DetachedSpan.start("test");
+        try (CloseableSpan ignored = span.attach()) {
+            Deadlines.parseFromRequest(Optional.empty(), request, DummyRequestDecoder.INSTANCE);
+            clock.elapsed += Duration.ofSeconds(5).toNanos();
+            // simulate making another outbound request, which should throw
+            Map<String, String> serverRequest = new HashMap<>();
+            assertThatThrownBy(() -> Deadlines.encodeToRequest(
+                            Duration.ofSeconds(1), serverRequest, DummyRequestEncoder.INSTANCE))
+                    .isInstanceOf(DeadlineExpiredException.External.class);
+        }
+    }
+
+    @Test
+    public void test_rpc_expiration_1hop_internal() {
+        TestClock clock = new TestClock();
+        Deadlines.setClock(clock);
+
+        Map<String, String> request = new HashMap<>();
+        Duration clientDeadline = Duration.ofSeconds(1);
+        Deadlines.encodeToRequest(clientDeadline, request, DummyRequestEncoder.INSTANCE);
+
+        DetachedSpan span = DetachedSpan.start("test");
+        try (CloseableSpan ignored = span.attach()) {
+            Deadlines.parseFromRequest(
+                    Optional.of(Duration.ofMillis(500)), /* server has an internal deadline smaller than the client's */
+                    request,
+                    DummyRequestDecoder.INSTANCE);
+            clock.elapsed += Duration.ofMillis(600).toNanos();
+
+            Map<String, String> serverRequest = new HashMap<>();
+            assertThatThrownBy(() -> Deadlines.encodeToRequest(
+                            Duration.ofSeconds(1), serverRequest, DummyRequestEncoder.INSTANCE))
+                    .isInstanceOf(DeadlineExpiredException.Internal.class);
+        }
+    }
+
+    @Test
+    public void test_rpc_expiration_2hop_external_then_internal() {
+        // client provides large initial deadline
+        // server1 has a self-imposed smaller deadline
+        // server1 sends request to server2 with this much smaller deadline
+        // server2 unable to meet server1 deadline
+        // server2 throws "external" expiration
+        // enough time elapses such that server1's internally imposed deadline has expired
+        // server1 throws "internal" expiration
+
+        TestClock clock = new TestClock();
+        Deadlines.setClock(clock);
+
+        Map<String, String> clientRequest = new HashMap<>();
+        Duration clientDeadline = Duration.ofSeconds(1);
+        Deadlines.encodeToRequest(clientDeadline, clientRequest, DummyRequestEncoder.INSTANCE);
+
+        DetachedSpan server1Span = DetachedSpan.start("server1");
+        DetachedSpan server2Span = DetachedSpan.start("server2");
+
+        try (CloseableSpan ignored = server1Span.attach()) {
+            Deadlines.parseFromRequest(
+                    Optional.of(Duration.ofMillis(200)), /* server1 has smaller internally imposed deadline */
+                    clientRequest,
+                    DummyRequestDecoder.INSTANCE);
+            clock.elapsed += Duration.ofMillis(100).toNanos();
+            Map<String, String> server1Request = new HashMap<>();
+            Deadlines.encodeToRequest(
+                    Duration.ofSeconds(500), /* server1 should select smaller internal deadline */
+                    server1Request,
+                    DummyRequestEncoder.INSTANCE);
+
+            assertThat(server1Request)
+                    .containsKey(DeadlinesHttpHeaders.EXPECT_WITHIN)
+                    .containsValue("0.100");
+
+            try (CloseableSpan ignored2 = server2Span.attach()) {
+                Deadlines.parseFromRequest(Optional.empty(), server1Request, DummyRequestDecoder.INSTANCE);
+                clock.elapsed += Duration.ofMillis(1500).toNanos();
+
+                // server2 should throw an external error
+                Map<String, String> server2Request = new HashMap<>();
+                assertThatThrownBy(() -> {
+                            Deadlines.encodeToRequest(
+                                    Duration.ofSeconds(100), server2Request, DummyRequestEncoder.INSTANCE);
+                        })
+                        .isInstanceOf(DeadlineExpiredException.External.class);
+            }
+
+            assertThatThrownBy(() -> {
+                        Deadlines.encodeToRequest(
+                                Duration.ofSeconds(100), server1Request, DummyRequestEncoder.INSTANCE);
+                    })
+                    .isInstanceOf(DeadlineExpiredException.Internal.class);
+        }
+    }
+
     private enum DummyRequestEncoder implements RequestEncodingAdapter<Map<String, String>> {
         INSTANCE;