-
Notifications
You must be signed in to change notification settings - Fork 0
Closed
Description
Problem
Event listeners are typed as (event) => void, but TypeScript allows async functions in void-returning callbacks. When a user (or our own code) passes an async listener:
- The returned Promise is silently ignored — no await, no
.catch() - Rejected Promises never reach
onError, becoming unhandled rejections - Users get no warning that their async work isn't being awaited
Our own code has this bug
withLogPersistence() registers an async listener on log:write:
durably.on('log:write', async (event) => {
await durably.storage.createLog({...}) // Promise silently dropped
})This means log persistence is fire-and-forget with no error handling — not the intended behavior.
Proposed Fix
Based on Codex review feedback:
- Detect Promise-returning listeners at runtime — if a listener returns a thenable, attach
.catch()to forward rejections toonError(but neverawait) - Add dev-time warning when a listener returns a Promise (e.g.,
console.warnin non-production) - Fix
withLogPersistence()— either make log persistence a first-class internal path, or move it to a proper async mechanism that doesn't rely onon()
Design Constraints
emit()must stay synchronous — async emit pushes user callback latency into the worker hot pathon()contract is observational (sync, non-blocking) — this should not change- If awaited async hooks are needed in the future, add a separate
hook()API with explicit semantics
References
- docs: document synchronous event listener behavior #126 (docs fix, separate PR)
packages/durably/src/events.tsL308-334 — emit looppackages/durably/src/plugins/log-persistence.tsL10 — async listener- Codex review findings from docs: document synchronous event listener behavior #126 discussion
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels