Report non-retryable PayloadValidationError as BAD_REQUEST

temporal-sdk/src/main/java/io/temporal/internal/nexus/PayloadSerializer.java

@@ -21,17 +21,26 @@
  * reports for them is decided here.
  *
  * <p>Input that will never decode into the expected type is the caller's fault and is reported as a
- * non-retryable {@link HandlerException.ErrorType#BAD_REQUEST}. Any other failure on the way to the
- * value, a {@link io.temporal.payload.codec.PayloadCodec} outage for example, may well succeed on a
- * retry, so it is left to the handling in {@link NexusTaskHandlerImpl} that a failure from an
- * operation handler would get.
+ * non-retryable {@link HandlerException.ErrorType#BAD_REQUEST}. A data converter can also opt into
+ * that treatment for input it decoded but rejected, by throwing a non-retryable {@link
+ * ApplicationFailure} of type {@value #PAYLOAD_VALIDATION_ERROR_TYPE}. Any other failure on the way
+ * to the value, a {@link io.temporal.payload.codec.PayloadCodec} outage for example, may well
+ * succeed on a retry, so it is left to the handling in {@link NexusTaskHandlerImpl} that a failure
+ * from an operation handler would get.
  *
  * <p>Serializing an operation result is not translated at all. Note that this still means a
  * converter can choose the outcome: a non-retryable {@link ApplicationFailure} raised while
  * serializing a result becomes a non-retryable {@code INTERNAL} handler error by way of {@link
  * NexusTaskHandlerImpl}, and anything else keeps the retryable {@code INTERNAL} default.
  */
 class PayloadSerializer implements Serializer {
+  /**
+   * {@link ApplicationFailure#getType()} a data converter uses to say it understood the input but
+   * considers it invalid. When non-retryable, it is reported as {@link
+   * HandlerException.ErrorType#BAD_REQUEST} rather than as a handler-side {@code INTERNAL} error.
+   */
+  static final String PAYLOAD_VALIDATION_ERROR_TYPE = "PayloadValidationError";
+
   private final DataConverter dataConverter;
 
   PayloadSerializer(DataConverter dataConverter) {
@@ -64,7 +73,16 @@ public Content serialize(@Nullable Object o) {
             null,
             HandlerException.RetryBehavior.NON_RETRYABLE);
       }
-    } catch (HandlerException | ApplicationFailure e) {
+    } catch (ApplicationFailure e) {
+      if (e.isNonRetryable() && PAYLOAD_VALIDATION_ERROR_TYPE.equals(e.getType())) {
+        // The data converter decoded the input and rejected it, so this is the caller's fault
+        // rather than a handler-side error.
+        throw new HandlerException(
+            HandlerException.ErrorType.BAD_REQUEST, "invalid operation input", e);
+      }
+      // Otherwise the data converter already picked an error type and retry behavior, keep them.
+      throw e;
+    } catch (HandlerException e) {
       // The data converter already picked an error type and retry behavior, keep them.
       throw e;
     } catch (InvalidProtocolBufferException | DataConverterException e) {

temporal-sdk/src/test/java/io/temporal/internal/nexus/PayloadSerializerTest.java

@@ -125,6 +125,65 @@ public void testDeserializeApplicationFailureIsPropagatedAsIs() {
             ApplicationFailure.class, () -> serializer.deserialize(content, String.class)));
   }
 
+  @Test
+  public void testDeserializeNonRetryablePayloadValidationErrorIsNonRetryableBadRequest() {
+    // The converter understood the input and rejected it, which makes this the caller's fault.
+    RuntimeException cause = new RuntimeException("field 'name' must not be empty");
+    ApplicationFailure original =
+        ApplicationFailure.newNonRetryableFailureWithCause(
+            "invalid input", PayloadSerializer.PAYLOAD_VALIDATION_ERROR_TYPE, cause);
+    PayloadSerializer serializer = failingSerializer(null, original);
+    PayloadSerializer.Content content = payloadSerializer.serialize("test");
+
+    HandlerException e =
+        Assert.assertThrows(
+            HandlerException.class, () -> serializer.deserialize(content, String.class));
+    Assert.assertEquals(HandlerException.ErrorType.BAD_REQUEST, e.getErrorType());
+    Assert.assertFalse(e.isRetryable());
+    Assert.assertEquals("invalid operation input", e.getMessage());
+    // The converter's own message is not in the wrapper, so it has to survive on the cause.
+    Assert.assertTrue(
+        "expected an ApplicationFailure cause, got " + e.getCause(),
+        e.getCause() instanceof ApplicationFailure);
+    ApplicationFailure causeFailure = (ApplicationFailure) e.getCause();
+    Assert.assertSame(original, causeFailure);
+    Assert.assertEquals(PayloadSerializer.PAYLOAD_VALIDATION_ERROR_TYPE, causeFailure.getType());
+    Assert.assertTrue(causeFailure.isNonRetryable());
+    Assert.assertEquals("invalid input", causeFailure.getOriginalMessage());
+    Assert.assertSame(cause, causeFailure.getCause());
+  }
+
+  @Test
+  public void testDeserializeNonRetryableOtherApplicationFailureTypeIsPropagatedAsIs() {
+    // Only the PayloadValidationError type opts into BAD_REQUEST, everything else keeps the
+    // non-retryable INTERNAL handling NexusTaskHandlerImpl applies.
+    ApplicationFailure original =
+        ApplicationFailure.newNonRetryableFailure("invalid input", "SomeOtherValidationError");
+    PayloadSerializer serializer = failingSerializer(null, original);
+    PayloadSerializer.Content content = payloadSerializer.serialize("test");
+
+    Assert.assertSame(
+        original,
+        Assert.assertThrows(
+            ApplicationFailure.class, () -> serializer.deserialize(content, String.class)));
+  }
+
+  @Test
+  public void testDeserializeRetryablePayloadValidationErrorIsPropagatedAsIs() {
+    // A retryable failure may succeed on a retry, so the type alone must not make it a
+    // non-retryable BAD_REQUEST.
+    ApplicationFailure original =
+        ApplicationFailure.newFailure(
+            "invalid input", PayloadSerializer.PAYLOAD_VALIDATION_ERROR_TYPE);
+    PayloadSerializer serializer = failingSerializer(null, original);
+    PayloadSerializer.Content content = payloadSerializer.serialize("test");
+
+    Assert.assertSame(
+        original,
+        Assert.assertThrows(
+            ApplicationFailure.class, () -> serializer.deserialize(content, String.class)));
+  }
+
   @Test
   public void testDeserializeTransientFailureIsNotTranslated() {
     // A payload codec outage is not the caller's fault and may succeed on a retry, so it must not

temporal-sdk/src/test/java/io/temporal/workflow/nexus/OperationInputDeserializationErrorPropagationTest.java

@@ -32,13 +32,20 @@
  * operation input. A {@link HandlerException} keeps the error type and retry behavior the converter
  * chose, and an {@link ApplicationFailure} is wrapped by {@link
  * io.temporal.internal.nexus.NexusTaskHandlerImpl} the same way one thrown from an operation
- * handler is. Neither is rewritten to BAD_REQUEST, which is what happens to every other failure.
+ * handler is. Neither is rewritten to BAD_REQUEST, which is what happens to every other failure,
+ * with one exception: a non-retryable {@code PayloadValidationError} is the converter's way of
+ * saying the input itself was invalid, so it is reported as a non-retryable BAD_REQUEST.
  */
 public class OperationInputDeserializationErrorPropagationTest {
   private static final String HANDLER_EXCEPTION = "handler-exception";
   private static final String NON_RETRYABLE_APPLICATION_FAILURE =
       "non-retryable-application-failure";
   private static final String RETRYABLE_APPLICATION_FAILURE = "retryable-application-failure";
+  private static final String NON_RETRYABLE_PAYLOAD_VALIDATION_ERROR =
+      "non-retryable-payload-validation-error";
+  private static final String RETRYABLE_PAYLOAD_VALIDATION_ERROR =
+      "retryable-payload-validation-error";
+  private static final String PAYLOAD_VALIDATION_ERROR_TYPE = "PayloadValidationError";
   private static final String CODEC_FAILURE = "codec-failure";
 
   private static final AtomicInteger deserializeAttempts = new AtomicInteger();
@@ -118,6 +125,41 @@ public void retryableApplicationFailureIsRetried() {
     assertRetriedUntilTimeout(RETRYABLE_APPLICATION_FAILURE);
   }
 
+  @Test
+  public void nonRetryablePayloadValidationErrorBecomesNonRetryableBadRequest() {
+    HandlerException handlerFailure =
+        executeAndGetHandlerException(NON_RETRYABLE_PAYLOAD_VALIDATION_ERROR);
+
+    // BAD_REQUEST rather than the INTERNAL every other non-retryable ApplicationFailure gets.
+    Assert.assertEquals(HandlerException.ErrorType.BAD_REQUEST, handlerFailure.getErrorType());
+    Assert.assertFalse(handlerFailure.isRetryable());
+    if (isUsingNewFormat()) {
+      Assert.assertEquals("invalid operation input", handlerFailure.getMessage());
+    }
+    // The wrapper message does not carry the converter's own message, so it has to survive on the
+    // cause for the caller to see why the input was rejected.
+    Throwable cause = handlerFailure.getCause();
+    Assert.assertNotNull(cause);
+    Assert.assertTrue(
+        "expected an ApplicationFailure cause, got " + cause, cause instanceof ApplicationFailure);
+    Assert.assertEquals("PayloadValidationError", ((ApplicationFailure) cause).getType());
+    Assert.assertTrue(
+        "expected the converter's message on the cause, got " + cause.getMessage(),
+        cause.getMessage().contains("intentional failure"));
+
+    Assert.assertEquals(1, deserializeAttempts.get());
+    Assert.assertEquals(0, operationInvocations.get());
+  }
+
+  /**
+   * The PayloadValidationError type only opts into BAD_REQUEST when the failure is non-retryable,
+   * so a retryable one keeps being retried.
+   */
+  @Test(timeout = 30000)
+  public void retryablePayloadValidationErrorIsRetried() {
+    assertRetriedUntilTimeout(RETRYABLE_PAYLOAD_VALIDATION_ERROR);
+  }
+
   /**
    * A payload codec outage is not the caller's fault and may resolve on its own, so it must not be
    * reported as a non-retryable BAD_REQUEST the way undeserializable input is.
@@ -245,6 +287,12 @@ private static RuntimeException failureFor(String mode) {
           return ApplicationFailure.newNonRetryableFailure("intentional failure", "TestFailure");
         case RETRYABLE_APPLICATION_FAILURE:
           return ApplicationFailure.newFailure("intentional failure", "TestFailure");
+        case NON_RETRYABLE_PAYLOAD_VALIDATION_ERROR:
+          return ApplicationFailure.newNonRetryableFailure(
+              "intentional failure", PAYLOAD_VALIDATION_ERROR_TYPE);
+        case RETRYABLE_PAYLOAD_VALIDATION_ERROR:
+          return ApplicationFailure.newFailure(
+              "intentional failure", PAYLOAD_VALIDATION_ERROR_TYPE);
         case CODEC_FAILURE:
           return new PayloadCodecException("intentional failure");
         default: