Failed Read-Only check causes a WFT failure in validator (#1329)

temporalio · Dec 30, 2023 · 5a64898 · 5a64898
1 parent 95fe6ab
commit 5a64898
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 11 deletions.
diff --git a/internal/internal_update.go b/internal/internal_update.go
@@ -353,6 +353,11 @@ func newUpdateHandler(
 func (h *updateHandler) validate(ctx Context, input []interface{}) (err error) {
 	defer func() {
 		if p := recover(); p != nil {
+			if p == panicIllegalAccessCoroutineState {
+				// Don't handle the panic since this error means the workflow state is
+				// likely corrupted and should be discarded.
+				panic(p)
+			}
 			st := getStackTraceRaw("update validator [panic]:", 7, 0)
 			err = newPanicError(fmt.Sprintf("update validator panic: %v", p), st)
 		}

diff --git a/internal/internal_update_test.go b/internal/internal_update_test.go
@@ -246,6 +246,22 @@ func TestDefaultUpdateHandler(t *testing.T) {
 		require.Equal(t, validatorFunc(ctx, argStr), rejectErr)
 	})
 
+	t.Run("illegal state panic from validator", func(t *testing.T) {
+		updateFunc := func(Context, string) error { panic("should not get called") }
+		validatorFunc := func(Context, string) error { panic(panicIllegalAccessCoroutineState) }
+		mustSetUpdateHandler(
+			t,
+			ctx,
+			t.Name(),
+			updateFunc,
+			UpdateHandlerOptions{Validator: validatorFunc},
+		)
+
+		require.Panics(t, func() {
+			defaultUpdateHandler(ctx, t.Name(), "testID", args, hdr, &testUpdateCallbacks{}, runOnCallingThread)
+		})
+	})
+
 	t.Run("error from update func", func(t *testing.T) {
 		updateFunc := func(Context, string) error { return errors.New("expected") }
 		mustSetUpdateHandler(t, ctx, t.Name(), updateFunc, UpdateHandlerOptions{})

diff --git a/internal/internal_workflow_test.go b/internal/internal_workflow_test.go
@@ -1438,15 +1438,11 @@ func (s *WorkflowUnitTest) Test_MutatingFunctionsInUpdateValidator() {
 	}
 	env.RegisterWorkflow(wf)
 	env.RegisterDelayedCallback(func() {
-		env.UpdateWorkflow(updateType, "testID", &updateCallback{
-			reject: func(err error) {
-				s.Error(err)
-			},
-		})
+		env.UpdateWorkflow(updateType, "testID", &updateCallback{})
 	}, time.Second)
 	env.ExecuteWorkflow(wf)
 	s.True(env.IsWorkflowCompleted())
-	s.NoError(env.GetWorkflowError())
+	s.Error(env.GetWorkflowError())
 }
 
 func (s *WorkflowUnitTest) Test_StaleGoroutinesAreShutDown() {

diff --git a/test/integration_test.go b/test/integration_test.go
@@ -1250,11 +1250,12 @@ func (ts *IntegrationTestSuite) TestMutatingUpdateValidator() {
 	run, err := ts.client.ExecuteWorkflow(ctx,
 		ts.startWorkflowOptions("test-mutating-update-validator"), ts.workflows.MutatingUpdateValidatorWorkflow)
 	ts.Nil(err)
-	handler, err := ts.client.UpdateWorkflow(ctx, "test-mutating-update-validator", run.GetRunID(), "mutating_update")
-	ts.NoError(err)
+	go func() {
+		_, err = ts.client.UpdateWorkflow(ctx, "test-mutating-update-validator", run.GetRunID(), "mutating_update")
+	}()
 
-	ts.Error(handler.Get(ctx, nil))
-	ts.Nil(ts.client.CancelWorkflow(ctx, "test-mutating-update-validator", ""))
+	wfErr := run.Get(ctx, nil)
+	ts.Error(wfErr)
 }
 
 func (ts *IntegrationTestSuite) TestWaitForCancelWithDisconnectedContext() {

diff --git a/test/workflow_test.go b/test/workflow_test.go
@@ -931,7 +931,6 @@ func (w *Workflows) MutatingQueryWorkflow(ctx workflow.Context) (string, error)
 
 func (w *Workflows) MutatingUpdateValidatorWorkflow(ctx workflow.Context) (string, error) {
 	err := workflow.SetUpdateHandlerWithOptions(ctx, "mutating_update", func(ctx workflow.Context) (string, error) {
-		_ = workflow.Sleep(ctx, time.Second)
 		return "failed", nil
 	}, workflow.UpdateHandlerOptions{
 		Validator: func(ctx workflow.Context) error {