Skip to content
27 changes: 20 additions & 7 deletions internal/internal_workflow_testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -2452,13 +2452,26 @@ func (env *testWorkflowEnvironmentImpl) RequestCancelChildWorkflow(_, workflowID

func (env *testWorkflowEnvironmentImpl) RequestCancelExternalWorkflow(namespace, workflowID, runID string, callback ResultHandler) {
if env.workflowInfo.WorkflowExecution.ID == workflowID {
// cancel current workflow
env.workflowCancelHandler()
// check if current workflow is a child workflow
if env.isChildWorkflow() && env.onChildWorkflowCanceledListener != nil {
env.postCallback(func() {
env.onChildWorkflowCanceledListener(env.workflowInfo)
}, false)
cancelFunc := func() {
env.workflowCancelHandler()

if env.isChildWorkflow() && env.onChildWorkflowCanceledListener != nil {
env.postCallback(func() {
env.onChildWorkflowCanceledListener(env.workflowInfo)
}, false)
}
}
// The way testWorkflowEnvironment is setup today, we close the child workflow dispatcher before calling
// the workflowCancelHandler. A larger refactor would be needed to handle this similar to non-test code.
// Maybe worth doing when https://github.com/temporalio/go-sdk/issues/50 is tackled.
if sd, ok := env.workflowDef.(*syncWorkflowDefinition); ok && env.isChildWorkflow() {
if !sd.dispatcher.IsClosed() {
sd.dispatcher.NewCoroutine(sd.rootCtx, "cancel-self", true, func(ctx Context) {
Comment thread
yuandrew marked this conversation as resolved.
cancelFunc()
})
}
} else {
cancelFunc()
Comment thread
yuandrew marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
}
return
} else if childHandle, ok := env.runningWorkflows[workflowID]; ok && !childHandle.handled {
Expand Down
34 changes: 34 additions & 0 deletions internal/workflow_testsuite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1234,6 +1234,40 @@ func TestDynamicWorkflows(t *testing.T) {
require.Equal(t, "dynamic-activity - grape - cherry", result)
}

func SleepHour(ctx Context) error {
return Sleep(ctx, time.Hour)
}

func SleepThenCancel(ctx Context) error {
cwf := ExecuteChildWorkflow(
ctx,
SleepHour,
)
var res WorkflowExecution
if err := cwf.GetChildWorkflowExecution().Get(ctx, &res); err != nil {
return err
}

// Canceling an external workflow that causes a timer to cancel used to fail due to
// "illegal access from outside of workflow context"
err := RequestCancelExternalWorkflow(ctx, res.ID, res.RunID).Get(ctx, nil)
if err != nil {
return err
}

// Give the workflow time to finish canceling the child workflow
return Sleep(ctx, 1*time.Second)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we poll the other workflow until it gets cancelled if we really need to give time for it to cancel? Maybe poll a generous overall time (5 seconds) and then either return on intentionally fail the test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comment was wrong, this is actually important part of the test that uses ctx to trigger the illegal access from outside of workflow context without the fix. Updated the comment

}

func TestRequestCancelExternalWorkflowInSelector(t *testing.T) {
testSuite := &WorkflowTestSuite{}
env := testSuite.NewTestWorkflowEnvironment()
env.RegisterWorkflow(SleepHour)
env.ExecuteWorkflow(SleepThenCancel)
require.NoError(t, env.GetWorkflowError())
env.IsWorkflowCompleted()
}
Comment thread
yuandrew marked this conversation as resolved.

func checkActivityInfo(ctx context.Context, isWorkflowActivity bool) error {
info := GetActivityInfo(ctx)
if isWorkflowActivity {
Expand Down