Repository navigation
Add experimental typed Nexus workflow clients - #1660
Quinn-With-Two-Ns wants to merge 4 commits into
Conversation
Sushisource
left a comment
Sushisource
left a comment
There was a problem hiding this comment.
Overall looking good. Some de-llm-ification to do.
| pub schedule_to_close_timeout: Option<Duration>, | ||
| /// Maximum time from scheduling until the handler starts or synchronously completes. | ||
| /// `None` or zero disables this timeout. | ||
| pub schedule_to_start_timeout: Option<Duration>, | ||
| /// Maximum time for an asynchronous operation to complete after starting. | ||
| /// Ignored for synchronous operations; `None` or zero disables this timeout. | ||
| pub start_to_close_timeout: Option<Duration>, |
There was a problem hiding this comment.
Are there no required timeouts for these?
There was a problem hiding this comment.
No, none are required
| Service definitions use the draft [Nexus Rust SDK](https://github.com/nexus-rpc/sdk-rust/pull/2). | ||
| Until that SDK is published, add the same pinned dependency used by this SDK: |
There was a problem hiding this comment.
Is this supposed to be here still? Should get removed fast-follow at least?
There was a problem hiding this comment.
Yeah I haven't published the Nexus crate yet, will need to do that before I can merge this
chris-olszewski
left a comment
chris-olszewski
left a comment
There was a problem hiding this comment.
Just serializing this since I have interview so I probably won't get to more before EoD, but the failure converter story seems off to me. I'll poke at it a bit locally so I have actual suggestions (and since I made the initial mess).
| pub enum NexusOperationError { | ||
| /// The Nexus operation failed, retaining its complete normalized failure. | ||
| #[error("Nexus operation failed: {}", .0.failure().message)] | ||
| Failed(#[source] Box<IncomingError>), |
There was a problem hiding this comment.
We should probably have a narrower variant here similar to other error types. Could we have something similar to ActivityFailureError but nexus flavored?
| .to_error(&context_data, failure, NoopDecodeHint) | ||
| .map(NexusOperationError::from) |
|
Took longer than expected, but here's my suggested changes wrt the error types: #1689 |
Co-authored-by: Spencer Judge <spencer@temporal.io>
Co-authored-by: Spencer Judge <spencer@temporal.io>
Summary
ctx.nexus_client(...), typedexecute/start, operation tokens, cancellation, and repeatable typed results.Note
This PR depends on https://github.com/nexus-rpc/sdk-rust/pull/2 I am holding off merging and releasing that until we are good with the Rust implementation as well.
Note
Medium Risk
Touches workflow replay determinism (stricter Nexus service/operation checks) and introduces experimental cross-service call paths with a git-pinned draft Nexus SDK; failure encoding/decoding changes affect how workflow errors surface.
Overview
Adds experimental, compile-time typed Nexus calls from workflows behind the
experimentalfeature, built on a pinned draftnexus-rpcSDK dependency.Workflow code can use
ctx.nexus_client(service, options)to get aNexusServiceClientwith typedexecute(start + wait for result) andstart(returnsNexusOperationHandlewithoperation_token,cancel, and repeatable typedresult). Options cover timeouts,cancellation_type(non-optional enum, default wait-for-completion), optional cancellation tokens, and event groups; wire service/operation names come from the Nexus service definition.NexusOperationErrorandNexusOperationDecodeHintextend failure conversion so start/complete failures keep Nexus metadata and distinguish failed vs cancelled vs timed-out resolutions when Core omits failure info.Replay breaking change: histories now surface nondeterminism if a replayed Nexus schedule’s service or operation does not match the command that recorded it. Docs and integration tests cover sync/async Nexus flows, cancellation, and replay.
Reviewed by Cursor Bugbot for commit f46ba05. Bugbot is set up for automated code reviews on this repo. Configure here.