Skip to content

Activity reset #1730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 3, 2025
Merged

Activity reset #1730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
156fd42
activity reset
THardy98 Jun 1, 2025
6d3d364
use current core commit
THardy98 Aug 18, 2025
600a776
fix test
THardy98 Aug 19, 2025
1aeb11a
add conditional branch for explicit cancellation log
THardy98 Aug 28, 2025
051ea89
address pr feedback, add pause + reset test
THardy98 Sep 2, 2025
cf9ecfc
make pause + reset test less flaky
THardy98 Sep 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions packages/client/src/async-completion-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ export class ActivityCancelledError extends Error {}
@SymbolBasedInstanceOfError('ActivityPausedError')
export class ActivityPausedError extends Error {}

/**
* Thrown by {@link AsyncCompletionClient.heartbeat} when the reporting Activity
* has been reset.
*/
@SymbolBasedInstanceOfError('ActivityResetError')
export class ActivityResetError extends Error {}

/**
* Options used to configure {@link AsyncCompletionClient}
*/
Expand Down Expand Up @@ -219,6 +226,7 @@ export class AsyncCompletionClient extends BaseClient {
const payloads = await encodeToPayloads(this.dataConverter, details);
let cancelRequested = false;
let paused = false;
let reset = false;
try {
if (taskTokenOrFullActivityId instanceof Uint8Array) {
const response = await this.workflowService.recordActivityTaskHeartbeat({
Expand All @@ -229,6 +237,7 @@ export class AsyncCompletionClient extends BaseClient {
});
cancelRequested = !!response.cancelRequested;
paused = !!response.activityPaused;
reset = !!response.activityReset;
} else {
const response = await this.workflowService.recordActivityTaskHeartbeatById({
identity: this.options.identity,
Expand All @@ -238,14 +247,18 @@ export class AsyncCompletionClient extends BaseClient {
});
cancelRequested = !!response.cancelRequested;
paused = !!response.activityPaused;
reset = !!response.activityReset;
}
} catch (err) {
this.handleError(err);
}
// Note that it is possible for a heartbeat response to have multiple fields
// set as true (i.e. cancelled and pause).
if (cancelRequested) {
throw new ActivityCancelledError('cancelled');
}
if (paused) {
} else if (reset) {
throw new ActivityResetError('reset');
} else if (paused) {
throw new ActivityPausedError('paused');
}
}
Expand Down
6 changes: 2 additions & 4 deletions packages/core-bridge/src/worker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,10 +672,8 @@ mod config {
self.local_activity_task_slot_supplier
.into_slot_supplier(&mut rbo),
);
tuner_holder.nexus_slot_options(
self.nexus_task_slot_supplier
.into_slot_supplier(&mut rbo)
);
tuner_holder
.nexus_slot_options(self.nexus_task_slot_supplier.into_slot_supplier(&mut rbo));
if let Some(rbo) = rbo {
tuner_holder.resource_based_options(rbo);
}
Expand Down
48 changes: 40 additions & 8 deletions packages/test/src/helpers-integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,11 @@ export function configurableHelpers<T>(
};
}

export async function setActivityPauseState(handle: WorkflowHandle, activityId: string, pause: boolean): Promise<void> {
export async function setActivityState(
handle: WorkflowHandle,
activityId: string,
state: 'pause' | 'unpause' | 'reset' | 'pause & reset'
): Promise<void> {
const desc = await handle.describe();
const req = {
namespace: handle.client.options.namespace,
Expand All @@ -306,22 +310,50 @@ export async function setActivityPauseState(handle: WorkflowHandle, activityId:
},
id: activityId,
};
if (pause) {
if (state === 'pause') {
await handle.client.workflowService.pauseActivity(req);
} else {
} else if (state === 'unpause') {
await handle.client.workflowService.unpauseActivity(req);
} else if (state === 'reset') {
await handle.client.workflowService.resetActivity({ ...req, resetHeartbeat: true });
} else {
await Promise.all([
handle.client.workflowService.pauseActivity(req),
handle.client.workflowService.resetActivity({ ...req, resetHeartbeat: true }),
]);
}
await waitUntil(async () => {
const { raw } = await handle.describe();
const activityInfo = raw.pendingActivities?.find((act) => act.activityId === activityId);
// If we are pausing: success when either
// • paused flag is true OR
// • the activity vanished (it completed / retried)
if (pause) return activityInfo ? activityInfo.paused ?? false : true;
// If we are unpausing: success when either
// • paused flag is false OR
// • the activity vanished (already completed)
return activityInfo ? !activityInfo.paused : true;
if (state === 'pause') {
if (!activityInfo) {
return true; // Activity vanished (completed/retried)
}
return activityInfo.paused ?? false;
} else if (state === 'unpause') {
// If we are unpausing: success when either
// • paused flag is false OR
// • the activity vanished (already completed)
return activityInfo ? !activityInfo.paused : true;
} else if (state === 'reset') {
// If we are resetting, success when either
// • heartbeat details have been reset OR
// • the activity vanished (completed / retried)
return activityInfo ? activityInfo.heartbeatDetails === null : true;
} else {
// If we are pausing & resetting, success when either
// • activity is paused AND heartbeat details have been reset OR
// • the activity vanished (completed / retried)
if (!activityInfo) {
return true; // Activity vanished (completed/retried)
}
const isPaused = activityInfo.paused ?? false;
const isHeartbeatReset = activityInfo.heartbeatDetails === null;
return isPaused && isHeartbeatReset;
}
}, 15000);
}

Expand Down
1 change: 0 additions & 1 deletion packages/test/src/test-integration-split-three.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ test(
await worker.runUntil(handle.result());
let firstChild = true;
const history = await handle.fetchHistory();
console.log('events');
for (const event of history?.events ?? []) {
switch (event.eventType) {
case temporal.api.enums.v1.EventType.EVENT_TYPE_WORKFLOW_EXECUTION_STARTED:
Expand Down
72 changes: 68 additions & 4 deletions packages/test/src/test-integration-workflows.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import {
hasActivityHeartbeat,
helpers,
makeTestFunction,
setActivityPauseState,
setActivityState,
} from './helpers-integration';
import { overrideSdkInternalFlag } from './mock-internal-flags';
import { heartbeatCancellationDetailsActivity } from './activities/heartbeat-cancellation-details';
Expand Down Expand Up @@ -1459,7 +1459,7 @@ test('Activity pause returns expected cancellation details', async (t) => {
}, 10000);

// Now pause the activity
await setActivityPauseState(handle, testActivityId, true);
await setActivityState(handle, testActivityId, 'pause');
// Get the result - should contain pause cancellation details
const result = await handle.result();

Expand Down Expand Up @@ -1494,15 +1494,79 @@ test('Activity can be cancelled via pause and retry after unpause', async (t) =>
return !!(activityInfo && (await hasActivityHeartbeat(handle, testActivityId, 'heartbeated')));
}, 10000);

await setActivityPauseState(handle, testActivityId, true);
await setActivityState(handle, testActivityId, 'pause');
await waitUntil(async () => hasActivityHeartbeat(handle, testActivityId, 'finally-complete'), 10000);
await setActivityPauseState(handle, testActivityId, false);
await setActivityState(handle, testActivityId, 'unpause');

const result = await handle.result();
t.true(result == null);
});
});

test('Activity reset returns expected cancellation details', async (t) => {
const { createWorker, startWorkflow } = helpers(t);
const worker = await createWorker({
activities: {
heartbeatCancellationDetailsActivity,
},
});

await worker.runUntil(async () => {
const testActivityId = randomUUID();
const handle = await startWorkflow(heartbeatPauseWorkflow, { args: [testActivityId, true, 1] });

// Wait for it to exist and heartbeat
await waitUntil(async () => {
const { raw } = await handle.describe();
const activityInfo = raw.pendingActivities?.find((act) => act.activityId === testActivityId);
return !!(activityInfo && (await hasActivityHeartbeat(handle, testActivityId, 'heartbeated')));
}, 10000);

await setActivityState(handle, testActivityId, 'reset');
const result = await handle.result();
t.deepEqual(result, {
cancelRequested: false,
notFound: false,
paused: false,
Copy link
Member

Choose a reason for hiding this comment

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

Do we have any tests where multiple things are true at once? EX: Paused and reset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

timedOut: false,
workerShutdown: false,
reset: true,
});
});
});

test('Activity set as both paused and reset returns expected cancellation details', async (t) => {
const { createWorker, startWorkflow } = helpers(t);
const worker = await createWorker({
activities: {
heartbeatCancellationDetailsActivity,
},
});

await worker.runUntil(async () => {
const testActivityId = randomUUID();
const handle = await startWorkflow(heartbeatPauseWorkflow, { args: [testActivityId, true, 1] });

// Wait for it to exist and heartbeat
await waitUntil(async () => {
const { raw } = await handle.describe();
const activityInfo = raw.pendingActivities?.find((act) => act.activityId === testActivityId);
return !!(activityInfo && (await hasActivityHeartbeat(handle, testActivityId, 'heartbeated')));
}, 10000);

await setActivityState(handle, testActivityId, 'pause & reset');
const result = await handle.result();
t.deepEqual(result, {
cancelRequested: false,
notFound: false,
paused: true,
timedOut: false,
workerShutdown: false,
reset: true,
});
});
});

const reservedNames = [TEMPORAL_RESERVED_PREFIX, STACK_TRACE_QUERY_NAME, ENHANCED_STACK_TRACE_QUERY_NAME];

test('Cannot register activities using reserved prefixes', async (t) => {
Expand Down
20 changes: 17 additions & 3 deletions packages/worker/src/activity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,14 @@ export class Activity {
(error instanceof CancelledFailure || isAbortError(error)) &&
this.context.cancellationSignal.aborted
) {
if (this.context.cancellationDetails?.paused) {
if (this.context.cancellationDetails?.cancelRequested) {
this.workerLogger.debug('Activity completed as cancelled', { durationMs });
} else if (this.context.cancellationDetails?.reset) {
this.workerLogger.debug('Activity reset', { durationMs });
} else if (this.context.cancellationDetails?.paused) {
this.workerLogger.debug('Activity paused', { durationMs });
} else {
// Fallback log - completed as cancelled.
this.workerLogger.debug('Activity completed as cancelled', { durationMs });
}
} else if (error instanceof CompleteAsyncError) {
Expand Down Expand Up @@ -204,8 +209,17 @@ export class Activity {
} else if (this.cancelReason) {
// Either a CancelledFailure that we threw or AbortError from AbortController
if (err instanceof CancelledFailure) {
// If cancel due to activity pause, emit an application failure for the pause.
if (this.context.cancellationDetails?.paused) {
// If cancel due to activity pause or reset, emit an application failure.
if (this.context.cancellationDetails?.reset) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we confident on if clause ordering? reset vs pause?

Copy link
Contributor Author

@THardy98 THardy98 Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Between reset and pause yes. Added a conditional for explicit cancellations, ordered before the reset branch (to keep the cancellation -> reset -> pause ordering, though this is just for logging)

return {
failed: {
failure: await encodeErrorToFailure(
this.dataConverter,
new ApplicationFailure('Activity reset', 'ActivityReset')
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Retryable or not? Prefer making this explicit, for the benefit of readers.
  • Is there any official semantic on this error? Does the server expect specific type?
  • Should we avoid attaching stack trace on that specific error?

I'm a bit concerned here that ApplicationFailure are meant for user usage, so exposing activity reset as that error type might be "misinterpreted" as something they need to investigate, so they would look at the stack trace and try to figure it out. "Activity Reset" is a pretty generic name, an error message that users could have written themselves...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I've make non-retryable explicitly false.
  2. The server does not expect a specific error type (this error returned is agnostic to server)
  3. I don't really see a reason why

I'm less concerned about using ApplicationFailure for this, partly because it's a user-triggered error, and because it's what we chose to do for Python and TS for activity pause (though other SDKs diverge - Java introduced ActivityPausedException as a subtype of ActivityCompletionException, Go cancels the context with a ErrActivityPaused cause).

The alternative is we don't wrap and we emit a CancelledFailure with message RESET, see:
https://github.com/temporalio/sdk-typescript/blob/9089bc682b4334287e52cd0bdb4547836f63e335/packages/worker/src/activity.ts#L77-84
and
https://github.com/temporalio/sdk-typescript/blob/9089bc682b4334287e52cd0bdb4547836f63e335/packages/worker/src/worker.ts#L1072-1092
(the else conditional)

Whatever choice we make, we should also apply to the pause case, and potentially to Python as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Sushisource wdyt

Copy link
Member

Choose a reason for hiding this comment

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

IMO what Java (wrap with more specific types) is probably the most sensible. But I agree we'd need to apply it to Python and pause as well.

You should get @dandavison 's take and coordinate Python change with him

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tconley1428 is working on activity reset right now

Copy link
Member

@Sushisource Sushisource Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Then him, hehe 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some discussion - we'll leave this as is.

),
},
};
} else if (this.context.cancellationDetails?.paused) {
return {
failed: {
failure: await encodeErrorToFailure(
Expand Down
2 changes: 1 addition & 1 deletion packages/worker/src/worker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ export class Worker {
details,
onError() {
// activity must be defined
// empty cancellation details, not corresponding detail for heartbeat detail conversion failure
// empty cancellation details, no corresponding detail for heartbeat detail conversion failure
activity?.cancel(
'HEARTBEAT_DETAILS_CONVERSION_FAILED',
ActivityCancellationDetails.fromProto(undefined)
Expand Down
Loading