DO NOT MERGE: sandbox benches

[NathanFlurry]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix(envoy-client): fix handling actor event ack #4599
• DO NOT MERGE: sandbox benches #4597 👈 (View in Graphite)
• fix(envoy-client): add tls #4595
• chore: fix driver test suite #4591
• chore: tunnel auth #4588
• chore: remove udb as a dependency of envoy-client #4587
• chore: revert actor v2 hack #4586
• fix: fix pool metadata protocol version refresh, rivetkit native ws #4585
• feat: US-001 - Define SqliteKv trait in rivetkit-sqlite-native #4584
• feat(rust/engine-runner): minimal rust engine runner #4289
• chore: delete kv channel #4583
• feat(engine): rust runner #4582
• chore: remove non-engine drivers #4574 : 1 other dependent PR (#4581 )
• feat: dynamic actors #4397
• fix: driver test suite & sqlite reliability #4559
• fix: remove serverless token #4572
• fix: misc token fixes #4569
• fix(pb): clean up actor stop decision handling #4566
• fix(envoy): use global instance, add signal handlers #4565
• chore: misc fixes, add pb snapshot test #4561
• fix(cache): make in memory cache global #4560
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Code Review: DO NOT MERGE: sandbox benches

This PR is marked DO NOT MERGE so these are early-stage notes.

Security: /api/bench has no auth (examples/sandbox/src/server.ts:7-10). The endpoint is publicly accessible on the deployed Cloud Run instance. Anyone who knows the URL can trigger benchmark runs against actor infrastructure. Guard with an internal secret or source IP restriction before deploying to a shared environment.

Bug: startServerlessActor is now fire-and-forget (wrapper.js around line 101). The function was changed from async (payload) => await jsHandle.startServerless(...) to a sync void wrapper with no return value. If the caller awaits it expecting a promise, errors will now be silently dropped. Verify callers do not need the returned promise.

Docker: COPY . . copies the full monorepo (examples/sandbox/Dockerfile). build-sandbox.sh correctly builds Docker from .sandbox-deploy/, but the Dockerfile itself has no guard against being used directly on the repo root. Add a comment clarifying the expected build context.

Build: silent WASM fallback (scripts/docker/build-sandbox.sh:74-78). When the .node binary is missing, the script prints a warning and continues, producing an image that falls back to WASM silently. Since this PR benchmarks native SQLite, a silent fallback means measuring the wrong backend. Consider exit 1 here so CI catches it.

Duplicated callAction: implemented identically in bench.ts, bench-sqlite.ts, bench-wake.ts, and bench-endpoint.ts. Fine for throwaway scripts, but any gateway URL or header change requires four edits.

Minor: package.json is missing a trailing newline (diff shows "No newline at end of file").

Minor: Cargo.toml adds rdkafka as a workspace dependency but no package in this PR uses it. Add it to the consuming package directly or include the consumer in this PR.

Observation: isPlainObject and toNativeBinding were deleted from wrapper.js. Confirm these are unused outside wrapper.js and that binding conversion is now fully handled on the Rust NAPI side.

---

Overall the benchmarking structure is solid. Separating client-side scripts from server-side bench-endpoint.ts is clean, and subtracting baseline RTT to isolate actor processing time from network variance is the right approach.

[claude]

PR Review: sandbox benches

Note: This PR is marked DO NOT MERGE and appears to be a WIP benchmarking branch. Review is provided for early feedback.

Summary

This PR adds a performance benchmarking suite for the sandbox environment, refactors the Docker build process to use pnpm deploy for symlink-free bundles, and makes several changes to wrapper.js including removing the exported openRawDatabaseFromEnvoy helper and converting async callback calls to sync.

---

Issues

Critical

1. startServerlessActor no longer returns the result or propagates errors

// Before
startServerlessActor: async (payload) =>
    await jsHandle.startServerless(Buffer.from(payload)),

// After
startServerlessActor: (payload) => {
    jsHandle.startServerless(Buffer.from(payload));
},

The new version silently drops the Promise. Callers that await startServerlessActor() will now get undefined immediately, and any rejection from startServerless becomes an unhandled Promise rejection. If this is intentional (fire-and-forget), it needs a comment explaining why, and ideally .catch() to log the error.

2. respondCallback calls dropped from await

All .then(async () => { await handle._raw.respondCallback(...) }) callbacks were changed to sync. If respondCallback is async (returns a Promise), the native addon will not wait for the JS side to finish before moving on. This could cause race conditions or lost responses. Verify that respondCallback is fire-and-forget from the native side's perspective before removing await.

Moderate

3. Debug console.log statements left in production code

console.log("[wrapper] websocket_open actorId:", event.actorId?.slice(0, 12), "path:", event.path);
console.log("[wrapper] websocket callback resolved, dispatching open event");
console.log("[wrapper] open event dispatched");

These will emit on every WebSocket connection in production. Replace with the existing tracing/logging mechanism used elsewhere in the codebase.

4. app.all("/api/rivet/*") may not cover all routes the original registry.serve() handled

// Before: registry.serve() — full handler
// After: explicit route prefix only
app.all("/api/rivet/*", (c) => registry.handler(c.req.raw));

If the registry handles routes outside /api/rivet/ (e.g. /actors/, websocket upgrades at /), those silently return 404. Verify the routing is complete.

5. openRawDatabaseFromEnvoy removed without a deprecation path

This was a public export (module.exports.openRawDatabaseFromEnvoy). If any external callers (other packages, user code, tests) depend on it, they will break with no warning. The removal should be accompanied by a migration note or a re-export shim if callers exist.

Minor

6. Hardcoded GCP project ID in build script

AR_PROJECT_ID="${AR_PROJECT_ID:-dev-projects-491221}"

The default value dev-projects-491221 looks like an internal project ID. While overridable via env var, contributors running this script without the env var set will silently target the wrong registry. Consider removing the default or documenting this clearly.

7. rdkafka and raw-foundationdb added to workspace deps but appear unused in this PR

These are non-trivial dependencies (rdkafka requires librdkafka headers at build time). Adding them to [workspace.dependencies] without a consumer in this PR increases build times and CI friction. Remove or defer until they are actually used.

8. Build script does not handle cross-compilation

NATIVE_NODE="$NATIVE_PKG/rivetkit-native.linux-x64-gnu.node"
if [ -f "$NATIVE_NODE" ]; then
  cp "$NATIVE_NODE" "$NATIVE_DST/"
else
  echo "WARNING: $NATIVE_NODE not found, native SQLite will not work"

Mac/ARM developers will hit the warning and silently get a WASM fallback in their Docker image. Consider failing fast (exit 1) or providing a cross-compilation path via cargo build --target x86_64-unknown-linux-gnu.

---

Positive Changes

• Docker simplification: Moving from a full build-in-container approach to pnpm deploy + pre-built bundle is a solid improvement for reproducibility and image size.
• notGlobal ?? false: The null-coalescing default is a good defensive fix.
• database.rs formatting: Import reordering is purely cosmetic and correct.
• Benchmark structure: The server-side /api/bench endpoint design (so latency measurements stay in-datacenter) is a smart approach for accurate benchmarking.