Implement Nexus operation cancellation types by pdoerner · Pull Request #2520 · temporalio/sdk-java

pdoerner · 2025-05-13T20:33:45Z

What was changed

Implemented cancellation types for Nexus operations which specify what action to take when the caller context is cancelled.

ABANDON -> Do not request cancellation of the operation.
TRY_CANCEL -> Initiate a cancellation request and immediately report cancellation to the caller.
WAIT_REQUESTED -> Request cancellation of the operation and wait for confirmation that the request was received.
WAIT_COMPLETED -> Wait for operation completion. Default.
These options are set on NexusOperationOptions which are passed in when starting an operation.

Why?

To give users more control over what happens to their handler workflows when their caller workflows are cancelled.

closes #2247

Quinn-With-Two-Ns · 2025-05-13T21:21:23Z

temporal-sdk/src/main/java/io/temporal/workflow/NexusOperationCancellationType.java

+ * is canceled. The result of the cancellation independently of the type is a {@link
+ * CanceledFailure} thrown from the Nexus operation method.
+ */
+public enum NexusOperationCancellationType {


Are we releasing this as GA or will it be Pre-release/Public preview?

It will be GA

Shouldn't it be Pre-release/Public preview, we talked about it during the nexus crew stand up ~2 weeks ago?

IIRC we did not mark it as experimental in the Go SDK. I could do so here if you prefer, I forget that conversation.

We did end up marking it experimental for Go, lets mark it experimental here as well

Quinn-With-Two-Ns · 2025-05-13T21:22:35Z

temporal-sdk/src/main/java/io/temporal/workflow/NexusOperationOptions.java

+     * @param cancellationType the cancellation type for the Nexus operation
+     * @return this
+     */
+    public NexusOperationOptions.Builder setCancellationType(


Can we document the default?

Quinn-With-Two-Ns · 2025-05-13T21:24:27Z

temporal-sdk/src/main/java/io/temporal/internal/statemachines/WorkflowStateMachines.java

    };
  }

+  private void notifyNexusOperationCanceled(


We need to be careful we don't copy this bug from child workflows to nexus operations #2160

essentially not run the event loop if we are notified by an event

I think this point still needs to be addressed

Does it? I just removed the call to eventLoop(). Was there some other change required?

Well removing the call to eventLoop() doesn't address the issue because if we are not notified by an event we do need to run the event loop

In what situation would we not be notified by an event? I guess I don't totally understand the event loop logic or how to check where the notification came from.

Quinn-With-Two-Ns · 2025-06-11T14:46:54Z

Looks like this PR changes temporal-serviceclient/src/main/protocloud to a very old commit can you please fix?

Quinn-With-Two-Ns · 2025-06-11T14:49:29Z

temporal-sdk/src/main/java/io/temporal/internal/statemachines/NexusOperationStateMachine.java

  }
+
+  public Failure createCancelNexusOperationFailure(Failure cause) {
+    Failure canceledFailure = Failure.newBuilder(cause).setSource(JAVA_SDK).build();


The source should only be set to the JAVA_SDK on the very specific code path where the "operation canceled before it was started", not in general

Well it applies in any case where the SDK is generating the failure

Quinn-With-Two-Ns · 2025-06-11T14:57:04Z

...l-sdk/src/main/java/io/temporal/internal/statemachines/CancelNexusOperationStateMachine.java


  private void notifyCompleted() {
-    setInitialCommandEventId();
+    Failure canceledFailure =


Why are we passing a failure back on notifyCompleted? If you look at CancelExternalStateMachine which is basically the same state machine but for workflows we don't pass anything back

It is to pass back the specific Nexus operation failure. I think CancelExternal always has the same cause.

The completionCallback takes two parameters. The intention is the first parameter is for communicating success, the second is for communication failure. notifyCompleted is not a failure so it shouldn't pass a value on the second parameter

otherwise completionCallback should just take one parameter since you are never using the first parameter

Gotcha. Updated, let me know if that looks better.

temporal-sdk/src/test/java/io/temporal/workflow/nexus/CancelWorkflowAsyncOperationTest.java

Quinn-With-Two-Ns · 2025-06-11T16:03:19Z

temporal-sdk/src/main/java/io/temporal/internal/statemachines/WorkflowStateMachines.java

            stateMachineSink);
    return () -> {
      if (operation.isCancellable()) {
        operation.cancel();


hm, if NexusOperationCancellationType.ABANDON should we cancel?

I think if the operation hasn't started, we should still cancel yes. ABANDON indicates that we should not send the RequestCancel command which we do not do if the operation hasn't been started.

hm so looking at the child workflow cancellation

if (cancellationType == ChildWorkflowCancellationType.ABANDON) { notifyChildCanceled(completionCallback); return; } // The only time child can be canceled directly is before its start command // was sent out to the service. After that RequestCancelExternal should be used. if (child.isCancellable()) { child.cancel(); return; }

We don't cancel the child workflow if ChildWorkflowCancellationType.ABANDON

If the schedule command hasn't been sent, I mean. Not that the operation itself has not started.

I think most users understand ABANDON as we don't request cancellation of the entity (child workflow or nexus operation) regardless of what state the entity happens to be in

Oh okay. That makes sense.

Quinn-With-Two-Ns · 2025-06-11T18:04:26Z

temporal-sdk/src/test/java/io/temporal/workflow/nexus/CancelWorkflowAsyncOperationTest.java

+import org.junit.Rule;
+import org.junit.Test;
+
+public class CancelWorkflowAsyncOperationTest extends BaseNexusTest {


Does this work?

Suggested change

public class CancelWorkflowAsyncOperationTest extends BaseNexusTest {

public class CancelWorkflowAsyncOperationTest {

SDKTestWorkflowRule should do all the setup for this test

I did it this way only for the replay test since we cannot use the test rule to get the endpoint name from within the static workflow definition

Quinn-With-Two-Ns · 2025-06-11T18:21:56Z

temporal-sdk/src/test/java/io/temporal/workflow/nexus/CancelWorkflowAsyncOperationTest.java

+
+      NexusOperationHandle<Void> handle =
+          Workflow.startNexusOperation(serviceStub::operation, 2000L);
+      handle.getExecution().get();


Can we also have a test where we cancel with the different cancellation types before calling handle.getExecution().get(); to trigger the code path when we cancel before the command is sent

Quinn-With-Two-Ns · 2025-06-11T18:31:49Z

LGTM! would like a bit more test coverage on the start then immediately cancel code path, but otherwise look good.

Quinn-With-Two-Ns · 2025-06-12T00:24:20Z

temporal-sdk/src/main/java/io/temporal/workflow/NexusOperationOptions.java


+    /**
+     * Sets the cancellation type for the Nexus operation. Defaults to WAIT_COMPLETED. Note:
+     * EXPERIMENTAL


Oh sorry I missed this, for the Java SDK we have an annotation to mark APIs as experimental you can add @Experimental to the API and type

temporal-sdk/src/main/java/io/temporal/common/Experimental.java

Implement Nexus operation cancellation types

49d1ba7

pdoerner requested a review from Quinn-With-Two-Ns May 13, 2025 20:35

Quinn-With-Two-Ns reviewed May 13, 2025

View reviewed changes

pdoerner added 3 commits June 3, 2025 10:02

Merge branch 'master' into nexus-cancellation-types

42a0923

Merge branch 'master' into nexus-cancellation-types

7e2b565

tests and cleanup

49c06fe

pdoerner marked this pull request as ready for review June 11, 2025 14:37

pdoerner requested a review from a team as a code owner June 11, 2025 14:37

pdoerner requested a review from Quinn-With-Two-Ns June 11, 2025 14:37

Quinn-With-Two-Ns reviewed Jun 11, 2025

View reviewed changes

skip tests with external service

0e2d4bb

Quinn-With-Two-Ns reviewed Jun 11, 2025

View reviewed changes

temporal-sdk/src/test/java/io/temporal/workflow/nexus/CancelWorkflowAsyncOperationTest.java Outdated Show resolved Hide resolved

pdoerner added 3 commits June 11, 2025 08:27

feedback

bd699fc

feedback

80c9788

mark experimental

cccc6f1

Quinn-With-Two-Ns reviewed Jun 11, 2025

View reviewed changes

pdoerner added 2 commits June 11, 2025 09:05

formatting

449e5ad

check for abandon before command sent

d7f9504

pdoerner requested a review from Quinn-With-Two-Ns June 11, 2025 17:15

Quinn-With-Two-Ns reviewed Jun 11, 2025

View reviewed changes

pdoerner and others added 2 commits June 11, 2025 15:18

tests

fd8dd8b

Merge branch 'master' into nexus-cancellation-types

982c498

pdoerner requested a review from Quinn-With-Two-Ns June 11, 2025 22:49

Quinn-With-Two-Ns reviewed Jun 12, 2025

View reviewed changes

pdoerner and others added 2 commits June 12, 2025 09:31

experimental

c6de0ba

Merge branch 'master' into nexus-cancellation-types

d3587a3

pdoerner enabled auto-merge (squash) June 12, 2025 17:14

pdoerner requested a review from Quinn-With-Two-Ns June 12, 2025 18:07

Quinn-With-Two-Ns approved these changes Jun 12, 2025

View reviewed changes

pdoerner merged commit e7e3fa6 into master Jun 12, 2025
18 checks passed

pdoerner deleted the nexus-cancellation-types branch June 12, 2025 18:46

	public class CancelWorkflowAsyncOperationTest extends BaseNexusTest {
	public class CancelWorkflowAsyncOperationTest {

Conversation

pdoerner commented May 13, 2025 • edited by Quinn-With-Two-Ns Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was changed

Why?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Quinn-With-Two-Ns commented Jun 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Quinn-With-Two-Ns commented Jun 11, 2025

Uh oh!

Quinn-With-Two-Ns Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

pdoerner commented May 13, 2025 •

edited by Quinn-With-Two-Ns

Loading

Quinn-With-Two-Ns Jun 12, 2025 •

edited

Loading