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..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 @@ -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 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..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 @@ -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: