You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add the universal actor onError hook across rivetkit-core, NAPI, wasm, and TypeScript runtime plumbing.
Report action, hook, queue, internal, fatal, timeout, and state-change errors without allowing onError to suppress the original failure.
Document the lifecycle hook and Sentry integration path.
Add driver coverage for actor-level and registry-level delivery, ordering, swallowed hook failures, timeout reporting, and actor.aborted suppression.
Verified with cargo check -p rivetkit-core, pnpm build:force in packages/rivetkit-napi, pnpm build in packages/rivetkit-wasm, and targeted native/wasm actor-onerror driver tests.
Reviewed the Rust core, NAPI/wasm bridges, TS config, and driver tests. Overall the design is sound (typed ActorErrorEvent, HookName/RawErrorRef context markers, suppression of expected lifecycle errors like actor.aborted), but there's one correctness issue worth fixing before merge, plus a few smaller cleanups.
This is a single AtomicBool shared across the whole actor. If two errors are reported at (nearly) the same instant from different tokio tasks — e.g. two concurrent actions failing, or a persist failure racing an action failure — whichever call loses the compare-exchange returns immediately and its ErrorReport is never delivered to onError, with no retry, queue, or log. This directly contradicts the PR's stated goal ("report ... errors without allowing onError to suppress the original failure") — the original failure still propagates to the caller, but the report to onError/Sentry is silently lost.
I don't see evidence this guards against a real reentrancy hazard: the hook itself is invoked non-blocking (NAPI ThreadsafeFunctionCallMode::NonBlocking) or via Promise.resolve().then(...) in the JS bridge (native.ts), so report_error never recurses synchronously through the hook. If the intent was to coalesce a burst of errors, that should be documented and probably scoped more narrowly (e.g. per error class or with a short debounce window) rather than a bare "only one report in flight for the whole actor, everything else is dropped" gate. As-is this looks like it will cause real, hard-to-notice gaps in Sentry/error-tracker coverage under load, and there's no test exercising concurrent report_error calls to catch it.
Simplification / dead code
Triplicated, effectively-dead string-matching fallback in hook_name_from_error (rivetkit-rust/packages/rivetkit-core/src/actor/task.rs:2249, rivetkit-typescript/packages/rivetkit-napi/src/napi_actor_events.rs:895, rivetkit-typescript/packages/rivetkit-wasm/src/lib.rs:606 — identical ~25-line function in all three files):
match cause.to_string().as_str(){"createState" => Some("createState"),"onCreate" => Some("onCreate"),
...}
Every call site that can produce these errors already attaches the typed HookName marker via with_timeout(...) (napi) or explicit .context(HookName("...")) (wasm run_preamble), which the first branch (downcast_ref::<HookName>()) already handles. I couldn't find anywhere in the current codebase that produces a bare anyhow error whose Display is literally "onCreate", "run", etc. without already carrying the typed marker — so this string-match branch looks unreachable today. Worse, it's a latent footgun: if a user's own error happens to have a Display that equals one of these literals (e.g. anyhow!("run"), or a config-validation error whose message is literally "onDestroy"), it will be silently misclassified as that hook's error. Recommend either deleting the fallback (relying solely on the typed marker) or, if it's meant as defense-in-depth, extracting it to one shared helper instead of copy-pasting it three times.
Style
Formatting regression in napi_actor_events.rs — the ActorEvent::RunGracefulCleanup and ActorEvent::DisconnectConn match arms (lines ~638-710) are indented one extra tab level relative to their sibling arms in the same match, and use inconsistent indentation internally. Looks like it wasn't run through the formatter (node scripts/format/agent-format.mjs per CLAUDE.md) after editing.
Minor / efficiency
stashRawError runs unconditionally for every native callback error (rivetkit-typescript/packages/rivetkit/src/registry/native.ts:1201-1209), even when neither the actor nor the registry defines onError. It's bounded (evicted past 1024 entries), so not a real leak, but it's unconditional Map churn on every actor error in the common case where nobody consumes onError. Consider gating stashRawError on whether an onError hook is actually configured (this is already known at factory-build time in buildNativeFactory).
Test coverage
Driver coverage for actor/registry ordering, swallowed hook failures, timeout reporting, and actor.aborted suppression looks solid. Given the concurrency concern above, it'd be worth adding a driver test that triggers two failures for the same actor near-simultaneously (e.g. a failing scheduled action plus a concurrent failing direct action) and asserts both show up in onError.
Nice use of the typed ActorErrorEvent/HookName/RawErrorRef markers to thread hook identity and the original JS error object through the Rust boundary back to native.ts — that's a clean way to preserve instanceof checks (e.g. UserError) in the Sentry example doc without re-serializing errors.
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
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.
onErrorhook across rivetkit-core, NAPI, wasm, and TypeScript runtime plumbing.onErrorto suppress the original failure.actor.abortedsuppression.cargo check -p rivetkit-core,pnpm build:forceinpackages/rivetkit-napi,pnpm buildinpackages/rivetkit-wasm, and targeted native/wasm actor-onerror driver tests.