Skip to content

Update Validator doesn't serialize exceptions properly #2875

@Quinn-With-Two-Ns

Description

@Quinn-With-Two-Ns

Expected Behavior

Exceptions thrown in Update Validators are serialized the same if they are thrown in the Update Handler or Workflow

Actual Behavior

Exceptions thrown in Update Validators are serialized differently if they are thrown in the Update Handler or Workflow

Steps to Reproduce the Problem

package io.temporal.workflow.updateTest;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

import io.temporal.api.common.v1.WorkflowExecution;
import io.temporal.client.WorkflowClient;
import io.temporal.client.WorkflowOptions;
import io.temporal.client.WorkflowUpdateException;
import io.temporal.failure.ApplicationFailure;
import io.temporal.testing.internal.SDKTestOptions;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.workflow.CompletablePromise;
import io.temporal.workflow.QueryMethod;
import io.temporal.workflow.UpdateMethod;
import io.temporal.workflow.UpdateValidatorMethod;
import io.temporal.workflow.Workflow;
import io.temporal.workflow.WorkflowInterface;
import io.temporal.workflow.WorkflowMethod;
import java.util.UUID;
import org.junit.Rule;
import org.junit.Test;

/**
 * Reproduction: ApplicationFailure thrown from an update validator is not surfaced to the client
 * with the same fidelity as one thrown from an update handler. The validator path strips type,
 * non-retryable flag, details, and message; the handler path preserves them.
 *
 * <p>Root cause: {@code SyncWorkflow#handleUpdate} catches the validator exception as a generic
 * {@code Exception}. By the time the validator-thrown {@code ApplicationFailure} reaches that
 * catch, {@code WorkflowExecutionHandler#applyWorkflowFailurePolicyAndRethrow} has already wrapped
 * it in a {@code WorkflowExecutionException} (because {@code ApplicationFailure} is a {@code
 * TemporalFailure}). Calling {@code dataConverter.exceptionToFailure(WorkflowExecutionException)}
 * then falls through to the unrecognized-exception branch in {@code DefaultFailureConverter}, which
 * synthesizes a brand-new {@code ApplicationFailureInfo} with type =
 * "io.temporal.internal.worker.WorkflowExecutionException" and {@code nonRetryable=false} — the
 * original ApplicationFailure is discarded. The execute path doesn't hit this because it has a
 * dedicated {@code catch (WorkflowExecutionException e)} branch that calls {@code e.getFailure()}.
 */
public class UpdateValidatorFailureTypeTest {

  private static final String FAILURE_MESSAGE = "boom";
  private static final String FAILURE_TYPE = "RockError";
  private static final String FAILURE_DETAIL = "the-detail";

  @Rule
  public SDKTestWorkflowRule testWorkflowRule =
      SDKTestWorkflowRule.newBuilder().setWorkflowTypes(TestWorkflowImpl.class).build();

  @WorkflowInterface
  public interface FailingWorkflow {
    @WorkflowMethod
    String execute();

    @QueryMethod
    String getState();

    /** {@code mode} = "validator" -> validator throws; "handler" -> handler throws. */
    @UpdateMethod
    String update(String mode);

    @UpdateValidatorMethod(updateName = "update")
    void updateValidator(String mode);

    @UpdateMethod
    void complete();
  }

  public static class TestWorkflowImpl implements FailingWorkflow {
    private final CompletablePromise<Void> done = Workflow.newPromise();

    @Override
    public String execute() {
      done.get();
      return "ok";
    }

    @Override
    public String getState() {
      return "ready";
    }

    @Override
    public String update(String mode) {
      if ("handler".equals(mode)) {
        throw ApplicationFailure.newNonRetryableFailureWithCause(
            FAILURE_MESSAGE, FAILURE_TYPE, null, FAILURE_DETAIL);
      }
      return "should-not-reach";
    }

    @Override
    public void updateValidator(String mode) {
      if ("validator".equals(mode)) {
        throw ApplicationFailure.newNonRetryableFailureWithCause(
            FAILURE_MESSAGE, FAILURE_TYPE, null, FAILURE_DETAIL);
      }
    }

    @Override
    public void complete() {
      done.complete(null);
    }
  }

  @Test
  public void handlerThrownApplicationFailure_isPreservedOnClient() {
    FailingWorkflow workflow = startWorkflow();

    WorkflowUpdateException ex =
        assertThrows(WorkflowUpdateException.class, () -> workflow.update("handler"));

    assertCausePreservesApplicationFailure(ex, "handler path");
    workflow.complete();
  }

  @Test
  public void validatorThrownApplicationFailure_isPreservedOnClient() {
    FailingWorkflow workflow = startWorkflow();

    WorkflowUpdateException ex =
        assertThrows(WorkflowUpdateException.class, () -> workflow.update("validator"));

    // These assertions FAIL today — the validator path loses type, non-retryable flag,
    // message, and details. The cause comes back as ApplicationFailure with
    // type="io.temporal.internal.worker.WorkflowExecutionException" and nonRetryable=false.
    assertCausePreservesApplicationFailure(ex, "validator path");
    workflow.complete();
  }

  private FailingWorkflow startWorkflow() {
    WorkflowClient client = testWorkflowRule.getWorkflowClient();
    WorkflowOptions options =
        SDKTestOptions.newWorkflowOptionsWithTimeouts(testWorkflowRule.getTaskQueue()).toBuilder()
            .setWorkflowId(UUID.randomUUID().toString())
            .build();
    FailingWorkflow workflow = client.newWorkflowStub(FailingWorkflow.class, options);
    WorkflowExecution exec = WorkflowClient.start(workflow::execute);
    SDKTestWorkflowRule.waitForOKQuery(workflow);
    assertNotNull(exec.getRunId());
    return workflow;
  }

  private static void assertCausePreservesApplicationFailure(
      WorkflowUpdateException ex, String context) {
    Throwable cause = ex.getCause();
    assertNotNull(context + ": expected non-null cause", cause);
    assertTrue(
        context
            + ": expected cause to be ApplicationFailure but was "
            + cause.getClass().getName()
            + " ("
            + cause.getMessage()
            + ")",
        cause instanceof ApplicationFailure);

    ApplicationFailure af = (ApplicationFailure) cause;

    assertEquals(context + ": type", FAILURE_TYPE, af.getType());
    assertTrue(context + ": non-retryable", af.isNonRetryable());
    // ApplicationFailure prepends "message='...', type='...', nonRetryable=..." but the original
    // message text should still be present.
    assertTrue(
        context + ": message contained, got: " + af.getOriginalMessage(),
        af.getOriginalMessage().contains(FAILURE_MESSAGE));
    assertEquals(
        context + ": detail payload", FAILURE_DETAIL, af.getDetails().get(0, String.class));
    // Sanity: no extra cause was injected (handler path has none; validator path should match).
    assertNull(context + ": ApplicationFailure cause", af.getCause());
  }
}

Specifications

  • Version:
  • Platform:

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions