Conversation
Add bindings for Workflows including step primitives (do, sleep, sleepUntil), event/instance management, and a proc macro for defining workflow entrypoints. Includes example worker, sys-level bindings, and integration tests.
|
Fixes #663 👏 |
|
Amazing, I will take a look soon. Hopefully we can get this in for the next release! |
Workflow async methods were failing to compile with the `http` feature because JsFuture is not Send. Wrap all JsFuture calls with SendFuture and extract promises to separate variables to avoid holding non-Send types across await points.
guybedford
left a comment
guybedford
left a comment
There was a problem hiding this comment.
Left some initial comments. Let me know your thoughts.
- Bind NonRetryableError to the actual JS class from cloudflare:workflows so the runtime correctly identifies non-retryable errors - Add --external cloudflare:workflows to esbuild in worker-build - Change wasm_bindgen extern bindings to async with return typing - Change step callbacks from FnOnce to Fn so retries can re-invoke them - Preserve Error::Internal JsValues through error conversion chain - Update workflow example to demonstrate both retryable and non-retryable error patterns with request body parsing
# Conflicts: # Cargo.lock # Cargo.toml # worker-build/src/main.rs
…ers, improve efficiency - Exclude __wf_* marker functions from generate_handlers to prevent spurious Entrypoint.prototype entries - Replace raw Reflect::set calls in send_event with a single serde serialization - Cache result_array.length() in create_batch to avoid redundant JS boundary crossings - Reorder expand_macro to parse ItemStruct first, avoiding unnecessary clone+parse on success path - Extract shared create_workflow_with_value helper from duplicated test handlers - Poll immediately before sleeping in test loops to reduce CI latency
Exercises the WorkflowStepEvent::from_js deserialization and WorkflowInstance::send_event serialization round-trip that wasn't covered by existing tests.
Merging this PR will not alter performance
|
- Use next_back() instead of last() on DoubleEndedIterator (clippy lint) - Use Closure::new instead of Closure::wrap to preserve UnwindSafe through the concrete closure type rather than erasing it via trait object cast
guybedford
left a comment
guybedford
left a comment
There was a problem hiding this comment.
This is very close to a perfect implementation, some small suggestions. Probably worth carefully working through this before landing so I won't rush it out in todays release actually.
|
|
||
| async fn run( | ||
| &self, | ||
| event: WorkflowEvent<serde_json::Value>, |
There was a problem hiding this comment.
Can we come up with a better typing solution here? It is quite restrictive to rely so heavily on serde. Can we instead make this generic with the bound being something something as simple as IntoWasmAbi?
There was a problem hiding this comment.
Does JsValue work here?
There was a problem hiding this comment.
What I mean specifically is making this a generic argument T so that run<T> can take any type with the IntoWasmAbi trait from wasm bindgen?
There was a problem hiding this comment.
Actually with the Wasm Bindgen generics you could just make T: JsGeneric on the new JsGeneric trait.
There was a problem hiding this comment.
JsGeneric is a bit awkward to work with here.
/// The event passed to a workflow's run method.
#[derive(Debug, Clone)]
pub struct WorkflowEvent<T = JsValue> {
pub payload: T,
pub timestamp: crate::Date,
pub instance_id: String,
}
impl<T: JsGeneric> WorkflowEvent<T> {
pub fn from_js(value: JsValue) -> Result<Self> {
let payload = get_property(&value, "payload")?;
Ok(Self {
// SAFETY: JsGeneric guarantees T has ErasableGeneric<Repr = JsValue>,
// so T and JsValue have an identical memory layout.
payload: unsafe {
core::mem::transmute_copy(&core::mem::ManuallyDrop::new(payload))
},
timestamp: get_timestamp_property(&value, "timestamp")?,
instance_id: get_string_property(&value, "instanceId")?,
})
}
}JsCast is slightly less awkward:
/// The event passed to a workflow's run method.
#[derive(Debug, Clone)]
pub struct WorkflowEvent<T = JsValue> {
pub payload: T,
pub timestamp: crate::Date,
pub instance_id: String,
}
impl<T: JsCast> WorkflowEvent<T> {
pub fn from_js(value: JsValue) -> Result<Self> {
Ok(Self {
payload: get_property(&value, "payload")?
.dyn_into()
.map_err(|_| crate::Error::JsError("payload is not the expected type".into()))?,
timestamp: get_timestamp_property(&value, "timestamp")?,
instance_id: get_string_property(&value, "instanceId")?,
})
}
}thoughts/opinions? or am I missing something/doing something wrong with JsGeneric?
There was a problem hiding this comment.
I likely need to properly follow the API design details here, I will aim to do a thorough review next week after this current release. Sorry for the delay and thanks for the patience as always!
There was a problem hiding this comment.
All good! Thank you!
* remove fetcher * remove result from id() * event_type -> type_ * workflowsleepduration serde_json::Value -> JsValue
noticed while looking at the workerd types
2 participants
Add bindings for Workflows including step primitives (do, sleep, sleepUntil), event/instance management, and a proc macro for defining workflow entrypoints. Includes example worker, sys-level bindings, and integration tests.