doc: Add lesson learned about dyn async traits.

This commit is contained in:
2026-03-14 01:42:50 -04:00
parent 8bf3366740
commit a39e511cc4

View File

@@ -0,0 +1,64 @@
# Lesson: async fn in traits vs dyn-compatibility
## The problem
The `HookHandler` trait needs to be stored as
`Arc<dyn HookHandler>` so the menu engine can hold a
type-erased handler without knowing whether it's an exec
hook, a handler hook, or a composite. But `async fn` in
traits (even with edition 2024's native support) makes the
trait not dyn-compatible. The compiler can't build a vtable
for an async method because the return type (an opaque
`impl Future`) varies per implementation.
## What we tried
Official documentation of edition 2024 supports async fn
in traits natively, async-trait is NOT needed." That's true
for static dispatch (generics), but wrong for `dyn` dispatch.
## The fix
Made `handle()` a regular sync `fn`. The trait is
deliberately synchronous:
```rust
pub trait HookHandler: Send + Sync {
fn handle(&self, event: HookEvent)
-> Result<Vec<HookResponse>, PiklError>;
}
```
This works because handlers don't actually need to `await`
anything at the call site:
- **Exec hooks** spawn a subprocess via `tokio::spawn` and
return immediately. The process runs in the background.
- **Handler hooks** write to an internal channel via
`try_send` (non-blocking) and return. A background task
handles the actual I/O.
Both return `Ok(vec![])` instantly. Responses from handler
hooks flow back through the action channel asynchronously.
## Takeaway
`async fn in trait` (edition 2024) is for generic/static
dispatch. If you need `dyn Trait`, you still need either:
1. A sync method (if the work can be spawned internally)
2. A boxed future return type
3. The `async-trait` crate
Option 1 is the simplest when the handler doesn't need to
return data from async work. If responses are needed, they
go through a separate channel rather than the return value.
## Related: tokio test-util for debounce tests
Tests that verify debounce timing need `tokio::time::pause`
and `tokio::time::advance`, which require the `test-util`
feature on tokio in dev-dependencies. Use
`#[tokio::test(start_paused = true)]` and
`tokio::time::sleep()` (which auto-advances in paused mode)
rather than manual `pause()` + `advance()` + `yield_now()`
chains, which are fragile with spawned tasks.