Skip to content

Commit f5290b0

Browse files
committed
make 'cancel' in mock activity env more well-defined, make cancellation details field in activity context private, make activity cancellation tests less flaky
1 parent 076cb06 commit f5290b0

File tree

6 files changed

+54
-52
lines changed

6 files changed

+54
-52
lines changed

packages/activity/src/index.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ export class Context {
301301
/**
302302
* Holder object for activity cancellation details
303303
*/
304-
public readonly cancellationDetails: ActivityCancellationDetailsHolder;
304+
private readonly _cancellationDetails: ActivityCancellationDetailsHolder;
305305

306306
/**
307307
* **Not** meant to instantiated by Activity code, used by the worker.
@@ -323,7 +323,7 @@ export class Context {
323323
this.heartbeatFn = heartbeat;
324324
this.log = log;
325325
this.metricMeter = metricMeter;
326-
this.cancellationDetails = details;
326+
this._cancellationDetails = details;
327327
}
328328

329329
/**
@@ -363,6 +363,10 @@ export class Context {
363363
});
364364
return Promise.race([this.cancelled.finally(() => clearTimeout(handle)), timer]);
365365
};
366+
367+
public cancellationDetails(): ActivityCancellationDetails | undefined {
368+
return this._cancellationDetails.details;
369+
}
366370
}
367371

368372
/**
@@ -447,7 +451,7 @@ export function cancelled(): Promise<never> {
447451
* Returns the cancellation details for this activity, if any.
448452
*/
449453
export function cancellationDetails(): ActivityCancellationDetails | undefined {
450-
return Context.current().cancellationDetails.details;
454+
return Context.current().cancellationDetails();
451455
}
452456

453457
/**

packages/common/src/activity-cancellation-details.ts

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,15 @@ export interface ActivityCancellationDetailsHolder {
44
details?: ActivityCancellationDetails;
55
}
66

7+
export interface ActivityCancellationDetailsOptions {
8+
notFound?: boolean;
9+
cancelRequested?: boolean;
10+
paused?: boolean;
11+
timedOut?: boolean;
12+
workerShutdown?: boolean;
13+
reset?: boolean;
14+
}
15+
716
/**
817
* Provides the reasons for the activity's cancellation. Cancellation details are set once and do not change once set.
918
*/
@@ -15,20 +24,13 @@ export class ActivityCancellationDetails {
1524
readonly workerShutdown: boolean;
1625
readonly reset: boolean;
1726

18-
private constructor(
19-
notFound: boolean = false,
20-
cancelRequested: boolean = false,
21-
paused: boolean = false,
22-
timedOut: boolean = false,
23-
workerShutdown: boolean = false,
24-
reset: boolean = false
25-
) {
26-
this.notFound = notFound;
27-
this.cancelRequested = cancelRequested;
28-
this.paused = paused;
29-
this.timedOut = timedOut;
30-
this.workerShutdown = workerShutdown;
31-
this.reset = reset;
27+
public constructor(options: ActivityCancellationDetailsOptions = {}) {
28+
this.notFound = options.notFound ?? false;
29+
this.cancelRequested = options.cancelRequested ?? false;
30+
this.paused = options.paused ?? false;
31+
this.timedOut = options.timedOut ?? false;
32+
this.workerShutdown = options.workerShutdown ?? false;
33+
this.reset = options.reset ?? false;
3234
}
3335

3436
static fromProto(
@@ -37,13 +39,13 @@ export class ActivityCancellationDetails {
3739
if (proto == null) {
3840
return new ActivityCancellationDetails();
3941
}
40-
return new ActivityCancellationDetails(
41-
proto.isNotFound ?? false,
42-
proto.isCancelled ?? false,
43-
proto.isPaused ?? false,
44-
proto.isTimedOut ?? false,
45-
proto.isWorkerShutdown ?? false,
46-
proto.isReset ?? false
47-
);
42+
return new ActivityCancellationDetails({
43+
notFound: proto.isNotFound ?? false,
44+
cancelRequested: proto.isCancelled ?? false,
45+
paused: proto.isPaused ?? false,
46+
timedOut: proto.isTimedOut ?? false,
47+
workerShutdown: proto.isWorkerShutdown ?? false,
48+
reset: proto.isReset ?? false
49+
});
4850
}
4951
}

packages/test/src/helpers-integration.ts

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -285,24 +285,6 @@ export function configurableHelpers<T>(
285285
};
286286
}
287287

288-
export async function assertPendingActivityExistsEventually(
289-
handle: WorkflowHandle<workflow.Workflow>,
290-
activityId: string,
291-
timeoutMs: number
292-
): Promise<temporal.api.workflow.v1.IPendingActivityInfo> {
293-
let activityInfo: temporal.api.workflow.v1.IPendingActivityInfo | undefined;
294-
try {
295-
await waitUntil(async () => {
296-
const desc = await handle.describe();
297-
activityInfo = desc.raw.pendingActivities?.find((pa) => pa.activityId === activityId);
298-
return activityInfo !== undefined;
299-
}, timeoutMs);
300-
} catch {
301-
throw new Error(`Unable to find pending activity for activity ${activityId}`);
302-
}
303-
return activityInfo as temporal.api.workflow.v1.IPendingActivityInfo;
304-
}
305-
306288
export async function setActivityPauseState(handle: WorkflowHandle, activityId: string, pause: boolean): Promise<void> {
307289
const desc = await handle.describe();
308290
const req = {
@@ -319,11 +301,13 @@ export async function setActivityPauseState(handle: WorkflowHandle, activityId:
319301
await handle.client.workflowService.unpauseActivity(req);
320302
}
321303
await waitUntil(async () => {
322-
const info = await assertPendingActivityExistsEventually(handle, activityId, 10000);
304+
const { raw } = await handle.describe();
305+
const activityInfo = raw.pendingActivities?.find((act) => act.activityId === activityId);
306+
if (!activityInfo) return false;
323307
if (pause) {
324-
return info.paused ?? false;
308+
return activityInfo.paused ?? false;
325309
}
326-
return !info.paused;
310+
return !activityInfo.paused;
327311
}, 10000);
328312
}
329313

packages/test/src/test-mockactivityenv.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ test('MockActivityEnvironment emits heartbeat events and can be cancelled', asyn
2424
const env = new MockActivityEnvironment();
2525
env.on('heartbeat', (d: unknown) => {
2626
if (d === 6) {
27-
env.cancel('test');
27+
env.cancel('CANCELLED');
2828
}
2929
});
3030
await t.throwsAsync(

packages/testing/src/mocking-activity-environment.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ import {
99
defaultPayloadConverter,
1010
MetricMeter,
1111
noopMetricMeter,
12+
ActivityCancellationDetails,
1213
} from '@temporalio/common';
1314
import { LoggerWithComposedMetadata } from '@temporalio/common/lib/logger';
1415
import { ActivityInterceptorsFactory, DefaultLogger } from '@temporalio/worker';
15-
import { Activity } from '@temporalio/worker/lib/activity';
16+
import { Activity, CancelReason } from '@temporalio/worker/lib/activity';
1617

1718
export interface MockActivityEnvironmentOptions {
1819
interceptors?: ActivityInterceptorsFactory[];
@@ -30,7 +31,10 @@ export interface MockActivityEnvironmentOptions {
3031
* will immediately be in a cancelled state.
3132
*/
3233
export class MockActivityEnvironment extends events.EventEmitter {
33-
public cancel: (reason?: any, details?: any) => void = () => undefined;
34+
public cancel: (
35+
reason?: CancelReason,
36+
details?: ActivityCancellationDetails
37+
) => void = () => undefined;
3438
public readonly context: activity.Context;
3539
private readonly activity: Activity;
3640

@@ -52,7 +56,15 @@ export class MockActivityEnvironment extends events.EventEmitter {
5256
opts?.interceptors ?? []
5357
);
5458
this.context = this.activity.context;
55-
this.cancel = this.activity.cancel;
59+
this.cancel = (
60+
reason?: CancelReason,
61+
details?: ActivityCancellationDetails
62+
) => {
63+
// Default to CANCELLED if nothing provided.
64+
const r = reason ?? "CANCELLED";
65+
const d = details ?? new ActivityCancellationDetails({ cancelRequested: true });
66+
this.activity.cancel(r, d);
67+
}
5668
}
5769

5870
/**

packages/worker/src/activity.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export class Activity {
144144
(error instanceof CancelledFailure || isAbortError(error)) &&
145145
this.context.cancellationSignal.aborted
146146
) {
147-
if (this.context.cancellationDetails.details?.paused) {
147+
if (this.context.cancellationDetails()?.paused) {
148148
this.workerLogger.debug('Activity paused', { durationMs });
149149
} else {
150150
this.workerLogger.debug('Activity completed as cancelled', { durationMs });
@@ -187,7 +187,7 @@ export class Activity {
187187
// Either a CancelledFailure that we threw or AbortError from AbortController
188188
if (err instanceof CancelledFailure) {
189189
// If cancel due to activity pause, emit an application failure for the pause.
190-
if (this.context.cancellationDetails.details?.paused) {
190+
if (this.context.cancellationDetails()?.paused) {
191191
return {
192192
failed: {
193193
failure: await encodeErrorToFailure(

0 commit comments

Comments
 (0)