Fix NPE when canceling workflow without run id (#1150)

Author: vytautas-karpavicius

internal/compatibility/proto/decision.go

@@ -108,7 +108,7 @@ func Decision(d *shared.Decision) *apiv1.Decision {
 		decision.Attributes = &apiv1.Decision_RequestCancelExternalWorkflowExecutionDecisionAttributes{
 			RequestCancelExternalWorkflowExecutionDecisionAttributes: &apiv1.RequestCancelExternalWorkflowExecutionDecisionAttributes{
 				Domain:            attr.GetDomain(),
-				WorkflowExecution: WorkflowRunPair(attr.WorkflowId, attr.RunId),
+				WorkflowExecution: WorkflowRunPair(attr.GetWorkflowId(), attr.GetRunId()),
 				Control:           attr.Control,
 				ChildWorkflowOnly: attr.GetChildWorkflowOnly(),
 			},

internal/compatibility/proto/request.go

@@ -182,7 +182,7 @@ func RecordActivityTaskHeartbeatByIdRequest(t *shared.RecordActivityTaskHeartbea
 	}
 	return &apiv1.RecordActivityTaskHeartbeatByIDRequest{
 		Domain:            t.GetDomain(),
-		WorkflowExecution: WorkflowRunPair(t.WorkflowID, t.RunID),
+		WorkflowExecution: WorkflowRunPair(t.GetWorkflowID(), t.GetRunID()),
 		ActivityId:        t.GetActivityID(),
 		Details:           Payload(t.Details),
 		Identity:          t.GetIdentity(),
@@ -263,7 +263,7 @@ func RespondActivityTaskCanceledByIdRequest(t *shared.RespondActivityTaskCancele
 	}
 	return &apiv1.RespondActivityTaskCanceledByIDRequest{
 		Domain:            t.GetDomain(),
-		WorkflowExecution: WorkflowRunPair(t.WorkflowID, t.RunID),
+		WorkflowExecution: WorkflowRunPair(t.GetWorkflowID(), t.GetRunID()),
 		ActivityId:        t.GetActivityID(),
 		Details:           Payload(t.Details),
 		Identity:          t.GetIdentity(),
@@ -287,7 +287,7 @@ func RespondActivityTaskCompletedByIdRequest(t *shared.RespondActivityTaskComple
 	}
 	return &apiv1.RespondActivityTaskCompletedByIDRequest{
 		Domain:            t.GetDomain(),
-		WorkflowExecution: WorkflowRunPair(t.WorkflowID, t.RunID),
+		WorkflowExecution: WorkflowRunPair(t.GetWorkflowID(), t.GetRunID()),
 		ActivityId:        t.GetActivityID(),
 		Result:            Payload(t.Result),
 		Identity:          t.GetIdentity(),
@@ -311,7 +311,7 @@ func RespondActivityTaskFailedByIdRequest(t *shared.RespondActivityTaskFailedByI
 	}
 	return &apiv1.RespondActivityTaskFailedByIDRequest{
 		Domain:            t.GetDomain(),
-		WorkflowExecution: WorkflowRunPair(t.WorkflowID, t.RunID),
+		WorkflowExecution: WorkflowRunPair(t.GetWorkflowID(), t.GetRunID()),
 		ActivityId:        t.GetActivityID(),
 		Failure:           Failure(t.Reason, t.Details),
 		Identity:          t.GetIdentity(),

internal/compatibility/proto/types.go

@@ -57,13 +57,13 @@ func WorkflowExecution(t *shared.WorkflowExecution) *apiv1.WorkflowExecution {
 	}
 }
 
-func WorkflowRunPair(workflowId, runId *string) *apiv1.WorkflowExecution {
-	if workflowId == nil && runId == nil {
+func WorkflowRunPair(workflowId, runId string) *apiv1.WorkflowExecution {
+	if workflowId == "" && runId == "" {
 		return nil
 	}
 	return &apiv1.WorkflowExecution{
-		WorkflowId: *workflowId,
-		RunId:      *runId,
+		WorkflowId: workflowId,
+		RunId:      runId,
 	}
 }
 
@@ -471,7 +471,7 @@ func PendingChildExecutionInfo(t *shared.PendingChildExecutionInfo) *apiv1.Pendi
 		return nil
 	}
 	return &apiv1.PendingChildExecutionInfo{
-		WorkflowExecution: WorkflowRunPair(t.WorkflowID, t.RunID),
+		WorkflowExecution: WorkflowRunPair(t.GetWorkflowID(), t.GetRunID()),
 		WorkflowTypeName:  t.GetWorkflowTypName(),
 		InitiatedId:       t.GetInitiatedID(),
 		ParentClosePolicy: ParentClosePolicy(t.ParentClosePolicy),

test/integration_test.go

@@ -433,6 +433,15 @@ func (ts *IntegrationTestSuite) TestChildWFWithParentClosePolicyAbandon() {
 	ts.True(resp.WorkflowExecutionInfo.GetCloseTime() == 0)
 }
 
+func (ts *IntegrationTestSuite) TestChildWFCancel() {
+	var childWorkflowID string
+	err := ts.executeWorkflow("test-childwf-cancel", ts.workflows.ChildWorkflowCancel, &childWorkflowID)
+	ts.NoError(err)
+	resp, err := ts.libClient.DescribeWorkflowExecution(context.Background(), childWorkflowID, "")
+	ts.NoError(err)
+	ts.Equal(shared.WorkflowExecutionCloseStatusCanceled, resp.WorkflowExecutionInfo.GetCloseStatus())
+}
+
 func (ts *IntegrationTestSuite) TestActivityCancelUsingReplay() {
 	replayer := worker.NewWorkflowReplayer()
 	replayer.RegisterWorkflowWithOptions(ts.workflows.ActivityCancelRepro, workflow.RegisterOptions{DisableAlreadyRegisteredCheck: true})

test/workflow_test.go

@@ -355,6 +355,28 @@ func (w *Workflows) ChildWorkflowSuccessWithParentClosePolicyAbandon(ctx workflo
 	return childWE.ID, err
 }
 
+func (w *Workflows) ChildWorkflowCancel(ctx workflow.Context) (result string, err error) {
+	opts := workflow.ChildWorkflowOptions{
+		TaskStartToCloseTimeout:      5 * time.Second,
+		ExecutionStartToCloseTimeout: 30 * time.Second,
+	}
+	childCtx := workflow.WithChildOptions(ctx, opts)
+	childCtx, cancel := workflow.WithCancel(childCtx)
+	ft := workflow.ExecuteChildWorkflow(childCtx, w.sleep, 20*time.Second)
+	err = workflow.Sleep(ctx, time.Second)
+	if err != nil {
+		return "", err
+	}
+	cancel()
+	err = workflow.Sleep(ctx, time.Second)
+	if err != nil {
+		return "", err
+	}
+	var childWE internal.WorkflowExecution
+	err = ft.GetChildWorkflowExecution().Get(ctx, &childWE)
+	return childWE.ID, err
+}
+
 func (w *Workflows) ActivityCancelRepro(ctx workflow.Context) ([]string, error) {
 	ctx, cancelFunc := workflow.WithCancel(ctx)
 
@@ -567,6 +589,7 @@ func (w *Workflows) register(worker worker.Worker) {
 	worker.RegisterWorkflow(w.ChildWorkflowSuccess)
 	worker.RegisterWorkflow(w.ChildWorkflowSuccessWithParentClosePolicyTerminate)
 	worker.RegisterWorkflow(w.ChildWorkflowSuccessWithParentClosePolicyAbandon)
+	worker.RegisterWorkflow(w.ChildWorkflowCancel)
 	worker.RegisterWorkflow(w.InspectActivityInfo)
 	worker.RegisterWorkflow(w.InspectLocalActivityInfo)
 	worker.RegisterWorkflow(w.sleep)