Nexus: raw ActivityClient starts inherit request ID and links - #2369
Nexus: raw ActivityClient starts inherit request ID and links#2369tekkaya wants to merge 3 commits into
Conversation
4d0af10 to
5eb639f
Compare
|
Note: Cloud Integ Tests seems like failing since PR #2367 is merged |
| * @internal | ||
| * @hidden | ||
| */ | ||
| export interface NexusActivityStartContext { |
There was a problem hiding this comment.
This shouldn't be specific to Activity start. This is the context around a StartNexusOperation task and should reflect that rather than being activity specific.
Let's also move it to packages/nexus/context.ts
There was a problem hiding this comment.
StartNexusOperationTaskContext or NexusOperationTaskContext or NexusStartOperationTaskContext any other suggestions for the naming? I am leaning towards NexusStartOperationTaskContext to align with NexusStartOperationInput existing NexusCancelOperationInputtypes
There was a problem hiding this comment.
Let's also move it to /packages/nexus/context.ts
nexus package already depends on client, so moving this into packages/nexus/context.ts and having ActivityClient read it back would create a real circular package dependency. Kept it in client, exported only through internal.ts, for that reason.
There was a problem hiding this comment.
Thinking about the name made me realize we actually already have a type with these exact fields that is passed to all handlers: TemporalStartOperationContext. This is the type that should be used I think.
There was a problem hiding this comment.
Let's take a look at making these changes via an internal ActivityClientInterceptor that is appended at the end of the interceptor chain (so user interceptors run first). That way we can encapsulate this Nexus context specific changes away from this general client implementation. The interceptor can add or modify the internal options and then hopefully we won't need any changes here.
There was a problem hiding this comment.
That makes a lot of sense. I'll go with that direction.
b30b066 to
6e78a9a
Compare
TemporalNexusClient.startActivity() guards the one Activity start that can complete a Nexus operation, but a synchronous handler starting additional Activities has to call the raw ActivityClient directly. Those raw starts previously carried none of the Nexus-aware wiring, so the caller had no link to them and a Nexus-task redelivery would double-execute the Activity instead of resolving to its original run. Raw Activity starts now inherit the handler's request ID (redelivery resolves to the original run) and inbound links (preserving the caller-to-Activity relationship), via a new ambient NexusActivityStartContext installed by the worker for the duration of each startOperation invocation. Completion callbacks remain limited to the guarded start, since only that start can complete the operation. Aligns with the approved sdk-go fix (temporalio/sdk-go#2633) and sdk-java's converged design (temporalio/sdk-java#3048).
- Removed short explanatory comments in ActivityClient flagged as
unnecessary ("Raw starts inherit...", "Reuse the handler request
ID...") and the "mirroring the pattern used by..." aside in the
ambient context module.
- Renamed NexusActivityStartContext -> NexusStartOperationTaskContext
(and its accessors) since the context is scoped to the Nexus
startOperation task, not specifically to Activity starts, and isn't
populated for cancelOperation tasks.
Per review feedback, encapsulate the Nexus-specific behavior away from the general ActivityClient implementation: a raw Activity start (one that bypasses TemporalNexusClient.startActivity()'s guarded helper) now gets its Nexus request ID and inbound links from an internal ActivityClientInterceptor, appended after any user-provided interceptors on the Worker's Client. ActivityClient itself is unchanged. The interceptor reads HandlerContext.nexusStartOperationContext, a new field on @temporalio/nexus's existing per-task ambient context, set for the duration of a startOperation task. This reuses the context/AsyncLocalStorage @temporalio/nexus already has, rather than introducing a second one.
6e78a9a to
ba0a222
Compare
2 participants
Summary
TemporalNexusClient.startActivity()guards the one Activity start that can complete a Nexus operation, but a synchronous handler that starts additional Activities must call the rawActivityClient. Those starts previously omitted the inbound Nexus request ID and links, so a Nexus-task redelivery could start a duplicate Activity instead of resolving to its original run.Raw Activity starts now inherit the handler’s request ID and inbound links. Completion callbacks remain limited to the guarded start, since only that start can complete the operation.
Aligns with the sdk-go PR #2633 and sdk-java PR #3048.
Changes
startOperationhandler invocation.ActivityClientstarts inherit the inbound Nexus request ID and links.Testing