Skip to content

feat(rivetkit): add rust counter example#5181

Open
NathanFlurry wants to merge 1 commit into
stack/refactor-depot-remove-sqlite-cold-storage-oxsukmwrfrom
stack/feat-rivetkit-add-rust-counter-example-mtrynmln
Open

feat(rivetkit): add rust counter example#5181
NathanFlurry wants to merge 1 commit into
stack/refactor-depot-remove-sqlite-cold-storage-oxsukmwrfrom
stack/feat-rivetkit-add-rust-counter-example-mtrynmln

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

Adds a standalone examples/rust-counter project built directly on rivetkit-core.

The example includes a runnable counter registry plus an e2e test that starts a temporary rivet-engine, creates a counter actor, calls increment twice, and verifies get returns 2.

Adds a standalone examples/rust-counter project built directly on rivetkit-core.

The example includes a runnable counter registry plus an e2e test that starts a temporary rivet-engine, creates a counter actor, calls increment twice, and verifies get returns 2.
@NathanFlurry
Copy link
Copy Markdown
Member Author

Stack for rivet-dev/rivet

Check out this stack: jj-stack get https://github.com/rivet-dev/rivet/pull/5181
Pull/update this stack: jj-stack sync
Publish local edits: jj-stack submit
Merge when ready: jj-stack merge

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 5, 2026

Code Review: feat(rivetkit): add rust counter example

Overview

This PR adds a standalone examples/rust-counter crate that demonstrates how to build directly on rivetkit-core, including a runnable server and an e2e test that spins up a temporary rivet-engine process. Good addition for developers exploring the Rust path.


Issues

Silent fallback on corrupt state (src/lib.rs:112)

If the stored snapshot is valid CBOR but does not contain a "count" field (e.g. a format mismatch), decode_count silently returns 0 via .unwrap_or_default() and the actor carries on unaware. Should return an explicit error instead.

Brittle error-string matching in wait_for_action (tests/e2e.rs:311)

Retrying on message.contains("actor_ready_timeout") || message.contains("Service Unavailable") is fragile - wording changes or HTTP middleware tweaks silently break the retry logic. Consider matching on HTTP status code (503 / known retry-able codes) instead.

action_output silently returns Null for malformed responses (tests/e2e.rs:334)

If the engine returns valid JSON with no output key, assert_eq!(action_output(&first)?, json!(1)) fails with an unhelpful message. Return an error when the field is absent so the failure message includes the actual body.

Undocumented dual-mode parsing in decode_increment_amount (src/lib.rs:91)

The function accepts either a bare integer or an array whose first element is an integer, with no documentation. The README shows {"args":[]} (empty array), meaning this always defaults to 1. Either add a one-line comment explaining the expected encoding, or simplify to only support the array form to match the README.


Smaller notes

  • workspace_root() (tests/e2e.rs:380) uses .ancestors().nth(2), relying on the example staying exactly two levels under the workspace root. Acceptable for an example, but a comment would clarify the assumption.
  • The polling loops in wait_for_engine_health, wait_for_envoy_ready, and wait_for_action match the pattern CLAUDE.md reserves for external-process health polling, so they are a valid exception - a brief comment on each noting that event-driven wakeup is not possible here would make that intent explicit.
  • shutdown_child uses start_kill (SIGKILL). Fine for test teardown; a SIGTERM + timeout approach would be cleaner if the engine needs to flush state, but not critical for an example.
  • serde dep split is correct: serde_json in main deps (needed by lib), serde and reqwest in dev-deps (tests only).

Positive

  • All ActorEvent variants are matched explicitly - no _ => fallthrough, consistent with CLAUDE.md.
  • Test includes fail-fast logic: checks whether the engine process has already exited before the health deadline.
  • Engine stdout/stderr tail is captured and surfaced in failure messages - very developer-friendly.
  • TestEngine::Drop sends start_kill as a safety net while the explicit shutdown() path waits; clean double-coverage.
  • CancellationToken used throughout for clean shutdown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant