Skip to content

Commit 2bc2e32

Browse files
committed
Address some claude & codex suggestions. Swap to taking options rather than requiring subclassing
1 parent 9a9fce3 commit 2bc2e32

4 files changed

Lines changed: 120 additions & 80 deletions

File tree

packages/nexus/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export {
1515

1616
export {
1717
startWorkflow,
18+
type TemporalOperationHandlerOptions,
1819
TemporalOperationHandler,
1920
TemporalOperationResult,
2021
type TemporalNexusClient,

packages/nexus/src/token.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
/**
2-
* OperationTokenType is used to identify the type of Operation token.
3-
* Currently, we only have one type of Operation token: WorkflowRun.
2+
* Serializable token identifying a Nexus operation target.
43
*
54
* @internal
65
* @hidden
@@ -28,11 +27,24 @@ export interface OperationToken {
2827
wid?: string;
2928
}
3029

30+
/**
31+
* An OperationToken that identifies a WorkflowRun operation.
32+
*
33+
* @internal
34+
* @hidden
35+
*/
3136
export interface WorkflowRunOperationToken extends OperationToken {
3237
t: typeof OperationTokenType.WORKFLOW_RUN;
3338
wid: string;
3439
}
3540

41+
/**
42+
* OperationTokenType is used to identify the type of Operation token.
43+
* Currently, we only have one type of Operation token: WorkflowRun.
44+
*
45+
* @internal
46+
* @hidden
47+
*/
3648
export type OperationTokenType = (typeof OperationTokenType)[keyof typeof OperationTokenType];
3749

3850
/**

packages/nexus/src/workflow-helpers.ts

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -279,16 +279,31 @@ export type TemporalOperationStartHandler<I, O> = (
279279
input: I
280280
) => Promise<TemporalOperationResult<O>>;
281281

282+
/**
283+
* Options for customizing a {@link TemporalOperationHandler}.
284+
*
285+
* @experimental Nexus support in Temporal SDK is experimental.
286+
*/
287+
export interface TemporalOperationHandlerOptions {
288+
cancelWorkflowRun?: (ctx: nexus.CancelOperationContext, client: Client, workflowId: string) => Promise<void>;
289+
}
290+
282291
/**
283292
* A Nexus Operation implementation for operations that interact with Temporal.
284293
*
285294
* @experimental Nexus support in Temporal SDK is experimental.
286295
*/
287296
export class TemporalOperationHandler<I, O> implements nexus.OperationHandler<I, O> {
288-
constructor(readonly handler: TemporalOperationStartHandler<I, O>) {}
297+
private readonly startHandler: TemporalOperationStartHandler<I, O>;
298+
private readonly cancelWorkflowRunHandler: NonNullable<TemporalOperationHandlerOptions['cancelWorkflowRun']>;
299+
300+
constructor(options: { start: TemporalOperationStartHandler<I, O> } & TemporalOperationHandlerOptions) {
301+
this.startHandler = options.start;
302+
this.cancelWorkflowRunHandler = options.cancelWorkflowRun ?? defaultCancelWorkflowRun;
303+
}
289304

290305
async start(ctx: nexus.StartOperationContext, input: I): Promise<nexus.HandlerStartOperationResult<O>> {
291-
const result = await this.handler(ctx, new TemporalNexusClientImpl(ctx), input);
306+
const result = await this.startHandler(ctx, new TemporalNexusClientImpl(ctx), input);
292307
return result[toHandlerResult]();
293308
}
294309

@@ -309,8 +324,14 @@ export class TemporalOperationHandler<I, O> implements nexus.OperationHandler<I,
309324
}
310325
switch (opToken.t) {
311326
case OperationTokenType.WORKFLOW_RUN:
312-
assertWorkflowRunOperationToken(opToken);
313-
await this.cancelWorkflowRun(ctx, opToken.wid);
327+
try {
328+
assertWorkflowRunOperationToken(opToken);
329+
} catch (err) {
330+
throw new nexus.HandlerError(nexus.HandlerErrorType.BAD_REQUEST, 'invalid workflow run operation token', {
331+
cause: err,
332+
});
333+
}
334+
await this.cancelWorkflowRunHandler(ctx, getClient(), opToken.wid);
314335
return;
315336
default:
316337
throw new nexus.HandlerError(
@@ -319,15 +340,8 @@ export class TemporalOperationHandler<I, O> implements nexus.OperationHandler<I,
319340
);
320341
}
321342
}
343+
}
322344

323-
/**
324-
* Cancel the workflow backing this Nexus Operation.
325-
*
326-
* Override this method to customize workflow cancellation behavior.
327-
*
328-
* @experimental Nexus support in Temporal SDK is experimental.
329-
*/
330-
protected async cancelWorkflowRun(_ctx: nexus.CancelOperationContext, workflowId: string): Promise<void> {
331-
await getClient().workflow.getHandle(workflowId).cancel();
332-
}
345+
async function defaultCancelWorkflowRun(_ctx: nexus.CancelOperationContext, client: Client, workflowId: string) {
346+
await client.workflow.getHandle(workflowId).cancel();
333347
}

packages/test/src/test-nexus-temporal-operation.ts

Lines changed: 77 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@ type TemporalOpServiceHandlers = nexus.ServiceHandlerFor<typeof temporalOpServic
4949
type TemporalCancelOpServiceHandlers = nexus.ServiceHandlerFor<typeof temporalCancelOpService.operations>;
5050

5151
function unusedTemporalOperationHandler<I, O>(): nexus.OperationHandler<I, O> {
52-
return new temporalnexus.TemporalOperationHandler<I, O>(async () => {
53-
throw new nexus.HandlerError('NOT_IMPLEMENTED', 'not used by this test');
52+
return new temporalnexus.TemporalOperationHandler<I, O>({
53+
async start() {
54+
throw new nexus.HandlerError('NOT_IMPLEMENTED', 'not used by this test');
55+
},
5456
});
5557
}
5658

@@ -114,67 +116,68 @@ export async function blockingTargetWorkflow(): Promise<void> {
114116
// Tests
115117

116118
test('TemporalOperationHandler infers correct output type from typed workflow function', async (t) => {
117-
const _stringOp: nexus.OperationHandler<string, string> = new temporalnexus.TemporalOperationHandler(
118-
async (_ctx, client, input: string) => {
119+
const _stringOp: nexus.OperationHandler<string, string> = new temporalnexus.TemporalOperationHandler({
120+
async start(_ctx, client, input: string) {
119121
return await client.startWorkflow(echoWorkflow, {
120122
args: [input],
121123
workflowId: 'test',
122124
});
123-
}
124-
);
125+
},
126+
});
125127

126128
// @ts-expect-error - Output type should be string, not number
127-
const _mismatchedOp: nexus.OperationHandler<string, number> = new temporalnexus.TemporalOperationHandler(
128-
async (_ctx, client, input: string) => {
129+
const _mismatchedOp: nexus.OperationHandler<string, number> = new temporalnexus.TemporalOperationHandler({
130+
async start(_ctx, client, input: string) {
129131
return await client.startWorkflow(echoWorkflow, {
130132
args: [input],
131133
workflowId: 'test',
132134
});
133-
}
134-
);
135+
},
136+
});
135137

136-
const _syncOp: nexus.OperationHandler<string, string> = new temporalnexus.TemporalOperationHandler(
137-
async (_ctx, _client, input: string) => {
138+
const _syncOp: nexus.OperationHandler<string, string> = new temporalnexus.TemporalOperationHandler({
139+
async start(_ctx, _client, input: string) {
138140
return temporalnexus.TemporalOperationResult.sync(input);
139-
}
140-
);
141+
},
142+
});
141143

142144
const _explicitStringOp: nexus.OperationHandler<string, string> = new temporalnexus.TemporalOperationHandler<
143145
string,
144146
string
145-
>(async (_ctx, client, input) => {
146-
return await client.startWorkflow(echoWorkflow, {
147-
args: [input],
148-
workflowId: 'test',
149-
});
147+
>({
148+
async start(_ctx, client, input) {
149+
return await client.startWorkflow(echoWorkflow, {
150+
args: [input],
151+
workflowId: 'test',
152+
});
153+
},
150154
});
151155

152156
// This test only checks for compile-time errors.
153157
t.pass();
154158
});
155159

156-
test('TemporalOperationHandler cancel delegates to overridable cancelWorkflow', async (t) => {
160+
test('TemporalOperationHandler cancel delegates to provided cancelWorkflowRun handler', async (t) => {
157161
const { createWorker, registerNexusEndpoint } = helpers(t);
158162
const { endpointName } = await registerNexusEndpoint();
159163
const workflowId = randomUUID();
160164

161165
let customCancelCalled = false;
162166

163-
class CustomCancelHandler extends temporalnexus.TemporalOperationHandler<string, void> {
164-
async cancelWorkflowRun(_ctx: nexus.CancelOperationContext, workflowId: string): Promise<void> {
165-
const handle = temporalnexus.getClient().workflow.getHandle(workflowId);
166-
await handle.cancel();
167-
customCancelCalled = true;
168-
}
169-
}
170-
171167
const worker = await createWorker({
172168
nexusServices: [
173169
makeTemporalCancelOpServiceHandler({
174-
blockingOp: new CustomCancelHandler(async (_ctx, client, workflowId) => {
175-
return await client.startWorkflow(blockingTargetWorkflow, {
176-
workflowId,
177-
});
170+
blockingOp: new temporalnexus.TemporalOperationHandler({
171+
async start(_ctx, client, workflowId) {
172+
return await client.startWorkflow(blockingTargetWorkflow, {
173+
workflowId,
174+
});
175+
},
176+
async cancelWorkflowRun(_ctx, client, workflowId) {
177+
const handle = client.workflow.getHandle(workflowId);
178+
await handle.cancel();
179+
customCancelCalled = true;
180+
},
178181
}),
179182
}),
180183
],
@@ -208,14 +211,16 @@ test('TemporalOperationHandler cancel delegates to overridable cancelWorkflow',
208211
});
209212
});
210213

211-
test('TemporalOperationHandler cancel rejects invalid operation token type', async (t) => {
212-
class CustomCancelHandler extends temporalnexus.TemporalOperationHandler<void, void> {
213-
async cancelWorkflowRun(_ctx: nexus.CancelOperationContext, _workflowId: string): Promise<void> {
214-
throw new Error('cancelWorkflowRun should not be called');
215-
}
216-
}
214+
test('TemporalOperationHandler.cancel rejects invalid operation token type before invoking cancellation hooks', async (t) => {
215+
const handler = new temporalnexus.TemporalOperationHandler({
216+
async start(_ctx, _client, _input) {
217+
return temporalnexus.TemporalOperationResult.sync(undefined);
218+
},
217219

218-
const handler = new CustomCancelHandler(async () => temporalnexus.TemporalOperationResult.sync(undefined));
220+
async cancelWorkflowRun(_ctx, _client, _workflowId) {
221+
throw new Error('cancelWorkflowRun should not be called');
222+
},
223+
});
219224
const token = base64URLEncodeNoPadding(JSON.stringify({ t: 2, ns: 'test-namespace' }));
220225

221226
const err = await asyncLocalStorage.run(
@@ -252,14 +257,18 @@ test('TemporalOperationHandler async and sync happy paths - caller workflow', as
252257
const worker = await createWorker({
253258
nexusServices: [
254259
makeTemporalOpServiceHandler({
255-
asyncOp: new temporalnexus.TemporalOperationHandler<string, string>(async (_ctx, client, input) => {
256-
return await client.startWorkflow(echoWorkflow, {
257-
workflowId: randomUUID(),
258-
args: [input],
259-
});
260+
asyncOp: new temporalnexus.TemporalOperationHandler<string, string>({
261+
async start(_ctx, client, input) {
262+
return await client.startWorkflow(echoWorkflow, {
263+
workflowId: randomUUID(),
264+
args: [input],
265+
});
266+
},
260267
}),
261-
syncOp: new temporalnexus.TemporalOperationHandler<string, string>(async (_ctx, _client, input) => {
262-
return temporalnexus.TemporalOperationResult.sync(input);
268+
syncOp: new temporalnexus.TemporalOperationHandler<string, string>({
269+
async start(_ctx, _client, input) {
270+
return temporalnexus.TemporalOperationResult.sync(input);
271+
},
263272
}),
264273
}),
265274
],
@@ -285,16 +294,18 @@ test('TemporalOperationHandler rejects multiple async starts', async (t) => {
285294
const worker = await createWorker({
286295
nexusServices: [
287296
makeTemporalOpServiceHandler({
288-
doubleStartOp: new temporalnexus.TemporalOperationHandler<string, void>(async (_ctx, client, input) => {
289-
await client.startWorkflow(echoWorkflow, {
290-
workflowId: randomUUID(),
291-
args: [input],
292-
});
293-
await client.startWorkflow(echoWorkflow, {
294-
workflowId: randomUUID(),
295-
args: [input],
296-
});
297-
throw new nexus.HandlerError('INTERNAL', 'expected previous error to be thrown');
297+
doubleStartOp: new temporalnexus.TemporalOperationHandler<string, void>({
298+
async start(_ctx, client, input) {
299+
await client.startWorkflow(echoWorkflow, {
300+
workflowId: randomUUID(),
301+
args: [input],
302+
});
303+
await client.startWorkflow(echoWorkflow, {
304+
workflowId: randomUUID(),
305+
args: [input],
306+
});
307+
throw new nexus.HandlerError('INTERNAL', 'expected previous error to be thrown');
308+
},
298309
}),
299310
}),
300311
],
@@ -326,8 +337,8 @@ test('TemporalOperationHandler allows retry after failed async start', async (t)
326337
const worker = await createWorker({
327338
nexusServices: [
328339
makeTemporalOpServiceHandler({
329-
retryAfterFailedStartOp: new temporalnexus.TemporalOperationHandler<string, string>(
330-
async (_ctx, client, workflowId) => {
340+
retryAfterFailedStartOp: new temporalnexus.TemporalOperationHandler<string, string>({
341+
async start(_ctx, client, workflowId) {
331342
try {
332343
await client.startWorkflow(blockingTargetWorkflow, {
333344
workflowId,
@@ -345,8 +356,8 @@ test('TemporalOperationHandler allows retry after failed async start', async (t)
345356
throw new nexus.HandlerError('INTERNAL', 'Expected first workflow start to fail', {
346357
retryableOverride: false,
347358
});
348-
}
349-
),
359+
},
360+
}),
350361
}),
351362
],
352363
});
@@ -374,10 +385,12 @@ test('TemporalOperationHandler default cancelWorkflowRun cancels backing workflo
374385
const worker = await createWorker({
375386
nexusServices: [
376387
makeTemporalCancelOpServiceHandler({
377-
blockingOp: new temporalnexus.TemporalOperationHandler<string, void>(async (_ctx, client, workflowId) => {
378-
return await client.startWorkflow(blockingTargetWorkflow, {
379-
workflowId,
380-
});
388+
blockingOp: new temporalnexus.TemporalOperationHandler<string, void>({
389+
async start(_ctx, client, workflowId) {
390+
return await client.startWorkflow(blockingTargetWorkflow, {
391+
workflowId,
392+
});
393+
},
381394
}),
382395
}),
383396
],

0 commit comments

Comments
 (0)