ReadOnly Exception in update validator fail WFT (#1918)

ReadOnly Exception in update validator fail WFT

Author: Quinn-With-Two-Ns

temporal-sdk/src/main/java/io/temporal/internal/sync/ReadOnlyException.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2022 Temporal Technologies, Inc. All Rights Reserved.
+ *
+ * Copyright (C) 2012-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Modifications copyright (C) 2017 Uber Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this material except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.temporal.internal.sync;
+
+/**
+ * This exception is thrown when a workflow tries to perform an operation that could generate a new
+ * command while it is in a read only context.
+ */
+public class ReadOnlyException extends IllegalStateException {
+  public ReadOnlyException(String action) {
+    super("While in read-only function, action attempted: " + action);
+  }
+}

temporal-sdk/src/main/java/io/temporal/internal/sync/SyncWorkflow.java

@@ -163,6 +163,9 @@ public void handleUpdate(
             try {
               workflowContext.setReadOnly(true);
               workflowProc.handleValidateUpdate(updateName, input, eventId, header);
+            } catch (ReadOnlyException r) {
+              // Rethrow instead on rejecting the update to fail the WFT
+              throw r;
             } catch (Exception e) {
               callbacks.reject(this.dataConverter.exceptionToFailure(e));
               return;

temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInternal.java

@@ -744,7 +744,7 @@ static boolean isReadOnly() {
 
   static void assertNotReadOnly(String action) {
     if (isReadOnly()) {
-      throw new IllegalStateException("While in read-only function, action attempted:" + action);
+      throw new ReadOnlyException(action);
     }
   }
 

temporal-sdk/src/test/java/io/temporal/workflow/shared/TestWorkflows.java

@@ -285,7 +285,7 @@ public String execute(String arg) {
     }
   }
 
-  public static String[] illegalCallCases = {
+  public static final String[] illegalCallCases = {
     "start_activity",
     "start_local_activity",
     "upsert_search_attribute",
@@ -312,7 +312,7 @@ public static void illegalCalls(String testCase) {
         Workflow.newLocalActivityStub(
                 TestActivities.TestActivity1.class,
                 LocalActivityOptions.newBuilder()
-                    .setScheduleToStartTimeout(Duration.ofHours(1))
+                    .setScheduleToCloseTimeout(Duration.ofHours(1))
                     .build())
             .execute("test");
         break;

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

@@ -21,7 +21,6 @@
 package io.temporal.workflow.updateTest;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThrows;
 
 import io.temporal.activity.*;
 import io.temporal.api.common.v1.WorkflowExecution;
@@ -33,6 +32,7 @@
 import io.temporal.workflow.Workflow;
 import io.temporal.workflow.shared.TestActivities;
 import io.temporal.workflow.shared.TestWorkflows;
+import java.time.Duration;
 import java.util.Optional;
 import java.util.UUID;
 import org.junit.Rule;
@@ -41,6 +41,7 @@
 import org.slf4j.LoggerFactory;
 
 public class UpdateBadValidator {
+  private static int testWorkflowTaskFailureReplayCount;
 
   private static final Logger log = LoggerFactory.getLogger(UpdateTest.class);
 
@@ -51,13 +52,14 @@ public class UpdateBadValidator {
           .setWorkflowTypes(TestUpdateWithBadValidatorWorkflowImpl.class)
           .build();
 
-  @Test
+  @Test(timeout = 30000)
   public void testBadUpdateValidator() {
     String workflowId = UUID.randomUUID().toString();
     WorkflowClient workflowClient = testWorkflowRule.getWorkflowClient();
     WorkflowOptions options =
         SDKTestOptions.newWorkflowOptionsWithTimeouts(testWorkflowRule.getTaskQueue()).toBuilder()
             .setWorkflowId(workflowId)
+            .setWorkflowTaskTimeout(Duration.ofSeconds(1))
             .build();
     TestWorkflows.WorkflowWithUpdate workflow =
         workflowClient.newWorkflowStub(TestWorkflows.WorkflowWithUpdate.class, options);
@@ -69,9 +71,9 @@ public void testBadUpdateValidator() {
     assertEquals("initial", workflow.getState());
 
     assertEquals(workflowId, execution.getWorkflowId());
-
     for (String testCase : TestWorkflows.illegalCallCases) {
-      assertThrows(WorkflowUpdateException.class, () -> workflow.update(0, testCase));
+      assertEquals("2", workflow.update(0, testCase));
+      testWorkflowTaskFailureReplayCount = 0;
     }
 
     workflow.complete();
@@ -102,12 +104,15 @@ public String getState() {
 
     @Override
     public String update(Integer index, String value) {
-      return "";
+      return String.valueOf(testWorkflowTaskFailureReplayCount);
     }
 
     @Override
     public void updateValidator(Integer index, String testCase) {
-      TestWorkflows.illegalCalls(testCase);
+      if (testWorkflowTaskFailureReplayCount < 2) {
+        testWorkflowTaskFailureReplayCount += 1;
+        TestWorkflows.illegalCalls(testCase);
+      }
     }
 
     @Override