Include dueTo and retryHint in QosException SafeLoggable.getArgs (#1248)

Include dueTo and retryHint in QosException SafeLoggable args
palantir · Oct 22, 2024 · f6da346 · f6da346
1 parent 6827e9d
commit f6da346
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 9 deletions.
diff --git a/changelog/@unreleased/pr-1248.v2.yml b/changelog/@unreleased/pr-1248.v2.yml
@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Include dueTo and retryHint in QosException SafeLoggable args
+  links:
+  - https://github.com/palantir/conjure-java-runtime-api/pull/1248
diff --git a/errors/src/main/java/com/palantir/conjure/java/api/errors/QosException.java b/errors/src/main/java/com/palantir/conjure/java/api/errors/QosException.java
@@ -24,7 +24,6 @@
 import com.palantir.logsafe.UnsafeArg;
 import java.net.URL;
 import java.time.Duration;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
@@ -224,7 +223,11 @@ public String getLogMessage() {
 
         @Override
         public List<Arg<?>> getArgs() {
-            return List.of(SafeArg.of("retryAfter", retryAfter.orElse(null)), SafeArg.of("reason", getReason()));
+            return List.of(
+                    SafeArg.of("retryAfter", retryAfter.orElse(null)),
+                    SafeArg.of("reason", getReason().reason()),
+                    SafeArg.of("dueTo", getReason().dueTo().orElse(null)),
+                    SafeArg.of("retryHint", getReason().retryHint().orElse(null)));
         }
     }
 
@@ -272,7 +275,11 @@ public String getLogMessage() {
         @Unsafe
         @Override
         public List<Arg<?>> getArgs() {
-            return List.of(UnsafeArg.of("redirectTo", redirectTo), SafeArg.of("reason", getReason()));
+            return List.of(
+                    UnsafeArg.of("redirectTo", redirectTo),
+                    SafeArg.of("reason", getReason().reason()),
+                    SafeArg.of("dueTo", getReason().dueTo().orElse(null)),
+                    SafeArg.of("retryHint", getReason().retryHint().orElse(null)));
         }
     }
 
@@ -310,7 +317,10 @@ public String getLogMessage() {
 
         @Override
         public List<Arg<?>> getArgs() {
-            return Collections.singletonList(SafeArg.of("reason", getReason()));
+            return List.of(
+                    SafeArg.of("reason", getReason().reason()),
+                    SafeArg.of("dueTo", getReason().dueTo().orElse(null)),
+                    SafeArg.of("retryHint", getReason().retryHint().orElse(null)));
         }
     }
 }
diff --git a/errors/src/test/java/com/palantir/conjure/java/api/errors/QosExceptionTest.java b/errors/src/test/java/com/palantir/conjure/java/api/errors/QosExceptionTest.java
@@ -20,12 +20,15 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import com.google.errorprone.annotations.CompileTimeConstant;
+import com.palantir.conjure.java.api.errors.QosReason.DueTo;
+import com.palantir.conjure.java.api.errors.QosReason.RetryHint;
 import com.palantir.logsafe.SafeArg;
 import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
 public final class QosExceptionTest {
@@ -55,14 +58,49 @@ public Class<?> visit(QosException.Unavailable exception) {
         assertThat(QosException.unavailable().accept(visitor)).isEqualTo(QosException.Unavailable.class);
     }
 
-    @Test
-    public void testReason() {
-        QosReason reason = QosReason.of("custom-reason");
+    private enum QosExceptionFactory {
+        THROTTLE() {
+            @Override
+            QosException create(QosReason reason) {
+                return QosException.throttle(reason);
+            }
+        },
+        UNAVAILABLE {
+            @Override
+            QosException create(QosReason reason) {
+                return QosException.unavailable(reason);
+            }
+        },
+        RETRY_OTHER {
+            @Override
+            QosException create(QosReason reason) {
+                try {
+                    return QosException.retryOther(reason, new URL("http://foo"));
+                } catch (MalformedURLException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+        };
+
+        abstract QosException create(QosReason reason);
+    }
+
+    @ParameterizedTest
+    @EnumSource(QosExceptionFactory.class)
+    void testReason(QosExceptionFactory qosException) {
+        QosReason reason = QosReason.builder()
+                .reason("custom-reason")
+                .dueTo(DueTo.CUSTOM)
+                .retryHint(RetryHint.DO_NOT_RETRY)
+                .build();
         assertThat(QosException.throttle(reason).getReason()).isEqualTo(reason);
         assertThatLoggableExceptionThrownBy(() -> {
-                    throw QosException.throttle(reason);
+                    throw qosException.create(reason);
                 })
-                .containsArgs(SafeArg.of("reason", reason));
+                .containsArgs(
+                        SafeArg.of("reason", "custom-reason"),
+                        SafeArg.of("retryHint", RetryHint.DO_NOT_RETRY),
+                        SafeArg.of("dueTo", DueTo.CUSTOM));
     }
 
     @ParameterizedTest