Add implementation of async mode - #357
Open
michelemin wants to merge 1 commit into
Open
michelemin wants to merge 1 commit into
michelemin wants to merge 1 commit into
Conversation
michelemin
added
enhancement
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Async completion delivery currently risks losing continuations under load and leaks task handles, which can break correctness and stability in production.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces an async “ticket” system to the Plaid runtime/STL so WASM rules can start long-running API calls without blocking an executor thread, then be re-invoked on completion via a continuation handler routed by a new STL entrypoint macro.
Changes:
- Add runtime-side async ticket registry + sweeper, plus host functions (
async_spawn,ticket_status,ticket_claim,ticket_cancel) and executor wiring to deliver completion messages back to the originating module. - Add STL guest API (
plaid_stl::r#async) andentrypoint_with_async!macro to route completion messages to named continuation handlers. - Add end-to-end test rule + harness and supporting config, plus documentation and version bumps to 48.0.0.
File summaries
| File | Description |
|---|---|
| runtime/README.md | Documents the async ticket system and its properties. |
| runtime/plaid/src/tests.rs | Adds test helper to build a stub PlaidModule. |
| runtime/plaid/src/loader/mod.rs | Exposes LimitableAmount fields and adds compile_for_tests helper. |
| runtime/plaid/src/lib.rs | Exposes async_ops module and test helpers. |
| runtime/plaid/src/functions/mod.rs | Registers async host function module and exposes sweeper start helper. |
| runtime/plaid/src/functions/async_ops.rs | Implements async ticket host functions and completion delivery. |
| runtime/plaid/src/functions/api.rs | Registers async host functions in the API function table. |
| runtime/plaid/src/executor/mod.rs | Threads TicketRegistry through executor env and execution loop. |
| runtime/plaid/src/config.rs | Adds [executor.async_tickets] configuration struct and defaults. |
| runtime/plaid/src/bin/request_handler.rs | Adds /async_echo test route for async integration testing. |
| runtime/plaid/src/bin/plaid.rs | Creates ticket registry, starts sweeper, wires registry into executor, joins sweeper on shutdown. |
| runtime/plaid/src/async_ops/mod.rs | Adds TicketRegistry implementation + sweeper + unit tests. |
| runtime/plaid/src/async_ops/dispatch.rs | Adds async dispatch table mapping host API names to spawned futures + test-mode allow list. |
| runtime/plaid/resources/config/webhooks.toml | Adds webhook entry for test_async. |
| runtime/plaid/resources/config/loading.toml | Adds test_async.wasm test-mode exemption and log type override. |
| runtime/plaid/resources/config/apis.toml | Adds a named request (test-async) used by the async integration test. |
| runtime/plaid/Cargo.toml | Bumps plaid crate version to 48.0.0. |
| runtime/plaid-stl/src/messages.rs | Adds LogSource::AsyncCompletion. |
| runtime/plaid-stl/src/lib.rs | Adds async entrypoint macro + new error variants and error-code mapping changes. |
| runtime/plaid-stl/src/async/mod.rs | Adds guest-side async API (AsyncContext, ticket types/status/result envelopes). |
| runtime/plaid-stl/Cargo.toml | Bumps plaid_stl crate version to 48.0.0. |
| runtime/Cargo.lock | Updates lockfile versions for plaid and plaid_stl. |
| modules/tests/test_async/src/lib.rs | Adds a test rule demonstrating chained async continuations with state echo. |
| modules/tests/test_async/harness/harness.sh | Adds integration harness script validating async completion chain. |
| modules/tests/test_async/Cargo.toml | Adds the test_async module crate manifest. |
| modules/Cargo.toml | Adds tests/test_async to the modules workspace. |
| modules/Cargo.lock | Adds test_async and bumps plaid_stl to 48.0.0 for modules. |
| ASYNC_IMPLEMENTATION_REPORT.md | Adds a detailed design/implementation report for async mode. |
Review details
Suppressed comments (1)
runtime/plaid/src/async_ops/dispatch.rs:559
- This inline "MM:" note reads like a leftover review comment and makes the feature-gating harder to understand. Please remove it (and consider aligning the cfgs with what’s actually in the list).
#[cfg(not(feature = "aws"))] // MM: what's the link with GCP here? I don't love this.
- Files reviewed: 26/28 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+277
to
+285
| // Happy path: not shutting down, immediate sender available. | ||
| if let (false, Some(sender)) = (cancelled, &immediate_sender) { | ||
| if let Err(e) = sender.try_send(message) { | ||
| let err = e.to_string(); | ||
| let source = e.into_inner().source; | ||
| error!("Failed to deliver async completion from {source}. Error: {err}"); | ||
| } | ||
| return; | ||
| } |
| # completion delivery. | ||
| sleep 10 | ||
|
|
||
| kill $RH_PID 2>&1 > /dev/null |
| .and(warp::path("async_echo")) | ||
| .and(warp::body::bytes()) | ||
| .map(|body: warp::hyper::body::Bytes| { | ||
| let body_str = String::from_utf8(body.to_vec()).unwrap(); |
| # Runtime | ||
|
|
||
| This is where the Plaid runtime code lives along with the STL used by modules. The reason the STL lives in here is because there are shared structures between the `plaid` and `plaid-stl` codebases and having the `plaid-stl` here allows it to be pulled in by modules, but the reverse was not true last time I tried. I also feel these have more in common that the modules do with the STL. No newline at end of file | ||
| This is where the Plaid runtime code lives along with the STL used by modules. The reason the STL lives in here is because there are shared structures between the `plaid` and `plaid-stl` codebases and having the `plaid-stl` here allows it to be pulled in by modules, but the reverse was not true last time I tried. I also feel these have more in common that the modules do with the STL. |
|
|
||
| /// Functions that may be spawned while the owning module is in test mode. | ||
| /// This list mirrors the `ALLOW_IN_TEST_MODE` entries in `functions/api.rs`. | ||
| #[cfg(feature = "aws")] // MM: why is this gated behind the AWS feature? It seems to make no sense. Oh there is another list below... hmmm... not sure how clean this is. |
Comment on lines
+205
to
+219
| let spawned = env.data().api.runtime.spawn(async move { | ||
| let result = fut.await; | ||
| deliver_completion( | ||
| ®istry, | ||
| ticket, | ||
| result, | ||
| module, | ||
| immediate_sender, | ||
| delayed_log_sender, | ||
| cancellation_token, | ||
| ); | ||
| }); | ||
| // Detach: the runtime keeps driving it independent of this | ||
| // execution. The sweeper reaps abandoned tickets. | ||
| std::mem::forget(spawned); |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See implementation report for all the details.