fix(envoy-client): dont abort drain after 20s

[MasterPtato]

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

[MasterPtato]

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(rivetkit): use engine actor stop threshold for shutdown #5049
• fix(rivetkit): exit pid1 after signal shutdown #5045
• fix(rivetkit-core): decrement active actor metrics #5044
• fix(envoy-client): dont abort drain after 20s #5043 👈 (View in Graphite)
• temp: patch wasm-pack to point to latest binary #5037
• fix(rivetkit): require engine ping for health #5022
• feat(rivetkit): add current actor metrics #5020
• feat(rivetkit): expose low-cardinality metrics routes #5019
• docs(serverless): document timeout tuning #5018
• feat(kitchen-sink): add load testing harness #5017
• fix(rivetkit): expose client actor metadata #5016
• fix(runtime): avoid bounded dispatch channel stalls #5015
• fix(sqlite): fence remote requests by generation #5014
• feat(rivetkit-core): expose metrics endpoint #5013
• WIP: counter latency monitor #5011
• main

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

[claude]

Code Review

Overview

This PR has two goals: (1) replace the hard-coded 20s SHUTDOWN_DRAIN_TIMEOUT in rivetkit-core with the server-provided actor_stop_threshold from protocol metadata, and (2) refactor the kitchen-sink example to use an explicit resolveMode() helper instead of inline env-var checks.

The kitchen-sink refactor is clean and well-structured. The core Rust change, however, contains critical compile errors that need to be fixed before this is ready to merge.

---

Critical: Compile Errors in registry/mod.rs

Bug 1: SHUTDOWN_DRAIN_TIMEOUT is removed but still referenced

The constant is deleted at the top of the file, but the timeout() call still uses it:

// constant removed ↑
match timeout(
    Duration::from_millis(SHUTDOWN_DRAIN_TIMEOUT as u64),  // ← compile error
    handle.shutdown_and_wait(false),
)

Additionally, SHUTDOWN_DRAIN_TIMEOUT was a Duration — as u64 on a Duration doesn't type-check regardless.

Bug 2: stop_threshold is computed but never used

let stop_threshold = handle
    .get_protocol_metadata  // ← missing () — see Bug 3
    .await
    .map(|x| x.actor_stop_threshold)
    .unwrap_or(30 * 60 * 1000);

match timeout(
    Duration::from_millis(SHUTDOWN_DRAIN_TIMEOUT as u64),  // ← should be stop_threshold
    ...
)

The stop_threshold variable is the intended replacement, but the timeout call still uses the deleted constant. The fix is:

match timeout(
    Duration::from_millis(stop_threshold as u64),
    handle.shutdown_and_wait(false),
)

Bug 3: Missing parentheses on async method call

get_protocol_metadata is defined as pub async fn get_protocol_metadata(&self). The code does:

handle.get_protocol_metadata.await   // ← field access, not a method call

Should be:

handle.get_protocol_metadata().await

---

Minor Issues

resolveMode() default changes implicit behavior

Previously in server.ts, running without any env vars resulted in serverlessMode = false (serverful). With the new code, the same environment defaults to "serverless" mode. This is intentional (the dev script now sets RIVET_KITCHEN_SINK_MODE=serverful explicitly), but it is a behavioral change for any existing local dev setup that relied on the implicit default.

TODO comment is untracked

// TODO: Move into envoy-client since timing out has to do with protocol compliance

This is noted inline but not tracked in .agent/todo/. Consider adding a todo entry so it doesn't get lost.

i64 to u64 cast

actor_stop_threshold is an i64 in the protocol schema. The cast stop_threshold as u64 is fine in practice (threshold is always positive), but wrapping on negative input is silent. A .max(0) guard before the cast would be more explicit.

---

Positive Changes

• The mode.ts refactor is clean: explicit union type, validated env var with a useful error message, and centralized mode detection.
• Removing the hard-coded 20s timeout in favour of the engine-provided actor_stop_threshold (default 30 min) is the right fix — the old timeout was too aggressive and would abort drains before the engine's own lost-timeout fired.
• The comment on the serverless handler guard in server.ts correctly explains why the serverful path must not wire /api/rivet handlers.
• Cosmetic config changes (2_000, 5 * 60 * 1000) improve readability.
• node:22-trixie-slim is a reasonable base image bump.
• Docstring indentation fix in index.d.ts is correct.