Respond to PR comments

Author: Quinn-With-Two-Ns

temporal-sdk/src/main/java/io/temporal/failure/DefaultFailureConverter.java

@@ -337,7 +337,7 @@ private Failure exceptionToFailure(Throwable throwable) {
     } else if (throwable instanceof HandlerException) {
       HandlerException he = (HandlerException) throwable;
       if (he.getOriginalFailure() != null) {
-        return NexusUtil.nexusFailureToAPIFailure(he.getOriginalFailure(), true);
+        return NexusUtil.nexusFailureToTemporalFailure(he.getOriginalFailure(), true);
       }
       NexusHandlerErrorRetryBehavior retryBehavior =
           NexusHandlerErrorRetryBehavior.NEXUS_HANDLER_ERROR_RETRY_BEHAVIOR_UNSPECIFIED;

temporal-sdk/src/main/java/io/temporal/internal/common/NexusUtil.java

@@ -80,7 +80,7 @@ public static Failure temporalFailureToNexusFailure(
     return failureBuilder.build();
   }
 
-  public static io.temporal.api.failure.v1.Failure nexusFailureToAPIFailure(
+  public static io.temporal.api.failure.v1.Failure nexusFailureToTemporalFailure(
       FailureInfo failureInfo, boolean retryable) {
     io.temporal.api.failure.v1.Failure.Builder apiFailure =
         io.temporal.api.failure.v1.Failure.newBuilder();

temporal-sdk/src/main/java/io/temporal/internal/worker/NexusWorker.java

@@ -395,7 +395,7 @@ private Response getResponseForOldServer(Response response) {
           .build();
     }
 
-    @SuppressWarnings("deprecation")
+    @SuppressWarnings("deprecation") // Uses setError() for backward compat with old servers
     private void sendReply(
         ByteString taskToken,
         boolean supportTemporalFailure,

temporal-sdk/src/test/java/io/temporal/internal/common/NexusUtilTest.java

@@ -65,7 +65,7 @@ public void testTemporalFailureToNexusFailureRoundTrip() {
     }
     FailureInfo failureInfo = failureInfoBuilder.build();
 
-    Failure reconstructed = NexusUtil.nexusFailureToAPIFailure(failureInfo, true);
+    Failure reconstructed = NexusUtil.nexusFailureToTemporalFailure(failureInfo, true);
 
     // Verify round-trip preserves all fields
     Assert.assertEquals("test failure", reconstructed.getMessage());
@@ -95,7 +95,7 @@ public void testTemporalFailureToNexusFailureInfoRoundTrip() {
 
     // Convert to FailureInfo and back
     FailureInfo failureInfo = NexusUtil.temporalFailureToNexusFailureInfo(outerFailure);
-    Failure reconstructed = NexusUtil.nexusFailureToAPIFailure(failureInfo, false);
+    Failure reconstructed = NexusUtil.nexusFailureToTemporalFailure(failureInfo, false);
 
     // Verify nested structure is preserved
     Assert.assertEquals("outer failure", reconstructed.getMessage());
@@ -175,7 +175,7 @@ public void testNexusFailureWithStackTracePreservation() {
     }
     FailureInfo failureInfo = failureInfoBuilder.build();
 
-    Failure reconstructed = NexusUtil.nexusFailureToAPIFailure(failureInfo, true);
+    Failure reconstructed = NexusUtil.nexusFailureToTemporalFailure(failureInfo, true);
     Assert.assertEquals(stackTrace, reconstructed.getStackTrace());
   }
 
@@ -189,7 +189,7 @@ public void testNexusFailureWithoutTemporalMetadata() {
             .setDetailsJson("{\"detail\":\"some data\"}")
             .build();
 
-    Failure apiFailure = NexusUtil.nexusFailureToAPIFailure(failureInfo, true);
+    Failure apiFailure = NexusUtil.nexusFailureToTemporalFailure(failureInfo, true);
 
     // Should be wrapped as NexusFailure type
     Assert.assertEquals("generic nexus failure", apiFailure.getMessage());
@@ -235,7 +235,7 @@ public void testDeeplyNestedFailureCauses() {
     // Convert through Nexus format and back
     io.temporal.api.nexus.v1.Failure nexusFailure = NexusUtil.temporalFailureToNexusFailure(level1);
     FailureInfo failureInfo = NexusUtil.temporalFailureToNexusFailureInfo(level1);
-    Failure reconstructed = NexusUtil.nexusFailureToAPIFailure(failureInfo, true);
+    Failure reconstructed = NexusUtil.nexusFailureToTemporalFailure(failureInfo, true);
 
     // Verify all levels are preserved
     Assert.assertEquals("level 1", reconstructed.getMessage());
@@ -253,7 +253,7 @@ public void testCanceledFailureConversion() {
             .build();
 
     FailureInfo failureInfo = NexusUtil.temporalFailureToNexusFailureInfo(canceledFailure);
-    Failure reconstructed = NexusUtil.nexusFailureToAPIFailure(failureInfo, true);
+    Failure reconstructed = NexusUtil.nexusFailureToTemporalFailure(failureInfo, true);
 
     Assert.assertEquals("operation canceled", reconstructed.getMessage());
     Assert.assertTrue(reconstructed.hasCanceledFailureInfo());

temporal-sdk/src/test/java/io/temporal/workflow/updateTest/UpdateWithStartTest.java

@@ -646,7 +646,7 @@ public void failWhenUpdateNamesDoNotMatch() {
   }
 
   @Test
-  @SuppressWarnings("deprecation")
+  @SuppressWarnings("deprecation") // Tests validation of deprecated TERMINATE_IF_RUNNING policy
   public void failServerSideWhenStartIsInvalid() {
     WorkflowClient workflowClient = testWorkflowRule.getWorkflowClient();
 

temporal-test-server/src/main/java/io/temporal/internal/testservice/TestWorkflowService.java

@@ -310,6 +310,7 @@ public void startWorkflowExecution(
     }
   }
 
+  @SuppressWarnings("deprecation") // Handles deprecated TERMINATE_IF_RUNNING reuse policy
   StartWorkflowExecutionResponse startWorkflowExecutionImpl(
       StartWorkflowExecutionRequest startRequest,
       Duration backoffStartInterval,
@@ -475,7 +476,7 @@ private StartWorkflowExecutionResponse throwDuplicatedWorkflow(
         WorkflowExecutionAlreadyStartedFailure.getDescriptor());
   }
 
-  @SuppressWarnings("deprecation")
+  @SuppressWarnings("deprecation") // Validates deprecated TERMINATE_IF_RUNNING reuse policy
   private void validateWorkflowIdReusePolicy(
       WorkflowIdReusePolicy reusePolicy, WorkflowIdConflictPolicy conflictPolicy) {
     if (conflictPolicy != WorkflowIdConflictPolicy.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED
@@ -1039,14 +1040,20 @@ public void respondNexusTaskCompleted(
     }
   }
 
-  @SuppressWarnings("deprecation")
+  @SuppressWarnings("deprecation") // to allow using Failure in the old and new formats
   @Override
   public void respondNexusTaskFailed(
       RespondNexusTaskFailedRequest request,
       StreamObserver<RespondNexusTaskFailedResponse> responseObserver) {
     try {
       Failure failure;
       if (request.hasFailure()) {
+        if (!request.getFailure().hasNexusHandlerFailureInfo()) {
+          throw Status.INVALID_ARGUMENT
+              .withDescription(
+                  "request Failure must contain error or failure with NexusHandlerFailureInfo")
+              .asRuntimeException();
+        }
         // New format: Failure directly contains the handler error with NexusHandlerFailureInfo
         // Don't wrap with NexusOperationFailureInfo - the state machine will do that if needed
         failure = request.getFailure();
@@ -1159,7 +1166,7 @@ private static boolean hasFailureContent(io.temporal.api.nexus.v1.Failure failur
   }
 
   /**
-   * nexusFailureToAPIFailure converts a Nexus Failure to an API proto Failure. If the failure
+   * nexusFailureToTemporalFailure converts a Nexus Failure to an API proto Failure. If the failure
    * metadata "type" field is set to the fullname of the temporal API Failure message, the failure
    * is reconstructed using protojson.Unmarshal on the failure details field.
    */

temporal-test-server/src/test/java/io/temporal/testserver/functional/NexusWorkflowTest.java

@@ -1102,7 +1102,7 @@ private CompletableFuture<Request> completeNexusTask(
         });
   }
 
-  @SuppressWarnings("deprecation")
+  @SuppressWarnings("deprecation") // Uses deprecated HandlerError/setError() to test old format
   private CompletableFuture<RespondNexusTaskFailedResponse> failNexusTask(
       ByteString taskToken, HandlerError err) {
     return CompletableFuture.supplyAsync(

temporal-test-server/src/test/java/io/temporal/testserver/functional/WorkflowIdReusePolicyTest.java

@@ -83,7 +83,7 @@ public void alreadyRunningWorkflowBlocksSecondEvenWithAllowDuplicate() {
   @SuppressWarnings("deprecation") // Test for deprecated policy behavior
   public void secondWorkflowTerminatesFirst() {
     String workflowId = "terminate-if-running-1";
-    @SuppressWarnings("deprecation")
+    @SuppressWarnings("deprecation") // Tests deprecated TERMINATE_IF_RUNNING reuse policy
     WorkflowOptions options =
         WorkflowOptions.newBuilder()
             .setWorkflowId(workflowId)