From 167fd686971b215c0b695fd4aaf5be65e00d7508 Mon Sep 17 00:00:00 2001 From: Roey Berman Date: Fri, 14 Aug 2026 15:48:59 -0700 Subject: [PATCH 1/2] Report non-retryable PayloadValidationError as BAD_REQUEST A data converter can signal that a Nexus operation's input is invalid by throwing a non-retryable ApplicationFailure of type PayloadValidationError while deserializing the input. Such a failure is now translated into a BAD_REQUEST HandlerException retaining the original failure as its cause, instead of the INTERNAL handler error any other application failure produces. Application failures of any other type, and retryable PayloadValidationError failures, keep their existing behavior. --- .../internal/nexus/PayloadSerializer.java | 28 ++++++++-- .../internal/nexus/PayloadSerializerTest.java | 53 +++++++++++++++++++ ...utDeserializationErrorPropagationTest.java | 47 +++++++++++++++- 3 files changed, 122 insertions(+), 6 deletions(-) diff --git a/temporal-sdk/src/main/java/io/temporal/internal/nexus/PayloadSerializer.java b/temporal-sdk/src/main/java/io/temporal/internal/nexus/PayloadSerializer.java index 1e517dd03..97127768c 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/nexus/PayloadSerializer.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/nexus/PayloadSerializer.java @@ -21,10 +21,12 @@ * reports for them is decided here. * *

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. * *

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 @@ -32,6 +34,13 @@ * 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) { diff --git a/temporal-sdk/src/test/java/io/temporal/internal/nexus/PayloadSerializerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/nexus/PayloadSerializerTest.java index 544fbb873..51641caed 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/nexus/PayloadSerializerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/nexus/PayloadSerializerTest.java @@ -125,6 +125,59 @@ 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.assertSame(original, e.getCause()); + Assert.assertEquals("invalid input", original.getOriginalMessage()); + Assert.assertSame(cause, e.getCause().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 diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/nexus/OperationInputDeserializationErrorPropagationTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/nexus/OperationInputDeserializationErrorPropagationTest.java index 1a9731852..8c0b5cf53 100644 --- a/temporal-sdk/src/test/java/io/temporal/workflow/nexus/OperationInputDeserializationErrorPropagationTest.java +++ b/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,38 @@ 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 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 +284,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: From 2bc62f523d3c8431b0cda2de9eb354431bdd4665 Mon Sep 17 00:00:00 2001 From: Roey Berman Date: Fri, 14 Aug 2026 15:59:21 -0700 Subject: [PATCH 2/2] Assert the bad request cause is an ApplicationFailure of the validation type --- .../internal/nexus/PayloadSerializerTest.java | 12 +++++++++--- ...tionInputDeserializationErrorPropagationTest.java | 3 +++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/temporal-sdk/src/test/java/io/temporal/internal/nexus/PayloadSerializerTest.java b/temporal-sdk/src/test/java/io/temporal/internal/nexus/PayloadSerializerTest.java index 51641caed..380cbd735 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/nexus/PayloadSerializerTest.java +++ b/temporal-sdk/src/test/java/io/temporal/internal/nexus/PayloadSerializerTest.java @@ -142,9 +142,15 @@ public void testDeserializeNonRetryablePayloadValidationErrorIsNonRetryableBadRe 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.assertSame(original, e.getCause()); - Assert.assertEquals("invalid input", original.getOriginalMessage()); - Assert.assertSame(cause, e.getCause().getCause()); + 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 diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/nexus/OperationInputDeserializationErrorPropagationTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/nexus/OperationInputDeserializationErrorPropagationTest.java index 8c0b5cf53..e3f4bc179 100644 --- a/temporal-sdk/src/test/java/io/temporal/workflow/nexus/OperationInputDeserializationErrorPropagationTest.java +++ b/temporal-sdk/src/test/java/io/temporal/workflow/nexus/OperationInputDeserializationErrorPropagationTest.java @@ -140,6 +140,9 @@ public void nonRetryablePayloadValidationErrorBecomesNonRetryableBadRequest() { // 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"));