Skip to content

Commit 7b630d8

Browse files
committed
feat: US-013 - Avoid panic on websocket_callback_regions ID overflow
1 parent 94aace7 commit 7b630d8

4 files changed

Lines changed: 89 additions & 10 deletions

File tree

rivetkit-typescript/CLAUDE.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,8 @@ Cloudflare Workers forbid `setTimeout`, `fetch`, `connect`, and other async I/O
145145
- Run `pnpm --filter @rivetkit/rivetkit-wasm run check:package` after wasm package export or files changes to verify the published tarball includes the root entrypoint and wasm artifacts.
146146
- Build `@rivetkit/rivetkit-wasm` with its package-local pinned `wasm-pack` dependency. Do not use `npx -y wasm-pack`.
147147
- Intern wasm bridged `RivetErrorSchema` values by `(group, code)` only; live per-error messages must stay on `RivetError.message` instead of expanding the schema cache key.
148-
- Track wasm websocket callback regions in a map keyed by monotonic region IDs and remove entries on end so repeated callbacks do not retain empty slots.
148+
- Track wasm websocket callback regions in a map keyed by active region IDs and remove entries on end so repeated callbacks do not retain empty slots.
149+
- Treat wasm websocket callback region ID `0` as the untracked sentinel; wrap `u32` allocation by skipping live IDs instead of panicking.
149150

150151
## Workflow Context Actor Access Guards
151152

rivetkit-typescript/packages/rivetkit-wasm/src/lib.rs

Lines changed: 74 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,6 +1076,27 @@ impl WasmActorContext {
10761076
next_websocket_callback_region_id: Rc::new(Cell::new(0)),
10771077
}
10781078
}
1079+
1080+
fn allocate_websocket_callback_region_id(
1081+
&self,
1082+
regions: &HashMap<u32, WebSocketCallbackRegion>,
1083+
) -> Option<u32> {
1084+
let start_id = self.next_websocket_callback_region_id.get();
1085+
let mut region_id = start_id;
1086+
1087+
for _ in 0..u32::MAX {
1088+
region_id = region_id.wrapping_add(1);
1089+
if region_id == 0 {
1090+
region_id = 1;
1091+
}
1092+
if !regions.contains_key(&region_id) {
1093+
self.next_websocket_callback_region_id.set(region_id);
1094+
return Some(region_id);
1095+
}
1096+
}
1097+
1098+
None
1099+
}
10791100
}
10801101

10811102
#[wasm_bindgen(js_class = ActorContext)]
@@ -1364,12 +1385,10 @@ impl WasmActorContext {
13641385
#[wasm_bindgen(js_name = beginWebsocketCallback)]
13651386
pub fn begin_websocket_callback(&self) -> u32 {
13661387
let mut regions = self.websocket_callback_regions.borrow_mut();
1367-
let region_id = self
1368-
.next_websocket_callback_region_id
1369-
.get()
1370-
.checked_add(1)
1371-
.expect("websocket callback region id overflow");
1372-
self.next_websocket_callback_region_id.set(region_id);
1388+
let Some(region_id) = self.allocate_websocket_callback_region_id(&regions) else {
1389+
console_error("failed to begin websocket callback region: no region ids available");
1390+
return 0;
1391+
};
13731392
regions.insert(region_id, self.inner.websocket_callback_region());
13741393
region_id
13751394
}
@@ -2890,6 +2909,55 @@ mod tests {
28902909
}
28912910
}
28922911

2912+
#[cfg(target_arch = "wasm32")]
2913+
#[test]
2914+
fn websocket_callback_region_ids_wrap_without_collision() {
2915+
let context = WasmActorContext::from_core(
2916+
rivetkit_core::ActorContext::new(
2917+
"websocket-region-wrap-test",
2918+
"websocket-region-wrap-test",
2919+
Vec::new(),
2920+
"local",
2921+
),
2922+
WasmCallbacks::new(Object::new().into()),
2923+
);
2924+
2925+
let first_id = context.begin_websocket_callback();
2926+
context
2927+
.next_websocket_callback_region_id
2928+
.set(u32::MAX - 1);
2929+
let max_id = context.begin_websocket_callback();
2930+
let wrapped_id = context.begin_websocket_callback();
2931+
2932+
assert_eq!(first_id, 1);
2933+
assert_eq!(max_id, u32::MAX);
2934+
assert_eq!(wrapped_id, 2);
2935+
assert_eq!(context.websocket_callback_regions.borrow().len(), 3);
2936+
assert!(
2937+
context
2938+
.websocket_callback_regions
2939+
.borrow()
2940+
.contains_key(&first_id)
2941+
);
2942+
assert!(
2943+
context
2944+
.websocket_callback_regions
2945+
.borrow()
2946+
.contains_key(&max_id)
2947+
);
2948+
assert!(
2949+
context
2950+
.websocket_callback_regions
2951+
.borrow()
2952+
.contains_key(&wrapped_id)
2953+
);
2954+
2955+
context.end_websocket_callback(first_id);
2956+
context.end_websocket_callback(max_id);
2957+
context.end_websocket_callback(wrapped_id);
2958+
assert!(context.websocket_callback_regions.borrow().is_empty());
2959+
}
2960+
28932961
#[test]
28942962
fn engine_binary_path_error_is_typed_with_field_metadata() {
28952963
let error = validate_wasm_serve_config(&test_serve_config(Some(

scripts/ralph/prd.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@
212212
"Tests pass"
213213
],
214214
"priority": 13,
215-
"passes": false,
216-
"notes": ""
215+
"passes": true,
216+
"notes": "Wasm websocket callback region IDs now wrap across the u32 boundary, skip ID 0, and skip live region IDs instead of panicking. Added wasm-target regression coverage for wraparound with live regions."
217217
},
218218
{
219219
"id": "US-014",

scripts/ralph/progress.txt

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
- `@rivetkit/rivetkit-wasm` builds must use the package-local pinned `wasm-pack` dependency; do not shell through `npx -y wasm-pack`.
1818
- Runtime parity tests can instantiate `NapiCoreRuntime` and `WasmCoreRuntime` with fake binding classes, then drive shared actor glue through `buildNativeFactory(...)` without requiring generated NAPI or wasm artifacts.
1919
- NAPI actor-event synthetic `RivetErrorSchema` values should be `static` when fields are compile-time constants, or process-interned by `(group, code)` with `scc::HashMap` when the default message is dynamic.
20-
- Wasm websocket callback regions should be tracked in a map keyed by monotonic region IDs and removed on end so callback churn does not retain empty slots.
20+
- Wasm websocket callback regions should be tracked in a map keyed by active region IDs and removed on end so callback churn does not retain empty slots.
21+
- Wasm websocket callback region ID `0` is the untracked sentinel; `u32` allocation wraps by skipping live IDs instead of panicking.
2122
- NAPI `Ref::unref` needs an `Env`; cleanup from worker or `Drop` paths should be routed through an Env-bearing TSF and must tolerate addon shutdown by falling back to a bounded leak.
2223
- Core `ActorContext::register_task(...)` must race registered runtime promises against `shutdown_deadline_token()` so shutdown drain cannot hang forever.
2324

@@ -144,3 +145,12 @@ Started: Sat May 2 02:13:32 AM PDT 2026
144145
- Keep native and wasm `register_task(...)` behavior in a shared helper where possible so the two cfg-specific public signatures cannot diverge.
145146
- The package-wide core test suite currently has an unrelated hanging inspector debounce test, so use focused tests plus `check-rivetkit-core-wasm.sh` for this shutdown path.
146147
---
148+
## 2026-05-02 03:33:22 PDT - US-013
149+
- Implemented wraparound allocation for wasm websocket callback region IDs, skipping ID `0` and any IDs still live in the active region map instead of panicking at `u32::MAX`.
150+
- Added a wasm-target regression test that seeds the allocator near `u32::MAX`, keeps a low ID live, and verifies wraparound does not collide.
151+
- Files changed: `rivetkit-typescript/packages/rivetkit-wasm/src/lib.rs`, `rivetkit-typescript/CLAUDE.md`, `scripts/ralph/prd.json`, `scripts/ralph/progress.txt`.
152+
- Checks: `cargo test -p rivetkit-wasm --target wasm32-unknown-unknown --no-run websocket_callback_region_ids_wrap_without_collision`, `pnpm --filter @rivetkit/rivetkit-wasm run check:wasm`, `pnpm --filter @rivetkit/rivetkit-wasm run check-types`, `pnpm --filter @rivetkit/rivetkit-wasm run check:package`.
153+
- **Learnings for future iterations:**
154+
- Wasm websocket callback region allocation preserves the existing `u32` JS surface and uses `0` as the untracked sentinel for the impossible exhausted-ID case.
155+
- Tests that assert wraparound should keep an existing low region ID live so the allocator proves it skips active IDs after wrapping.
156+
---

0 commit comments

Comments
 (0)