diff --git a/Cargo.lock b/Cargo.lock index 32a31d4fbce9..d12f226a25c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2205,14 +2205,8 @@ dependencies = [ name = "bun_valkey" version = "0.0.0" dependencies = [ - "bitflags", "bstr", "bun_core", - "const_format", - "enum-map", - "enumset", - "libc", - "scopeguard", "strum", ] diff --git a/docs/runtime/redis.mdx b/docs/runtime/redis.mdx index 6e81051bf893..6aef1d9f6c9e 100644 --- a/docs/runtime/redis.mdx +++ b/docs/runtime/redis.mdx @@ -476,6 +476,7 @@ Common error codes: - `ERR_REDIS_CONNECTION_CLOSED` - Connection to the server was closed - `ERR_REDIS_AUTHENTICATION_FAILED` - Failed to authenticate with the server - `ERR_REDIS_INVALID_RESPONSE` - Received an invalid response from the server +- `ERR_REDIS_SERVER_ERROR` - The Redis server returned an error reply --- diff --git a/src/collections/linear_fifo.rs b/src/collections/linear_fifo.rs index 470598502485..83ca4d31b3a2 100644 --- a/src/collections/linear_fifo.rs +++ b/src/collections/linear_fifo.rs @@ -430,6 +430,15 @@ impl> LinearFifo { } } + /// Iterate every readable element in FIFO order across both halves of a + /// possibly-wrapped ring. `readable_slice(0)` alone yields only the first + /// contiguous segment. + pub fn iter(&self) -> impl Iterator { + let first = self.readable_slice(0); + let second = self.readable_slice(first.len()); + first.iter().chain(second.iter()) + } + /// Discard first `count` items in the fifo pub fn discard(&mut self, count: usize) { debug_assert!(count <= self.count); @@ -1098,6 +1107,26 @@ mod tests { assert_eq!(expected, vec![200, 201, 202, 203, 204, 206, 207]); } + #[test] + fn iter_visits_both_halves_of_wrapped_ring() { + let mut fifo = WrapFifo::init(); + for v in 0..12 { + fifo.write_item(v).unwrap(); + } + for _ in 0..8 { + fifo.read_item().unwrap(); + } + for v in 100..110 { + fifo.write_item(v).unwrap(); + } + assert!(fifo.buf_len() - fifo.head < fifo.count, "must be wrapped"); + assert!(fifo.readable_slice(0).len() < fifo.readable_length()); + + let got: Vec = fifo.iter().copied().collect(); + assert_eq!(got, fifo_to_vec(&fifo)); + assert_eq!(got.len(), fifo.readable_length()); + } + // Exhaustively remove every valid offset from a wrapped layout and compare // against a reference `Vec`. Uses a fresh FIFO per offset (remove mutates). #[test] diff --git a/src/jsc/bindings/ErrorCode.ts b/src/jsc/bindings/ErrorCode.ts index d9b47da211f4..7370d6386fc3 100644 --- a/src/jsc/bindings/ErrorCode.ts +++ b/src/jsc/bindings/ErrorCode.ts @@ -292,16 +292,14 @@ const errors: ErrorCodeMapping = [ ["ERR_REDIS_CONNECTION_TIMEOUT", Error, "RedisError"], ["ERR_REDIS_IDLE_TIMEOUT", Error, "RedisError"], ["ERR_REDIS_INVALID_ARGUMENT", Error, "RedisError"], - ["ERR_REDIS_INVALID_ARRAY", Error, "RedisError"], ["ERR_REDIS_INVALID_BULK_STRING", Error, "RedisError"], ["ERR_REDIS_INVALID_COMMAND", Error, "RedisError"], ["ERR_REDIS_INVALID_DATABASE", Error, "RedisError"], - ["ERR_REDIS_INVALID_ERROR_STRING", Error, "RedisError"], ["ERR_REDIS_INVALID_INTEGER", Error, "RedisError"], ["ERR_REDIS_INVALID_PASSWORD", Error, "RedisError"], ["ERR_REDIS_INVALID_RESPONSE", Error, "RedisError"], ["ERR_REDIS_INVALID_RESPONSE_TYPE", Error, "RedisError"], - ["ERR_REDIS_INVALID_SIMPLE_STRING", Error, "RedisError"], + ["ERR_REDIS_SERVER_ERROR", Error, "RedisError"], ["ERR_REDIS_INVALID_STATE", Error, "RedisError"], ["ERR_REDIS_INVALID_USERNAME", Error, "RedisError"], ["ERR_REDIS_TLS_NOT_AVAILABLE", Error, "RedisError"], diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index e3fbc234c67c..891f2898a392 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -113,7 +113,6 @@ use crate::api::{HashObject, JSON5Object, TOMLObject, UnsafeObject, YAMLObject}; use crate::crypto as Crypto; use crate::node; use crate::test_runner::jest::Jest; -use crate::valkey_jsc::js_valkey::SubscriptionCtx; use bun_core::zig_string::Slice as ZigStringSlice; use bun_jsc::ZigStringJsc as _; // to_error_instance / to_type_error_instance use bun_jsc::call_frame::ArgumentsSlice; @@ -2025,21 +2024,7 @@ pub(crate) fn get_valkey_default_client(global_this: &JSGlobalObject, _: &JSObje }; let as_js = JSValkeyClient::ptr_to_js(valkey, global_this); - - // SAFETY: `valkey` is a fresh heap allocation owned by the JS wrapper; we - // hold the only reference for field init below. - let valkey_ref = unsafe { &*valkey }; - valkey_ref.this_value.set(jsc::JsRef::init_weak(as_js)); - match SubscriptionCtx::init(valkey_ref) { - Ok(ctx) => valkey_ref._subscription_ctx.set(ctx), - Err(jsc::JsError::Thrown) | Err(jsc::JsError::Terminated) => return JSValue::ZERO, - Err(err) => { - let _ = - global_this.throw_error(crate::Error::from(err), "Failed to create Redis client"); - return JSValue::ZERO; - } - } - + JSValkeyClient::bind_js(valkey, as_js); as_js } diff --git a/src/runtime/valkey_jsc/ValkeyCommand.rs b/src/runtime/valkey_jsc/command.rs similarity index 56% rename from src/runtime/valkey_jsc/ValkeyCommand.rs rename to src/runtime/valkey_jsc/command.rs index 54d7e94e39c3..55dff51aa1d3 100644 --- a/src/runtime/valkey_jsc/ValkeyCommand.rs +++ b/src/runtime/valkey_jsc/command.rs @@ -1,13 +1,14 @@ use bun_collections::linear_fifo::{DynamicBuffer, LinearFifo}; -use bun_jsc::{self as jsc, JSGlobalObject, JSValue, JsResult}; +use bun_jsc::{self as jsc, JSGlobalObject, JSValue}; use bun_valkey::valkey_protocol as protocol; +use bun_valkey::valkey_protocol::RedisError; use super::protocol_jsc::{ToJSOptions, resp_value_to_js_with_options}; type Slice = bun_core::ZigStringSlice; // Note: callers in `js_valkey_functions.rs` construct -// `Vec` directly, so `Args::Args` must accept +// `Vec` directly, so `Args::Blobs` must accept // that exact type. The upstream `bun_jsc::Node::BlobOrStringOrBuffer` re-export is a // stub; use the real in-crate definition (which already provides `slice()` / // `byte_length()`). @@ -35,7 +36,7 @@ impl<'a> Default for Command<'a> { #[derive(Copy, Clone)] pub enum Args<'a> { Slices(&'a [Slice]), - Args(&'a [BlobOrStringOrBuffer]), + Blobs(&'a [BlobOrStringOrBuffer]), Raw(&'a [&'a [u8]]), } @@ -49,45 +50,49 @@ impl<'a> Args<'a> { pub(crate) fn len(&self) -> usize { match self { Args::Slices(args) => args.len(), - Args::Args(args) => args.len(), + Args::Blobs(args) => args.len(), Args::Raw(args) => args.len(), } } } impl<'a> Command<'a> { - pub fn write(&self, writer: &mut impl bun_io::Write) -> Result<(), crate::Error> { - // Serialize as RESP array format directly - write!(writer, "*{}\r\n", 1 + self.args.len())?; - write!(writer, "${}\r\n", self.command.len())?; - writer.write_all(self.command)?; - writer.write_all(b"\r\n")?; - - match &self.args { - Args::Slices(args) => { - for arg in args.iter() { - let bytes = arg.slice(); - write!(writer, "${}\r\n", bytes.len())?; - writer.write_all(bytes)?; - writer.write_all(b"\r\n")?; + pub fn write(&self, writer: &mut impl bun_io::Write) -> Result<(), RedisError> { + // Serialize as RESP array format directly; `bun_io::Write` can only + // fail with an allocator error, so collapse to `OutOfMemory`. + (|| -> bun_io::Result<()> { + write!(writer, "*{}\r\n", 1 + self.args.len())?; + write!(writer, "${}\r\n", self.command.len())?; + writer.write_all(self.command)?; + writer.write_all(b"\r\n")?; + + match &self.args { + Args::Slices(args) => { + for arg in args.iter() { + let bytes = arg.slice(); + write!(writer, "${}\r\n", bytes.len())?; + writer.write_all(bytes)?; + writer.write_all(b"\r\n")?; + } } - } - Args::Args(args) => { - for arg in args.iter() { - write!(writer, "${}\r\n", arg.byte_length())?; - writer.write_all(arg.slice())?; - writer.write_all(b"\r\n")?; + Args::Blobs(args) => { + for arg in args.iter() { + write!(writer, "${}\r\n", arg.byte_length())?; + writer.write_all(arg.slice())?; + writer.write_all(b"\r\n")?; + } } - } - Args::Raw(args) => { - for arg in args.iter() { - write!(writer, "${}\r\n", arg.len())?; - writer.write_all(arg)?; - writer.write_all(b"\r\n")?; + Args::Raw(args) => { + for arg in args.iter() { + write!(writer, "${}\r\n", arg.len())?; + writer.write_all(arg)?; + writer.write_all(b"\r\n")?; + } } } - } - Ok(()) + Ok(()) + })() + .map_err(|_| RedisError::OutOfMemory) } pub fn byte_length(&self) -> usize { @@ -97,7 +102,7 @@ impl<'a> Command<'a> { counter.count } - pub fn serialize(&self) -> Result, crate::Error> { + pub fn serialize(&self) -> Result, RedisError> { let mut buf: Vec = Vec::with_capacity(self.byte_length()); self.write(&mut buf)?; Ok(buf.into_boxed_slice()) @@ -107,24 +112,16 @@ impl<'a> Command<'a> { /// Command stored in offline queue when disconnected pub struct Entry { pub serialized_data: Box<[u8]>, // Pre-serialized RESP protocol bytes - pub meta: Meta, pub promise: Promise, } -// Inherent associated -// types are unstable on stable Rust, so expose as a sibling module alias instead. -pub mod entry { - pub(crate) type Queue = super::LinearFifo>; -} +pub(crate) type EntryQueue = LinearFifo>; impl Entry { // Create an Offline by serializing the Valkey command directly - pub fn create(command: &Command<'_>, promise: Promise) -> Result { + pub fn create(command: &Command<'_>, promise: Promise) -> Result { Ok(Entry { serialized_data: command.serialize()?, - // We should be calling .check against command here but due - // to a hack introduced to let SUBSCRIBE work, we are not doing that for now. - meta: command.meta, promise, }) } @@ -166,18 +163,49 @@ bun_core::comptime_string_set! { b"PIPELINE", b"SUBSCRIBE", b"PSUBSCRIBE", + b"SSUBSCRIBE", b"UNSUBSCRIBE", - b"UNPSUBSCRIBE", + b"PUNSUBSCRIBE", + b"SUNSUBSCRIBE", + }; +} + +bun_core::comptime_string_set! { + /// Commands whose reply is a subscribe/unsubscribe ack push rather than a + /// normal in-line reply; their promise must carry SUBSCRIPTION_REQUEST so + /// the push handler knows which in-flight entry to consume. + static SUBSCRIPTION_COMMANDS = { + b"SUBSCRIBE", + b"PSUBSCRIBE", + b"SSUBSCRIBE", + b"UNSUBSCRIBE", + b"PUNSUBSCRIBE", + b"SUNSUBSCRIBE", }; } impl Meta { - pub fn check(self, command: &Command<'_>) -> Self { + pub fn check(self, command_name: &[u8]) -> Self { let mut new = self; + // Case-insensitive probe: all disallowed entries are ≤12 bytes, so any + // name longer than our 32-byte scratch cannot match and can skip the copy. + let mut upper = [0u8; 32]; + let n = command_name.len().min(32); + for i in 0..n { + upper[i] = command_name[i].to_ascii_uppercase(); + } + let probe: &[u8] = if command_name.len() <= 32 { + &upper[..n] + } else { + command_name + }; new.set( Meta::SUPPORTS_AUTO_PIPELINING, - !AUTO_PIPELINE_DISALLOWED_COMMANDS.contains(command.command), + !AUTO_PIPELINE_DISALLOWED_COMMANDS.contains(probe), ); + if SUBSCRIPTION_COMMANDS.contains(probe) { + new.insert(Meta::SUBSCRIPTION_REQUEST); + } new } } @@ -188,6 +216,8 @@ pub struct Promise { pub promise: jsc::JSPromiseStrong, } +pub(crate) type PromiseQueue = LinearFifo>; + impl Promise { pub fn create(global_object: &JSGlobalObject, meta: Meta) -> Promise { let promise = jsc::JSPromiseStrong::init(global_object); @@ -197,8 +227,13 @@ impl Promise { pub fn resolve( &mut self, global_object: &JSGlobalObject, - value: &mut protocol::RESPValue, + mut value: protocol::RESPValue, ) -> Result<(), jsc::JsTerminated> { + if self.meta.contains(Meta::RETURN_AS_BOOL) { + if let protocol::RESPValue::Integer(int_value) = value { + value = protocol::RESPValue::Boolean(int_value > 0); + } + } let options = ToJSOptions { return_as_buffer: self.meta.contains(Meta::RETURN_AS_BUFFER), }; @@ -206,7 +241,7 @@ impl Promise { let js_value = match resp_value_to_js_with_options(value, global_object, options) { Ok(v) => v, Err(err) => { - self.reject(global_object, Ok(global_object.take_error(err)))?; + self.promise.reject(global_object, Err(err))?; return Ok(()); } }; @@ -217,32 +252,9 @@ impl Promise { pub fn reject( &mut self, global_object: &JSGlobalObject, - jsvalue: JsResult, - ) -> Result<(), jsc::JsTerminated> { - self.promise.reject(global_object, jsvalue)?; - Ok(()) - } -} - -// Command+Promise pair for tracking which command corresponds to which promise -pub struct PromisePair { - pub meta: Meta, - pub promise: Promise, -} - -// See `entry` note above. -pub mod promise_pair { - pub(crate) type Queue = - super::LinearFifo>; -} - -impl PromisePair { - pub fn reject_command( - &mut self, - global_object: &JSGlobalObject, - jsvalue: JSValue, + value: JSValue, ) -> Result<(), jsc::JsTerminated> { - self.promise.reject(global_object, Ok(jsvalue))?; + self.promise.reject(global_object, Ok(value))?; Ok(()) } } diff --git a/src/runtime/valkey_jsc/index.rs b/src/runtime/valkey_jsc/index.rs deleted file mode 100644 index 92a2a92fb8bf..000000000000 --- a/src/runtime/valkey_jsc/index.rs +++ /dev/null @@ -1,21 +0,0 @@ -// Entry point for Valkey client -// -// Exports: -// - Core Valkey client implementation in valkey.rs -// - JavaScript wrapper in js_valkey.rs -// - Valkey protocol implementation in valkey_protocol.rs - -// Import modules -pub use super::js_valkey; -pub use super::valkey; -pub use bun_valkey::valkey_protocol as protocol; - -// Export JS client -pub use super::js_valkey::JSValkeyClient; - -// Re-export key types for easy access -pub use super::valkey::Options; -pub use super::valkey::Protocol; -pub use super::valkey::Status; -pub use super::valkey::ValkeyClient; -pub use super::valkey_command as Command; diff --git a/src/runtime/valkey_jsc/js_valkey.rs b/src/runtime/valkey_jsc/js_valkey.rs index 9f6c186dcb90..86ee47d2ea44 100644 --- a/src/runtime/valkey_jsc/js_valkey.rs +++ b/src/runtime/valkey_jsc/js_valkey.rs @@ -15,36 +15,13 @@ use bun_jsc::{ use bun_ptr::{AsCtxPtr, BackRef, ScopedRef}; use bun_uws as uws; +use super::command; +use super::command::Command; use super::protocol_jsc; use super::valkey; -use super::valkey_command_body as command; -use super::valkey_command_body::Command; use bun_jsc::url::URL; use bun_valkey::valkey_protocol as protocol; -/// `bun.JSTerminated!T` -// Widened to `JsResult` to match `valkey.rs`; can be narrowed once -// `ValkeyClient::{fail,on_open,on_close,start}` are tightened to the -// `jsc::JsTerminatedResult` alias from `bun_jsc::event_loop`. -type JsTerminatedResult = jsc::JsResult; - -/// Narrow `valkey::ValkeyClient`'s `JsResult<()>` (its local `JsTerminated` -/// alias) back to the spec'd `bun.JSTerminated!void`. The inner client only -/// ever propagates `JsError::Terminated` (originating from `JSPromise::reject` -/// / `resolve`); the other variants are unreachable on this path. -// While `JsTerminatedResult` is widened to `JsResult` (see above), this is -// effectively identity-with-OOM-crash. Once both aliases tighten to -// `jsc::JsTerminatedResult`, restore the `JsTerminated::JSTerminated` mapping. -#[inline] -fn narrow_terminated(r: JsResult<()>) -> JsTerminatedResult<()> { - r.map_err(|e| match e { - jsc::JsError::Terminated => jsc::JsError::Terminated, - jsc::JsError::OutOfMemory => bun_core::out_of_memory(), - // valkey.rs never throws into JS from these paths; treat as terminal. - jsc::JsError::Thrown => jsc::JsError::Terminated, - }) -} - // ─────────────────────────────────────────────────────────────────────────── // Local shims / extension traits (adapt-on-our-side) // ─────────────────────────────────────────────────────────────────────────── @@ -64,11 +41,16 @@ type Socket = uws::AnySocket; // SubscriptionCtx // ─────────────────────────────────────────────────────────────────────────── +#[derive(Clone, Copy)] +pub struct SavedFlags { + pub enable_offline_queue: bool, + pub enable_auto_pipelining: bool, +} + #[derive(Default)] pub struct SubscriptionCtx { - pub is_subscriber: bool, - pub original_enable_offline_queue: bool, - pub original_enable_auto_pipelining: bool, + /// `Some` while in subscriber mode; holds the flag values to restore on exit. + pub saved_flags: Option, } /// The generate-classes.ts output emits a @@ -76,60 +58,53 @@ pub struct SubscriptionCtx { /// free-fns plus `to_js`/`from_js`. Re-exported here as `Js`. pub use crate::generated_classes::js_RedisClient as Js; -// SAFETY: `SubscriptionCtx` lives at `JSValkeyClient._subscription_ctx` +// SAFETY: `SubscriptionCtx` lives at `JSValkeyClient.subscription_ctx` // (intrusive backref). `JsCell` is `#[repr(transparent)]`. -bun_core::impl_field_parent! { SubscriptionCtx => JSValkeyClient._subscription_ctx; fn parent; } +bun_core::impl_field_parent! { SubscriptionCtx => JSValkeyClient.subscription_ctx; fn parent; } impl SubscriptionCtx { - pub fn init(valkey_parent: &JSValkeyClient) -> JsResult { + pub fn init(valkey_parent: &JSValkeyClient, parent_this: JSValue) -> Self { let callback_map = JSMap::create(&valkey_parent.global_object); - let parent_this = valkey_parent - .this_value - .get() - .try_get() - .expect("unreachable"); - Js::subscription_callback_map_set_cached( parent_this, &valkey_parent.global_object, callback_map, ); - Ok(SubscriptionCtx { - original_enable_offline_queue: valkey_parent.client.get().flags.enable_offline_queue, - original_enable_auto_pipelining: valkey_parent - .client - .get() - .flags - .enable_auto_pipelining, - is_subscriber: false, - }) + SubscriptionCtx { saved_flags: None } } - fn subscription_callback_map(&self) -> &mut JSMap { - let parent_this = self - .parent() - .this_value - .get() - .try_get() - .expect("unreachable"); - let value_js = Js::subscription_callback_map_get_cached(parent_this).unwrap(); + #[inline] + pub fn is_subscriber(&self) -> bool { + self.saved_flags.is_some() + } + + /// `None` once the JS wrapper has been finalized (or before `init()`). + fn subscription_callback_map(&self) -> Option<&mut JSMap> { + let parent_this = self.parent().this_value.get().try_get()?; + let value_js = Js::subscription_callback_map_get_cached(parent_this)?; // `JSMap` is an `opaque_ffi!` ZST — `opaque_mut` is the safe deref. - // `from_js` returns a non-null heap cell when the slot was set by - // `init()`; single JS thread. - JSMap::opaque_mut(JSMap::from_js(value_js).unwrap().as_ptr()) + Some(JSMap::opaque_mut(JSMap::from_js(value_js)?.as_ptr())) } /// Get the total number of channels that this subscription context is subscribed to. - pub fn channels_subscribed_to_count(&self, global_object: &JSGlobalObject) -> JsResult { - let count = self.subscription_callback_map().size(global_object)?; - Ok(count) + pub fn channels_subscribed_to_count(&self, global_object: &JSGlobalObject) -> u32 { + let Some(map) = self.subscription_callback_map() else { + return 0; + }; + match map.size(global_object) { + Ok(n) => n, + Err(e) => { + global_object.report_active_exception_as_unhandled(e); + 0 + } + } } /// Test whether this context has any subscriptions. It is mandatory to /// guard deinit with this function. - pub fn has_subscriptions(&self, global_object: &JSGlobalObject) -> JsResult { - Ok(self.channels_subscribed_to_count(global_object)? > 0) + pub fn has_subscriptions(&self, global_object: &JSGlobalObject) -> bool { + self.channels_subscribed_to_count(global_object) > 0 } pub fn clear_receive_handlers( @@ -137,13 +112,18 @@ impl SubscriptionCtx { global_object: &JSGlobalObject, channel_name: JSValue, ) -> JsResult<()> { - let map = self.subscription_callback_map(); + let Some(map) = self.subscription_callback_map() else { + return Ok(()); + }; let _ = map.remove(global_object, channel_name)?; Ok(()) } pub fn clear_all_receive_handlers(&self, global_object: &JSGlobalObject) -> JsResult<()> { - self.subscription_callback_map().clear(global_object) + let Some(map) = self.subscription_callback_map() else { + return Ok(()); + }; + map.clear(global_object) } /// Remove a specific receive handler. @@ -158,7 +138,9 @@ impl SubscriptionCtx { channel_name: JSValue, callback: JSValue, ) -> JsResult> { - let map = self.subscription_callback_map(); + let Some(map) = self.subscription_callback_map() else { + return Ok(None); + }; let existing = map.get(global_object, channel_name)?; if existing.is_undefined_or_null() { @@ -208,31 +190,17 @@ impl SubscriptionCtx { let _guard = scopeguard::guard(parent_br, |p| { p.on_new_subscription_callback_insert(); }); - let map = self.subscription_callback_map(); - - let handlers_array: JSValue; - let mut is_new_channel = false; - let existing_handler_arr = map.get(global_object, channel_name)?; - if existing_handler_arr != JSValue::UNDEFINED { - debug!("Adding a new receive handler."); - // Note that we need to cover this case because maps in JSC can return undefined when - // the key has never been set. - if existing_handler_arr.is_undefined() { - // Create a new array if the existing_handler_arr is undefined/null - handlers_array = JSArray::create_empty(global_object, 0)?; - is_new_channel = true; - } else if existing_handler_arr.is_array() { - // Use the existing array - handlers_array = existing_handler_arr; - } else { - unreachable!(); - } + let Some(map) = self.subscription_callback_map() else { + return Ok(()); + }; + + let existing = map.get(global_object, channel_name)?; + let handlers_array = if existing.is_undefined_or_null() { + JSArray::create_empty(global_object, 0)? } else { - // No existing_handler_arr exists, create a new array - handlers_array = JSArray::create_empty(global_object, 0)?; - is_new_channel = true; - } - let _ = is_new_channel; + debug_assert!(existing.is_array()); + existing + }; // Append the new callback to the array handlers_array.push(global_object, callback)?; @@ -247,9 +215,10 @@ impl SubscriptionCtx { global_object: &JSGlobalObject, channel_name: JSValue, ) -> JsResult> { - let result = self - .subscription_callback_map() - .get(global_object, channel_name)?; + let Some(map) = self.subscription_callback_map() else { + return Ok(None); + }; + let result = map.get(global_object, channel_name)?; if result == JSValue::UNDEFINED { return Ok(None); } @@ -267,10 +236,7 @@ impl SubscriptionCtx { let Some(callbacks) = self.get_callbacks(global_object, channel_name)? else { debug!( "No callbacks found for channel {}", - // `JSString` is an `opaque_ffi!` ZST — `opaque_ref` is the safe - // deref (`as_string()` returns a live cell for string values). - bun_jsc::JSString::opaque_ref(channel_name.as_string()) - .get_zig_string(global_object) + bun_core::OwnedString::new(channel_name.to_bun_string(global_object)?) ); return Ok(()); }; @@ -299,31 +265,6 @@ impl SubscriptionCtx { } Ok(()) } - - /// Return whether the subscription context is ready to be deleted by the JS garbage collector. - pub fn is_deletable(&self, global_object: &JSGlobalObject) -> JsResult { - // The user may request .close(), in which case we can dispose of the subscription object. - // If that is the case, finalized will be true. Otherwise, we should treat the object as - // disposable if there are no active subscriptions. - Ok(self.parent().client.get().flags.finalized || !self.has_subscriptions(global_object)?) - } - - // Cannot be `Drop` — takes a `global_object` param. Exposed as explicit - // `close` per PORTING.md (never expose `pub fn deinit`). - pub fn close(&self, global_object: &JSGlobalObject) { - if cfg!(debug_assertions) { - let go = self.parent().global_object; - debug_assert!(self.is_deletable(&go).expect("unreachable")); - } - - if let Some(parent_this) = self.parent().this_value.get().try_get() { - Js::subscription_callback_map_set_cached( - parent_this, - global_object, - JSValue::UNDEFINED, - ); - } - } } // ─────────────────────────────────────────────────────────────────────────── @@ -352,10 +293,10 @@ pub struct JSValkeyClient { pub this_value: JsCell, pub poll_ref: JsCell, - pub _subscription_ctx: JsCell, + pub subscription_ctx: JsCell, /// `us_ssl_ctx_t` for `tls: { …custom CA… }`. `tls: true` borrows - /// `RareData.defaultClientSslCtx()` instead; `tls: false` leaves this null. - pub _secure: Cell>, + /// `RareData.defaultClientSslCtx()` instead; `tls: false` leaves this `None`. + pub ssl_ctx: JsCell>, pub timer: RefCountedTimer, pub reconnect_timer: RefCountedTimer, @@ -471,6 +412,204 @@ impl bun_ptr::RefCounted for JSValkeyClient { } } +/// Connection parameters extracted from a `valkey://` / `redis://` URL. +struct ParsedValkeyUrl { + address: valkey::Address, + username: Box<[u8]>, + password: Box<[u8]>, + database: u32, + tls_from_scheme: bool, +} + +/// Parse a Valkey/Redis connection URL into owned connection parameters. +/// +/// Accepts `redis[s]://`, `valkey[s]://`, `*+unix://`, `*+tls://` (see +/// [`valkey::Protocol::MAP`]). A bare `host[:port][/db]` with no scheme is +/// prefixed with `valkey://` before parsing. Throws a JS `TypeError` for +/// malformed URLs / unknown schemes / bad port / bad db index. +fn parse_valkey_url( + global_object: &JSGlobalObject, + url_str: &BunString, +) -> JsResult { + let mut fallback_url_buf = [0u8; 2048]; + + // Parse and validate the URL using `URL::from_string`, which returns null for invalid URLs. + // `URL::from_string` does not reject scheme-less input, so check for `://` explicitly first. + let parsed_url: NonNull = 'get_url: { + let url_slice = url_str.to_utf8(); + let url_byte_slice = url_slice.slice(); + + if url_byte_slice.is_empty() { + return Err(global_object.throw_invalid_arguments(format_args!("Invalid URL format"))); + } + + if strings::contains(url_byte_slice, b"://") { + break 'get_url match URL::from_utf8(url_byte_slice) { + Some(u) => u, + None => { + return Err( + global_object.throw_invalid_arguments(format_args!("Invalid URL format")) + ); + } + }; + } + + let corrected_url = 'get_url_slice: { + use std::io::Write; + let mut cursor = &mut fallback_url_buf[..]; + let start_len = cursor.len(); + // No NUL terminator needed here — we immediately re-parse via fromUTF8. + if write!(&mut cursor, "valkey://").is_err() + || cursor.write_all(url_byte_slice).is_err() + { + return Err(global_object.throw_invalid_arguments(format_args!("URL is too long."))); + } + let written = start_len - cursor.len(); + break 'get_url_slice &fallback_url_buf[..written]; + }; + + match URL::from_utf8(corrected_url) { + Some(u) => u, + None => { + return Err( + global_object.throw_invalid_arguments(format_args!("Invalid URL format")) + ); + } + } + }; + // SAFETY: `from_utf8` heap-allocates; release on scope exit. + let _parsed_url_drop = scopeguard::guard(parsed_url, |p| unsafe { URL::destroy(p.as_ptr()) }); + // `_parsed_url_drop` keeps the heap `URL` live for this scope, so the + // `BackRef` liveness invariant holds; `Deref` encapsulates the single + // `NonNull::as_ref` site. + let parsed_url = bun_ptr::BackRef::from(parsed_url); + + // Extract protocol string + let protocol_str = parsed_url.protocol(); + let protocol_utf8 = protocol_str.to_utf8(); + // Remove the trailing ':' from protocol (e.g., "redis:" -> "redis") + let p = protocol_utf8.slice(); + let protocol_slice = if !p.is_empty() && p[p.len() - 1] == b':' { + &p[..p.len() - 1] + } else { + p + }; + + let uri: valkey::Protocol = if !protocol_slice.is_empty() { + match valkey::Protocol::MAP.get(protocol_slice) { + Some(v) => *v, + None => { + let mut list = std::string::String::new(); + for (i, k) in valkey::Protocol::MAP.keys().enumerate() { + if i > 0 { + list.push_str(", "); + } + list.push_str(core::str::from_utf8(k).unwrap_or("?")); + } + return Err( + global_object.throw(format_args!("Expected url protocol to be one of {list}",)) + ); + } + } + } else { + valkey::Protocol::Standalone + }; + + // Extract all URL components + let username_str = parsed_url.username(); + let username_utf8 = username_str.to_utf8(); + + let password_str = parsed_url.password(); + let password_utf8 = password_str.to_utf8(); + + let hostname_str = parsed_url.host(); + let hostname_utf8 = hostname_str.to_utf8(); + + let pathname_str = parsed_url.pathname(); + let pathname_utf8 = pathname_str.to_utf8(); + + // Determine hostname based on protocol type + let hostname_slice: &[u8] = if uri.is_unix() { + // For unix sockets, the path is in the pathname + if pathname_utf8.slice().is_empty() { + return Err(global_object.throw_invalid_arguments(format_args!( + "Expected unix socket path after valkey+unix:// or valkey+tls+unix://", + ))); + } + pathname_utf8.slice() + } else { + hostname_utf8.slice() + }; + + let port: u16 = if uri.is_unix() { + 0 + } else { + 'brk: { + let port_value = parsed_url.port(); + // URL.port() returns u32::MAX if port is not set + if port_value == u32::MAX { + // No port specified, use default + break 'brk 6379; + } else { + // Port was explicitly specified + if port_value == 0 { + // Port 0 is invalid for TCP connections (though it's allowed for unix sockets) + return Err(global_object.throw_invalid_arguments(format_args!( + "Port 0 is not valid for TCP connections", + ))); + } + if port_value > 65535 { + return Err(global_object.throw_invalid_arguments(format_args!( + "Invalid port number in URL. Port must be a number between 0 and 65535", + ))); + } + break 'brk u16::try_from(port_value).expect("int cast"); + } + } + }; + + // Copy strings into owned buffers since the URL object will be deinitialized + let username = Box::<[u8]>::from(username_utf8.slice()); + let password = Box::<[u8]>::from(password_utf8.slice()); + let hostname = Box::<[u8]>::from(hostname_slice); + + // Parse database number from pathname (e.g., "/1" -> database 1) + let database: u32 = if uri.is_unix() { + // For unix sockets the pathname is the socket path, not a db index. + 0 + } else { + let path = pathname_utf8.slice(); + if path.len() > 1 { + match bun_core::fmt::parse_int::(&path[1..], 10) { + Ok(n) => n, + Err(_) => { + return Err(global_object.throw_invalid_arguments(format_args!( + "Invalid database number in Redis URL: {}", + bun_core::fmt::quote(&path[1..]), + ))); + } + } + } else { + 0 + } + }; + + Ok(ParsedValkeyUrl { + address: if uri.is_unix() { + valkey::Address::Unix(hostname) + } else { + valkey::Address::Host { + host: hostname, + port, + } + }, + username, + password, + database, + tls_from_scheme: uri.is_tls(), + }) +} + impl JSValkeyClient { #[inline] pub fn ref_(&self) { @@ -546,279 +685,131 @@ impl JSValkeyClient { Self::create(global_object, callframe.arguments(), js_this) } - /// Create a Valkey client that does not have an associated JS object nor a SubscriptionCtx. + /// Heap-allocate a fresh client in the initial `Disconnected` state. /// - /// This whole client needs a refactor. + /// The single construction site for the `JSValkeyClient` struct literal — + /// shared by both JS construction (`create_no_js_no_pubsub`) and + /// `.duplicate()` (`clone_without_connecting`). `subscription_ctx` is a + /// placeholder here; properly initialized later by [`bind_js`](Self::bind_js). + #[allow(clippy::too_many_arguments)] + fn new_disconnected( + global_object: GlobalRef, + address: valkey::Address, + username: Box<[u8]>, + password: Box<[u8]>, + database: u32, + tls: valkey::TLS, + flags: valkey::ConnectionFlags, + max_retries: u32, + connection_timeout_ms: u32, + idle_timeout_ms: u32, + ) -> *mut JSValkeyClient { + let vm: &'static VirtualMachine = global_object.bun_vm(); + JSValkeyClient::new(JSValkeyClient { + ref_count: bun_ptr::RefCount::init(), + subscription_ctx: JsCell::new(SubscriptionCtx::default()), + client: JsCell::new(valkey::ValkeyClient { + vm, + address, + username, + password, + in_flight: command::PromiseQueue::init(), + queue: command::EntryQueue::init(), + status: valkey::Status::Disconnected, + handshake: valkey::Handshake::default(), + socket: Socket::SocketTcp(uws::SocketTCP { + socket: uws::InternalSocket::Detached, + }), + tls, + database, + flags, + max_retries, + connection_timeout_ms, + idle_timeout_ms, + write_buffer: Default::default(), + read_buffer: Default::default(), + reply_scanner: Default::default(), + retry_attempts: 0, + auto_flusher: Default::default(), + }), + global_object, + this_value: JsCell::new(JsRef::empty()), + poll_ref: JsCell::new(KeepAlive::default()), + ssl_ctx: JsCell::new(None), + timer: RefCountedTimer::new(Timer::Tag::ValkeyConnectionTimeout), + reconnect_timer: RefCountedTimer::new(Timer::Tag::ValkeyConnectionReconnect), + }) + } + + /// Create a Valkey client that does not have an associated JS object nor a SubscriptionCtx. pub fn create_no_js_no_pubsub( global_object: &JSGlobalObject, arguments: &[JSValue], ) -> JsResult<*mut JSValkeyClient> { let global_object = GlobalRef::from(global_object); let vm: &'static VirtualMachine = global_object.bun_vm(); - let vm_ref = vm; let url_str = if arguments.len() >= 1 && !arguments[0].is_undefined_or_null() { arguments[0].to_bun_string(&global_object)? } else { - let env = vm_ref.env_loader(); + let env = vm.env_loader(); match env.get(b"REDIS_URL").or_else(|| env.get(b"VALKEY_URL")) { Some(url) => BunString::borrow_utf8(url), None => BunString::static_(b"valkey://localhost:6379"), } }; // `defer url_str.deref();` — bun_core::String drops on scope exit. - let mut fallback_url_buf = [0u8; 2048]; - - // Parse and validate the URL using `URL::from_string`, which returns null for invalid URLs - // TODO(markovejnovic): The following check for :// is a stop-gap. It is my expectation - // that URL.fromString returns null if the protocol is not specified. This is not, in-fact, - // the case right now and I do not understand why. It will take some work in JSC to - // understand why this is happening, but since I need to uncork valkey, I'm adding this as - // a stop-gap. - let parsed_url: NonNull = 'get_url: { - let url_slice = url_str.to_utf8(); - let url_byte_slice = url_slice.slice(); - - if url_byte_slice.is_empty() { - return Err( - global_object.throw_invalid_arguments(format_args!("Invalid URL format")) - ); - } - if strings::contains(url_byte_slice, b"://") { - break 'get_url match URL::from_utf8(url_byte_slice) { - Some(u) => u, - None => { - return Err(global_object - .throw_invalid_arguments(format_args!("Invalid URL format"))); - } - }; - } - - let corrected_url = 'get_url_slice: { - use std::io::Write; - let mut cursor = &mut fallback_url_buf[..]; - let start_len = cursor.len(); - // No NUL terminator needed here — we immediately re-parse via fromUTF8. - if write!(&mut cursor, "valkey://").is_err() - || cursor.write_all(url_byte_slice).is_err() - { - return Err( - global_object.throw_invalid_arguments(format_args!("URL is too long.")) - ); - } - let written = start_len - cursor.len(); - break 'get_url_slice &fallback_url_buf[..written]; - }; - - match URL::from_utf8(corrected_url) { - Some(u) => u, - None => { - return Err( - global_object.throw_invalid_arguments(format_args!("Invalid URL format")) - ); - } - } - }; - // SAFETY: `from_utf8` heap-allocates; release on scope exit. - let _parsed_url_drop = - scopeguard::guard(parsed_url, |p| unsafe { URL::destroy(p.as_ptr()) }); - // `_parsed_url_drop` keeps the heap `URL` live for this scope, so the - // `BackRef` liveness invariant holds; `Deref` encapsulates the single - // `NonNull::as_ref` site. - let parsed_url = bun_ptr::BackRef::from(parsed_url); - - // Extract protocol string - let protocol_str = parsed_url.protocol(); - let protocol_utf8 = protocol_str.to_utf8(); - // Remove the trailing ':' from protocol (e.g., "redis:" -> "redis") - let p = protocol_utf8.slice(); - let protocol_slice = if !p.is_empty() && p[p.len() - 1] == b':' { - &p[..p.len() - 1] - } else { - p - }; - - let uri: valkey::Protocol = if !protocol_slice.is_empty() { - match valkey::Protocol::MAP.get(protocol_slice) { - Some(v) => *v, - None => return Err(global_object.throw(format_args!( - "Expected url protocol to be one of redis, valkey, rediss, valkeys, redis+tls, redis+unix, redis+tls+unix", - ))), - } - } else { - valkey::Protocol::Standalone - }; - - // Extract all URL components - let username_str = parsed_url.username(); - let username_utf8 = username_str.to_utf8(); - - let password_str = parsed_url.password(); - let password_utf8 = password_str.to_utf8(); - - let hostname_str = parsed_url.host(); - let hostname_utf8 = hostname_str.to_utf8(); - - let pathname_str = parsed_url.pathname(); - let pathname_utf8 = pathname_str.to_utf8(); - - // Determine hostname based on protocol type - let hostname_slice: &[u8] = match uri { - valkey::Protocol::StandaloneTls | valkey::Protocol::Standalone => hostname_utf8.slice(), - valkey::Protocol::StandaloneUnix | valkey::Protocol::StandaloneTlsUnix => { - // For unix sockets, the path is in the pathname - if pathname_utf8.slice().is_empty() { - return Err(global_object.throw_invalid_arguments(format_args!( - "Expected unix socket path after valkey+unix:// or valkey+tls+unix://", - ))); - } - pathname_utf8.slice() - } - }; - - let port: u16 = match uri { - valkey::Protocol::StandaloneUnix | valkey::Protocol::StandaloneTlsUnix => 0, - _ => 'brk: { - let port_value = parsed_url.port(); - // URL.port() returns u32::MAX if port is not set - if port_value == u32::MAX { - // No port specified, use default - break 'brk 6379; - } else { - // Port was explicitly specified - if port_value == 0 { - // Port 0 is invalid for TCP connections (though it's allowed for unix sockets) - return Err(global_object.throw_invalid_arguments(format_args!( - "Port 0 is not valid for TCP connections", - ))); - } - if port_value > 65535 { - return Err(global_object.throw_invalid_arguments(format_args!( - "Invalid port number in URL. Port must be a number between 0 and 65535", - ))); - } - break 'brk u16::try_from(port_value).expect("int cast"); - } - } - }; + let parsed = parse_valkey_url(&global_object, &url_str)?; let options = if arguments.len() >= 2 && !arguments[1].is_undefined_or_null() && arguments[1].is_object() { - Options::from_js(&global_object, arguments[1])? + parse_valkey_options_from_js(&global_object, arguments[1])? } else { valkey::Options::default() }; - // Copy strings into a persistent buffer since the URL object will be deinitialized - let mut connection_strings: Box<[u8]> = Box::default(); - let mut username: Box<[u8]> = Box::default(); - let mut password: Box<[u8]> = Box::default(); - let mut hostname: Box<[u8]> = Box::default(); - - // errdefer free(connection_strings) — handled by Box drop on `?`. - - if !username_utf8.slice().is_empty() - || !password_utf8.slice().is_empty() - || !hostname_slice.is_empty() - { - let mut b = bun_core::StringBuilder::default(); - b.count(username_utf8.slice()); - b.count(password_utf8.slice()); - b.count(hostname_slice); - b.allocate()?; - let user_sp = b.append_count(username_utf8.slice()); - let pass_sp = b.append_count(password_utf8.slice()); - let host_sp = b.append_count(hostname_slice); - connection_strings = b.move_to_slice(); - // `ValkeyClient` owns each field as an independent - // `Box<[u8]>`, so re-slice from the pointers. - username = Box::<[u8]>::from(user_sp.slice(&connection_strings)); - password = Box::<[u8]>::from(pass_sp.slice(&connection_strings)); - hostname = Box::<[u8]>::from(host_sp.slice(&connection_strings)); - } - - // Parse database number from pathname (e.g., "/1" -> database 1) - let database: u32 = match uri { - // For unix sockets the pathname is the socket path, not a db index. - valkey::Protocol::StandaloneUnix | valkey::Protocol::StandaloneTlsUnix => 0, - _ => { - let path = pathname_utf8.slice(); - if path.len() > 1 { - match bun_core::fmt::parse_int::(&path[1..], 10) { - Ok(n) => n, - Err(_) => { - return Err(global_object.throw_invalid_arguments(format_args!( - "Invalid database number in Redis URL: {}", - bun_core::fmt::quote(&path[1..]), - ))); - } - } - } else { - 0 - } - } + let tls = if !options.tls.is_none() { + options.tls + } else if parsed.tls_from_scheme { + valkey::TLS::Enabled + } else { + valkey::TLS::None }; bun_core::analytics::Features::VALKEY.fetch_add(1, core::sync::atomic::Ordering::Relaxed); - // `_subscription_ctx` is a placeholder here; properly initialized later by `create()`. - Ok(JSValkeyClient::new(JSValkeyClient { - ref_count: bun_ptr::RefCount::init(), - _subscription_ctx: JsCell::new(SubscriptionCtx::default()), - client: JsCell::new(valkey::ValkeyClient { - vm, - address: match uri { - valkey::Protocol::StandaloneUnix | valkey::Protocol::StandaloneTlsUnix => { - valkey::Address::Unix(hostname) - } - _ => valkey::Address::Host { - host: hostname, - port, - }, - }, - protocol: uri, - username, - password, - in_flight: command::promise_pair::Queue::init(), - queue: command::entry::Queue::init(), - status: valkey::Status::Disconnected, - connection_strings, - socket: Socket::SocketTcp(uws::SocketTCP { - socket: uws::InternalSocket::Detached, - }), - tls: if options.tls != valkey::TLS::None { - options.tls - } else if uri.is_tls() { - valkey::TLS::Enabled - } else { - valkey::TLS::None - }, - database, - flags: valkey::ConnectionFlags { - enable_auto_reconnect: options.enable_auto_reconnect, - enable_offline_queue: options.enable_offline_queue, - enable_auto_pipelining: options.enable_auto_pipelining, - ..Default::default() - }, - max_retries: options.max_retries, - connection_timeout_ms: options.connection_timeout_ms, - idle_timeout_interval_ms: options.idle_timeout_ms, - write_buffer: Default::default(), - read_buffer: Default::default(), - reply_scanner: Default::default(), - retry_attempts: 0, - auto_flusher: Default::default(), - }), + Ok(Self::new_disconnected( global_object, - this_value: JsCell::new(JsRef::empty()), - poll_ref: JsCell::new(KeepAlive::default()), - _secure: Cell::new(None), - timer: RefCountedTimer::new(Timer::Tag::ValkeyConnectionTimeout), - reconnect_timer: RefCountedTimer::new(Timer::Tag::ValkeyConnectionReconnect), - })) + parsed.address, + parsed.username, + parsed.password, + parsed.database, + tls, + valkey::ConnectionFlags { + enable_auto_reconnect: options.enable_auto_reconnect, + enable_offline_queue: options.enable_offline_queue, + enable_auto_pipelining: options.enable_auto_pipelining, + ..Default::default() + }, + options.max_retries, + options.connection_timeout_ms, + options.idle_timeout_ms, + )) + } + + /// Wire a freshly-allocated client to its JS wrapper: sets `this_value` and + /// initialises the subscription context (which stores a JSMap on `js_this`). + /// Must be called exactly once, after `create_no_js_no_pubsub` / `clone_without_connecting`. + pub fn bind_js(this: *mut Self, js_this: JSValue) { + // SAFETY: `this` is a fresh heap allocation owned by the caller. + let this_ref = unsafe { &*this }; + this_ref.this_value.set(JsRef::init_weak(js_this)); + this_ref + .subscription_ctx + .set(SubscriptionCtx::init(this_ref, js_this)); } pub fn create( @@ -826,19 +817,9 @@ impl JSValkeyClient { arguments: &[JSValue], js_this: JSValue, ) -> JsResult<*mut JSValkeyClient> { - let new_client_ptr = JSValkeyClient::create_no_js_no_pubsub(global_object, arguments)?; - // SAFETY: just allocated above - let new_client = unsafe { &*new_client_ptr }; - - // Initially, we only need to hold a weak reference to the JS object. - new_client.this_value.set(JsRef::init_weak(js_this)); - - // Need to associate the subscription context, after the JS ref has been populated. - new_client - ._subscription_ctx - .set(SubscriptionCtx::init(new_client)?); - - Ok(new_client_ptr) + let ptr = JSValkeyClient::create_no_js_no_pubsub(global_object, arguments)?; + JSValkeyClient::bind_js(ptr, js_this); + Ok(ptr) } /// Clone this client while remaining in the initial disconnected state. @@ -849,180 +830,98 @@ impl JSValkeyClient { &self, global_object: &JSGlobalObject, ) -> Result<*mut JSValkeyClient, bun_alloc::AllocError> { - let global_object = GlobalRef::from(global_object); - let vm: &'static VirtualMachine = global_object.bun_vm(); - let client = self.client.get(); - let sub_ctx = self._subscription_ctx.get(); - - // `ValkeyClient` (see valkey.rs:290-299) owns `username`/`password`/ - // `address.hostname` as independent `Box<[u8]>`s rather than sub-slices - // of the single `connection_strings` allocation, so rebase arithmetic - // against `connection_strings` would compute a garbage offset and read - // OOB. Clone each owned buffer directly. - let connection_strings_copy: Box<[u8]> = Box::<[u8]>::from(&client.connection_strings[..]); - let username: Box<[u8]> = Box::<[u8]>::from(&client.username[..]); - let password: Box<[u8]> = Box::<[u8]>::from(&client.password[..]); - let hostname: Box<[u8]> = Box::<[u8]>::from(client.address.hostname()); - // TODO: we could ref count it instead of cloning it - let tls: valkey::TLS = match &client.tls { - valkey::TLS::None => valkey::TLS::None, - valkey::TLS::Enabled => valkey::TLS::Enabled, - valkey::TLS::Custom(cfg) => valkey::TLS::Custom(cfg.clone()), - }; - - Ok(JSValkeyClient::new(JSValkeyClient { - ref_count: bun_ptr::RefCount::init(), - _subscription_ctx: JsCell::new(SubscriptionCtx::default()), - client: JsCell::new(valkey::ValkeyClient { - vm, - address: match client.protocol { - valkey::Protocol::StandaloneUnix | valkey::Protocol::StandaloneTlsUnix => { - valkey::Address::Unix(hostname) - } - _ => valkey::Address::Host { - host: hostname, - port: match &client.address { - valkey::Address::Host { port, .. } => *port, - valkey::Address::Unix(_) => unreachable!(), - }, - }, - }, - protocol: client.protocol, - username, - password, - in_flight: command::promise_pair::Queue::init(), - queue: command::entry::Queue::init(), - status: valkey::Status::Disconnected, - connection_strings: connection_strings_copy, - socket: Socket::SocketTcp(uws::SocketTCP { - socket: uws::InternalSocket::Detached, - }), - tls, - database: client.database, - flags: valkey::ConnectionFlags { - // Because this starts in the disconnected state, we need to reset some flags. - is_authenticated: false, - // If the user manually closed the connection, then duplicating a closed client - // means the new client remains finalized. - is_manually_closed: client.flags.is_manually_closed, - enable_offline_queue: if sub_ctx.is_subscriber { - sub_ctx.original_enable_offline_queue - } else { - client.flags.enable_offline_queue - }, - needs_to_open_socket: true, - enable_auto_reconnect: client.flags.enable_auto_reconnect, - is_reconnecting: false, - enable_auto_pipelining: if sub_ctx.is_subscriber { - sub_ctx.original_enable_auto_pipelining - } else { - client.flags.enable_auto_pipelining - }, - // Duplicating a finalized client means it stays finalized. - finalized: client.flags.finalized, - ..Default::default() - }, - max_retries: client.max_retries, - connection_timeout_ms: client.connection_timeout_ms, - idle_timeout_interval_ms: client.idle_timeout_interval_ms, - write_buffer: Default::default(), - read_buffer: Default::default(), - reply_scanner: Default::default(), - retry_attempts: 0, - auto_flusher: Default::default(), - }), - global_object, - this_value: JsCell::new(JsRef::empty()), - poll_ref: JsCell::new(KeepAlive::default()), - _secure: Cell::new(None), - timer: RefCountedTimer::new(Timer::Tag::ValkeyConnectionTimeout), - reconnect_timer: RefCountedTimer::new(Timer::Tag::ValkeyConnectionReconnect), - })) + let sub_ctx = self.subscription_ctx.get(); + + Ok(Self::new_disconnected( + GlobalRef::from(global_object), + client.address.clone(), + Box::from(&client.username[..]), + Box::from(&client.password[..]), + client.database, + client.tls.clone(), + valkey::ConnectionFlags { + // If the user manually closed the connection, then duplicating a closed client + // means the new client remains finalized. + is_manually_closed: client.flags.is_manually_closed, + enable_offline_queue: sub_ctx + .saved_flags + .map(|s| s.enable_offline_queue) + .unwrap_or(client.flags.enable_offline_queue), + enable_auto_pipelining: sub_ctx + .saved_flags + .map(|s| s.enable_auto_pipelining) + .unwrap_or(client.flags.enable_auto_pipelining), + enable_auto_reconnect: client.flags.enable_auto_reconnect, + // Duplicating a finalized client means it stays finalized. + finalized: client.flags.finalized, + ..Default::default() + }, + client.max_retries, + client.connection_timeout_ms, + client.idle_timeout_ms, + )) } pub fn add_subscription(&self) { debug!( "addSubscription: entering, current subscriber state: {}", - self._subscription_ctx.get().is_subscriber + self.subscription_ctx.get().is_subscriber() ); debug_assert!(self.client.get().status == valkey::Status::Connected); let _guard = self.ref_scope(); - if !self._subscription_ctx.get().is_subscriber { - let flags = &self.client.get().flags; - let (q, p) = (flags.enable_offline_queue, flags.enable_auto_pipelining); - self._subscription_ctx.with_mut(|s| { - s.original_enable_offline_queue = q; - s.original_enable_auto_pipelining = p; - }); + let flags = &self.client.get().flags; + let (q, p) = (flags.enable_offline_queue, flags.enable_auto_pipelining); + let entered = self.subscription_ctx.with_mut(|s| { + if s.saved_flags.is_none() { + s.saved_flags = Some(SavedFlags { + enable_offline_queue: q, + enable_auto_pipelining: p, + }); + true + } else { + false + } + }); + if entered { debug!("addSubscription: calling updatePollRef"); self.update_poll_ref(); } - - self._subscription_ctx.with_mut(|s| s.is_subscriber = true); debug!( "addSubscription: exiting, new subscriber state: {}", - self._subscription_ctx.get().is_subscriber + self.subscription_ctx.get().is_subscriber() ); } pub fn remove_subscription(&self) { debug!( "removeSubscription: entering, has subscriptions: {}", - self._subscription_ctx + self.subscription_ctx .get() .has_subscriptions(&self.global_object) - .unwrap_or(false) ); let _guard = self.ref_scope(); // This is the last subscription, restore original flags if !self - ._subscription_ctx + .subscription_ctx .get() .has_subscriptions(&self.global_object) - .unwrap_or(false) { - let (q, p) = { - let s = self._subscription_ctx.get(); - ( - s.original_enable_offline_queue, - s.original_enable_auto_pipelining, - ) - }; - self.client_mut().flags.enable_offline_queue = q; - self.client_mut().flags.enable_auto_pipelining = p; - self._subscription_ctx.with_mut(|s| s.is_subscriber = false); + if let Some(saved) = self.subscription_ctx.get().saved_flags { + self.client_mut().flags.enable_offline_queue = saved.enable_offline_queue; + self.client_mut().flags.enable_auto_pipelining = saved.enable_auto_pipelining; + self.subscription_ctx.with_mut(|s| s.saved_flags = None); + } debug!("removeSubscription: calling updatePollRef"); self.update_poll_ref(); } debug!("removeSubscription: exiting"); } - pub fn get_or_create_subscription_ctx(&self) -> JsResult<&SubscriptionCtx> { - // Return the existing ctx so we don't unconditionally reinit. - if self._subscription_ctx.get().is_subscriber { - return Ok(self._subscription_ctx.get()); - } - - // Save the original flag values and create a new subscription context - self._subscription_ctx.set(SubscriptionCtx::init(self)?); - - // We need to make sure we disable the offline queue, but we actually want to make sure - // that our HELLO message goes through first. Consequently, we only disable the offline - // queue if we're already connected. - if self.client.get().status == valkey::Status::Connected { - self.client_mut().flags.enable_offline_queue = false; - } - - self.client_mut().flags.enable_auto_pipelining = false; - - Ok(self._subscription_ctx.get()) - } - pub fn is_subscriber(&self) -> bool { - self._subscription_ctx.get().is_subscriber + self.subscription_ctx.get().is_subscriber() } #[bun_jsc::host_fn(getter)] @@ -1163,36 +1062,25 @@ impl JSValkeyClient { return; } - let mut buf = [0u8; 128]; match self.client.get().status { valkey::Status::Connected => { - use std::io::Write; - let mut cur = &mut buf[..]; - let start = cur.len(); - write!( - &mut cur, - "Idle timeout reached after {}ms", - self.client.get().idle_timeout_interval_ms - ) - .expect("unreachable"); - let len = start - cur.len(); - let msg = &buf[..len]; - let _ = self.client_fail(msg, protocol::RedisError::IdleTimeout); + let _ = self.fail_fmt( + protocol::RedisError::IdleTimeout, + format_args!( + "Idle timeout reached after {}ms", + self.client.get().idle_timeout_ms + ), + ); // TODO: properly propagate exception upwards } valkey::Status::Disconnected | valkey::Status::Connecting => { - use std::io::Write; - let mut cur = &mut buf[..]; - let start = cur.len(); - write!( - &mut cur, - "Connection timeout reached after {}ms", - self.client.get().connection_timeout_ms - ) - .expect("unreachable"); - let len = start - cur.len(); - let msg = &buf[..len]; - let _ = self.client_fail(msg, protocol::RedisError::ConnectionTimeout); + let _ = self.fail_fmt( + protocol::RedisError::ConnectionTimeout, + format_args!( + "Connection timeout reached after {}ms", + self.client.get().connection_timeout_ms + ), + ); // TODO: properly propagate exception upwards } } @@ -1229,14 +1117,31 @@ impl JSValkeyClient { }); if let Err(err) = self.connect() { - self.fail_with_js_value( - self.global_object - .err( - jsc::ErrorCode::SOCKET_CLOSED_BEFORE_CONNECTION, - format_args!("{} reconnecting", err.name()), - ) - .to_js(), - ); + let err_js = self + .global_object + .err( + jsc::ErrorCode::SOCKET_CLOSED_BEFORE_CONNECTION, + format_args!("{} reconnecting", err.name()), + ) + .to_js(); + let _ = self + .client_mut() + .fail_with_js_value(&self.global_object, err_js); + // Socket is already detached here, so close() inside fail_with_js_value + // early-returns and on_valkey_close never fires; replicate its + // connection-promise rejection + onclose here. + if let Some(this_jsvalue) = self.this_value.get().try_get() { + if let Some(promise) = Js::connection_promise_get_cached(this_jsvalue) { + Js::connection_promise_set_cached( + this_jsvalue, + &self.global_object, + JSValue::ZERO, + ); + let _ = JSPromise::opaque_mut(promise.as_promise().unwrap()) + .reject(&self.global_object, Ok(err_js)); + } + } + self.call_onclose_handler(err_js); self.poll_ref.with_mut(|r| r.disable()); return; } @@ -1246,24 +1151,20 @@ impl JSValkeyClient { } // Callback for when Valkey client connects - pub fn on_valkey_connect(&self, value: &mut protocol::RESPValue) -> JsTerminatedResult<()> { + pub fn on_valkey_connect(&self, value: &mut protocol::RESPValue) -> JsResult<()> { debug_assert!(self.client.get().status == valkey::Status::Connected); // we should always have a strong reference to the object here debug_assert!(self.this_value.get().is_strong()); let self_ptr = self.as_ctx_ptr(); - let _defer = scopeguard::guard(self_ptr, |p| { - // SAFETY: `p` was `self.as_ctx_ptr()` at guard creation; the caller - // holds an intrusive ref across this scope so `*p` is live here. - unsafe { - (*p).client_mut().on_writable(); - (*p).update_poll_ref(); - } - }); + // SAFETY: `p` was `self.as_ctx_ptr()` at guard creation; the caller + // holds an intrusive ref across this scope so `*p` is live here. + let _defer = scopeguard::guard(self_ptr, |p| unsafe { (*p).flush_and_update_poll_ref() }); let global_object = self.global_object; let _exit = self.vm().enter_event_loop_scope(); if let Some(this_value) = self.this_value.get().try_get() { + let value = core::mem::replace(value, protocol::RESPValue::Null); let hello_value: JSValue = 'js_hello: { match protocol_jsc::resp_value_to_js(value, &global_object) { Ok(v) => break 'js_hello v, @@ -1306,30 +1207,24 @@ impl JSValkeyClient { /// `SubscriptionCtx` will invoke this to communicate that it has added a new listener. pub fn on_new_subscription_callback_insert(&self) { let _guard = self.ref_scope(); - - self.client_mut().on_writable(); - self.update_poll_ref(); + self.flush_and_update_poll_ref(); } - pub fn on_valkey_subscribe(&self, value: &mut protocol::RESPValue) { + pub fn on_valkey_subscribe(&self) { debug_assert!(self.is_subscriber()); debug_assert!(self.this_value.get().is_strong()); let _guard = self.ref_scope(); - - let _ = value; - - self.client_mut().on_writable(); - self.update_poll_ref(); + self.flush_and_update_poll_ref(); } - pub fn on_valkey_unsubscribe(&self) -> JsResult<()> { - debug_assert!(self.is_subscriber()); + pub fn on_valkey_unsubscribe(&self) { + // A trailing UNSUBSCRIBE ack can legitimately arrive after subscriber + // mode has exited (two queued unsubscribes empty the handler map before + // the second ack lands), so do not assert `is_subscriber()` here. debug_assert!(self.this_value.get().is_strong()); - self.client_mut().on_writable(); - self.update_poll_ref(); - Ok(()) + self.flush_and_update_poll_ref(); } pub fn on_valkey_message(&self, value: &mut [protocol::RESPValue]) { @@ -1348,33 +1243,34 @@ impl JSValkeyClient { } // Extract channel and message - let Ok(channel_value) = protocol_jsc::resp_value_to_js(&mut value[0], &global_object) - else { - debug!("Failed to convert channel to JS"); - return; + let channel = core::mem::replace(&mut value[0], protocol::RESPValue::Null); + let channel_value = match protocol_jsc::resp_value_to_js(channel, &global_object) { + Ok(v) => v, + Err(e) => { + global_object.report_active_exception_as_unhandled(e); + return; + } }; - let Ok(message_value) = protocol_jsc::resp_value_to_js(&mut value[1], &global_object) - else { - debug!("Failed to convert message to JS"); - return; + let message = core::mem::replace(&mut value[1], protocol::RESPValue::Null); + let message_value = match protocol_jsc::resp_value_to_js(message, &global_object) { + Ok(v) => v, + Err(e) => { + global_object.report_active_exception_as_unhandled(e); + return; + } }; // Invoke callbacks for this channel with message and channel as arguments - if self - ._subscription_ctx - .get() - .invoke_callbacks( - &global_object, - channel_value, - &[message_value, channel_value], - ) - .is_err() - { + if let Err(e) = self.subscription_ctx.get().invoke_callbacks( + &global_object, + channel_value, + &[message_value, channel_value], + ) { + global_object.report_active_exception_as_unhandled(e); return; } - self.client_mut().on_writable(); - self.update_poll_ref(); + self.flush_and_update_poll_ref(); } // Callback for when Valkey client needs to reconnect @@ -1390,7 +1286,7 @@ impl JSValkeyClient { } // Callback for when Valkey client closes - pub fn on_valkey_close(&self) -> JsTerminatedResult<()> { + pub fn on_valkey_close(&self) -> JsResult<()> { let global_object = self.global_object; // SAFETY: adopts connect()'s socket keep-alive ref; the caller holds @@ -1431,19 +1327,22 @@ impl JSValkeyClient { Ok(()) } - // Callback for when Valkey client times out - pub fn on_valkey_timeout(&self) { - let _ = self.client_fail( - b"Connection timeout", - protocol::RedisError::ConnectionClosed, - ); + pub fn client_fail(&self, message: &[u8], err: protocol::RedisError) -> JsResult<()> { + self.client_mut().fail(message, err) } - pub fn client_fail(&self, message: &[u8], err: protocol::RedisError) -> JsTerminatedResult<()> { - narrow_terminated(self.client_mut().fail(message, err)) + fn fail_fmt(&self, err: protocol::RedisError, args: core::fmt::Arguments<'_>) -> JsResult<()> { + use std::io::Write; + let mut buf = [0u8; 160]; + let mut cur = &mut buf[..]; + let start = cur.len(); + // Truncation is acceptable for a diagnostic string; ignore the Result. + let _ = cur.write_fmt(args); + let len = start - cur.len(); + self.client_fail(&buf[..len], err) } - pub fn fail_with_js_value(&self, value: JSValue) { + pub fn call_onclose_handler(&self, value: JSValue) { let Some(this_value) = self.this_value.get().try_get() else { return; }; @@ -1525,11 +1424,9 @@ impl JSValkeyClient { this.this_value.with_mut(|t| t.finalize()); this.client_mut().flags.finalized = true; this.close_socket_next_tick(); - // `_subscription_ctx` is three inline bools (no allocation, no GC - // ref); `is_subscriber` can legitimately still be set here if the - // server never confirmed UNSUBSCRIBE before disconnect, since - // `update_poll_ref()` gates on the JS handler map, not this flag. - // Nothing to release. + // `subscription_ctx` is an inline `Option` (no allocation, + // no GC ref); nothing to release. `update_poll_ref()` gates on the JS + // handler map, not this flag. } pub fn stop_timers(&self) { @@ -1548,7 +1445,7 @@ impl JSValkeyClient { // Forgotten on success (the socket adopts it). let socket_ref = self.ref_scope(); - let is_tls = self.client.get().tls != valkey::TLS::None; + let is_tls = !self.client.get().tls.is_none(); // `vm.rare_data()` needs `&mut VirtualMachine`; `client.vm` // is `&'static`. Cast through raw — the per-thread VM is single-owner // on the JS thread, and `valkey_group` only touches the embedded @@ -1566,13 +1463,13 @@ impl JSValkeyClient { } }; - // Populate `_secure` first, then handle the failure branch outside the + // Populate `ssl_ctx` first, then handle the failure branch outside the // borrow of `self.client.tls`. + let mut tls_err = uws::create_bun_socket_error_t::none; let tls_ctx_failed = if let valkey::TLS::Custom(ref custom) = self.client.get().tls { // Reuse across reconnect — the SSL_CTX is the only thing the // old `_socket_ctx` cache existed to preserve. - if self._secure.get().is_none() { - let mut err = uws::create_bun_socket_error_t::none; + if self.ssl_ctx.get().is_none() { // Per-VM weak cache: a `duplicate()`'d client (or any // other client with the same config) hits the same // `SSL_CTX*` instead of rebuilding. @@ -1581,22 +1478,33 @@ impl JSValkeyClient { // SAFETY: per-thread `RuntimeState`; `ssl_ctx_cache` has a // stable address for the VM's lifetime, JS-thread-only. let cache = unsafe { &mut (*state).ssl_ctx_cache }; - self._secure.set(cache.get_or_create(custom, &mut err)); + // SAFETY: `get_or_create` returns a +1-ref `SSL_CTX*` (or null). + self.ssl_ctx.set( + cache + .get_or_create(custom, &mut tls_err) + .and_then(|p| unsafe { boringssl::c::OwnedSslCtx::from_raw(p) }), + ); } - self._secure.get().is_none() + self.ssl_ctx.get().is_none() } else { false }; if tls_ctx_failed { self.client_mut().flags.enable_auto_reconnect = false; - self.client_fail( - b"Failed to create TLS context", + // JS-side failures here are reported, not `?`-propagated: callers + // treat `Err` from `connect()` as a socket-connect syscall failure. + if let Err(e) = self.fail_fmt( protocol::RedisError::ConnectionClosed, - )?; + format_args!("Failed to create TLS context ({:?})", tls_err), + ) { + self.global_object.report_active_exception_as_unhandled(e); + } // `on_valkey_close()` consumes the socket ref; hand it over so it // isn't released twice. socket_ref.forget(); - self.client_mut().on_valkey_close()?; + if let Err(e) = self.on_valkey_close() { + self.global_object.report_active_exception_as_unhandled(e); + } self.client_mut().status = valkey::Status::Disconnected; return Ok(()); } @@ -1606,7 +1514,7 @@ impl JSValkeyClient { // SAFETY: `vm_ptr` is the live per-thread VM (see above). Some(unsafe { crate::jsc_hooks::default_client_ssl_ctx(vm_ptr) }) } - valkey::TLS::Custom(_) => Some(self._secure.get().unwrap()), + valkey::TLS::Custom(_) => Some(self.ssl_ctx.get().as_ref().unwrap().as_ptr()), }; self.client_mut().status = valkey::Status::Connecting; @@ -1643,9 +1551,8 @@ impl JSValkeyClient { pub fn send( &self, global_this: &JSGlobalObject, - _this_value: JSValue, command: &Command, - ) -> Result<*mut JSPromise, crate::Error> { + ) -> Result<*mut JSPromise, protocol::RedisError> { // Keep `*self` alive across re-entrant connect/close paths below; // the host-fn shim passes a bare `&self` with no ref of its own. let _guard = self.ref_scope(); @@ -1663,7 +1570,7 @@ impl JSValkeyClient { .to_js(); let promise = JSPromise::create(global_this); let _exit = self.vm().enter_event_loop_scope(); - promise.reject(global_this, Ok(err_value))?; + let _ = promise.reject(global_this, Ok(err_value)); return Ok(promise); } self.reset_connection_timeout(); @@ -1685,13 +1592,13 @@ impl JSValkeyClient { memory_cost += client.read_buffer.byte_list.capacity() as usize; // Add queue sizes - memory_cost += client.in_flight.readable_length() - * core::mem::size_of::(); - for command in client.queue.readable_slice(0) { + memory_cost += + client.in_flight.readable_length() * core::mem::size_of::(); + for command in client.queue.iter() { memory_cost += command.serialized_data.len(); } memory_cost += - client.queue.readable_length() * core::mem::size_of::(); + client.queue.readable_length() * core::mem::size_of::(); memory_cost } @@ -1709,10 +1616,6 @@ impl JSValkeyClient { debug_assert!(this_ref.client.get().socket.is_closed()); debug_assert!(!this_ref.timer.ref_held.get()); debug_assert!(!this_ref.reconnect_timer.ref_held.get()); - if let Some(s) = this_ref._secure.get() { - // SAFETY: SSL_CTX is C-refcounted; this releases our ref. - unsafe { boringssl::c::SSL_CTX_free(s) }; - } this_ref.client_mut().shutdown(None); this_ref.poll_ref.with_mut(|r| r.disable()); this_ref.stop_timers(); @@ -1726,6 +1629,13 @@ impl JSValkeyClient { drop(unsafe { bun_core::heap::take(this) }); } + /// Flush any buffered outbound writes, then re-evaluate event-loop keep-alive. + #[inline] + fn flush_and_update_poll_ref(&self) { + self.client_mut().on_writable(); + self.update_poll_ref(); + } + /// Keep the event loop alive, or don't keep it alive /// /// This requires this_value to be alive. @@ -1736,21 +1646,10 @@ impl JSValkeyClient { // This is a mess beyond belief and it is incredibly fragile. let has_pending_commands = self.client.get().has_any_pending_commands(); - // isDeletable may throw an exception, and if it does, we have to assume - // that the object still has references. Best we can do is hope nothing - // catastrophic happens. - // - // Once the JS wrapper has been finalized, the subscription callback map - // (stored on the JS object) is gone. Reading it would hit `unreachable` - // in `subscriptionCallbackMap()` because `this_value.tryGet()` returns - // null for a finalized ref. Short-circuit here: a finalized client has - // no subscriptions by definition. - let subs_deletable: bool = self.client.get().flags.finalized - || !self - ._subscription_ctx - .get() - .has_subscriptions(&self.global_object) - .unwrap_or(false); + let subs_deletable = !self + .subscription_ctx + .get() + .has_subscriptions(&self.global_object); let has_activity = has_pending_commands || !subs_deletable || self.client.get().flags.is_reconnecting; @@ -1831,9 +1730,9 @@ impl SocketHandler { } } - pub fn on_open(this: &JSValkeyClient, socket: SocketType) -> JsTerminatedResult<()> { + pub fn on_open(this: &JSValkeyClient, socket: SocketType) -> JsResult<()> { this.client_mut().socket = Self::_socket(socket); - narrow_terminated(this.client_mut().on_open(Self::_socket(socket))) + this.client_mut().on_open(Self::_socket(socket)) } pub fn on_handshake_( @@ -1841,7 +1740,7 @@ impl SocketHandler { _socket: SocketType, success: i32, ssl_error: uws::us_bun_verify_error_t, - ) -> JsTerminatedResult<()> { + ) -> JsResult<()> { debug!( "onHandshake: {} error={} reason={} code={}", success, @@ -1860,13 +1759,13 @@ impl SocketHandler { let handshake_success = success == 1; let _guard = this.ref_scope(); let _update = scopeguard::guard(BackRef::new(this), |p| p.update_poll_ref()); - let vm = this.client.get().vm; if handshake_success { + let vm = this.client.get().vm; if this.client.get().tls.reject_unauthorized(vm) { // only reject the connection if reject_unauthorized == true if ssl_error.error_no != 0 { // Certificate chain validation failed. - return Self::fail_handshake_with_verify_error(this, vm, &ssl_error); + return Self::fail_handshake_with_verify_error(this, &ssl_error); } // Certificate chain is valid; verify the hostname matches the @@ -1918,24 +1817,23 @@ impl SocketHandler { ), ) .to_js(); - return Self::fail_handshake(this, vm, err); + return Self::fail_handshake(this, err); } } - narrow_terminated(this.client_mut().start())?; + this.client_mut().start()?; } else { // if we are here is because the server rejected us, and the error_no is the cause of // this no matter if reject_unauthorized is false, because we were disconnected by the // server - return Self::fail_handshake_with_verify_error(this, vm, &ssl_error); + return Self::fail_handshake_with_verify_error(this, &ssl_error); } Ok(()) } fn fail_handshake_with_verify_error( this: &JSValkeyClient, - vm: &VirtualMachine, ssl_error: &uws::us_bun_verify_error_t, - ) -> JsTerminatedResult<()> { + ) -> JsResult<()> { let ssl_js_value = match crate::socket::uws_jsc::verify_error_to_js(ssl_error, &this.global_object) { Ok(v) => v, @@ -1947,39 +1845,28 @@ impl SocketHandler { // through to the authenticated state after a rejected // handshake. this.global_object.clear_exception(); - this.client_mut().flags.is_authenticated = false; + this.client_mut().handshake = valkey::Handshake::AwaitingHello; this.client_mut().flags.is_manually_closed = true; this.client_mut().close(); return Ok(()); } }; - Self::fail_handshake(this, vm, ssl_js_value) + Self::fail_handshake(this, ssl_js_value) } - fn fail_handshake( - this: &JSValkeyClient, - _vm: &VirtualMachine, - err_value: JSValue, - ) -> JsTerminatedResult<()> { - this.client_mut().flags.is_authenticated = false; + fn fail_handshake(this: &JSValkeyClient, err_value: JSValue) -> JsResult<()> { + this.client_mut().handshake = valkey::Handshake::AwaitingHello; let _exit = this.vm().enter_event_loop_scope(); this.client_mut().flags.is_manually_closed = true; let this_br = BackRef::new(this); let _close = scopeguard::guard(this_br, |p| p.client_mut().close()); - narrow_terminated( - this.client_mut() - .fail_with_js_value(&this.global_object, err_value), - ) + this.client_mut() + .fail_with_js_value(&this.global_object, err_value) } // `pub const onHandshake = if (ssl) onHandshake_ else null;` pub const ON_HANDSHAKE: Option< - fn( - &JSValkeyClient, - SocketType, - i32, - uws::us_bun_verify_error_t, - ) -> JsTerminatedResult<()>, + fn(&JSValkeyClient, SocketType, i32, uws::us_bun_verify_error_t) -> JsResult<()>, > = if SSL { Some(Self::on_handshake_) } else { None }; pub fn on_close( @@ -1987,7 +1874,7 @@ impl SocketHandler { _socket: SocketType, _code: i32, _reason: Option<*mut c_void>, - ) { + ) -> JsResult<()> { debug!("Socket closed."); let _guard = this.ref_scope(); // Ensure the socket pointer is updated. @@ -1997,23 +1884,21 @@ impl SocketHandler { p.update_poll_ref(); }); - let _ = this.client_mut().on_close(); // TODO: properly propagate exception upwards + this.client_mut().on_close() } - pub fn on_end(this: &JSValkeyClient, socket: SocketType) { - let _ = this; - let _ = socket; - + pub fn on_end(_this: &JSValkeyClient, _socket: SocketType) -> JsResult<()> { // Half-opened sockets are not allowed. // usockets will always call onClose after onEnd in this case so we don't need to do // anything here + Ok(()) } pub fn on_connect_error( this: &JSValkeyClient, _socket: SocketType, - _code: i32, - ) -> JsTerminatedResult<()> { + code: i32, + ) -> JsResult<()> { // Ensure the socket pointer is updated. this.client_mut().socket = Socket::SocketTcp(uws::SocketTCP::detached()); let _guard = this.ref_scope(); @@ -2022,30 +1907,31 @@ impl SocketHandler { p.update_poll_ref(); }); - narrow_terminated(this.client_mut().on_close()) + this.client_mut().on_connect_error(code) } - pub fn on_timeout(this: &JSValkeyClient, socket: SocketType) { + pub fn on_timeout(this: &JSValkeyClient, socket: SocketType) -> JsResult<()> { debug!("Socket timed out."); this.client_mut().socket = Self::_socket(socket); - // Handle socket timeout + Ok(()) } - pub fn on_data(this: &JSValkeyClient, socket: SocketType, data: &[u8]) { + pub fn on_data(this: &JSValkeyClient, socket: SocketType, data: &[u8]) -> JsResult<()> { // Ensure the socket pointer is updated. this.client_mut().socket = Self::_socket(socket); let _guard = this.ref_scope(); - let _ = this.client_mut().on_data(data); // TODO: properly propagate exception upwards - this.update_poll_ref(); + let _update = scopeguard::guard(BackRef::new(this), |p| p.update_poll_ref()); + this.client_mut().on_data(data) } - pub fn on_writable(this: &JSValkeyClient, socket: SocketType) { + pub fn on_writable(this: &JSValkeyClient, socket: SocketType) -> JsResult<()> { this.client_mut().socket = Self::_socket(socket); let _guard = this.ref_scope(); + let _update = scopeguard::guard(BackRef::new(this), |p| p.update_poll_ref()); this.client_mut().on_writable(); - this.update_poll_ref(); + Ok(()) } } @@ -2054,82 +1940,77 @@ impl SocketHandler { // ─────────────────────────────────────────────────────────────────────────── // Parse JavaScript options into Valkey client options -struct Options; +fn parse_valkey_options_from_js( + global_object: &JSGlobalObject, + options_obj: JSValue, +) -> JsResult { + let mut this = valkey::Options::default(); -impl Options { - pub(crate) fn from_js( - global_object: &JSGlobalObject, - options_obj: JSValue, - ) -> JsResult { - let mut this = valkey::Options { - enable_auto_pipelining: - !bun_core::env_var::feature_flag::BUN_FEATURE_FLAG_DISABLE_REDIS_AUTO_PIPELINING - .get() - .unwrap_or(false), - ..Default::default() - }; - - if let Some(idle_timeout) = - options_obj.get_optional_int::(global_object, "idleTimeout")? - { - this.idle_timeout_ms = idle_timeout; - } + if let Some(idle_timeout) = options_obj.get_optional_int::(global_object, "idleTimeout")? { + this.idle_timeout_ms = idle_timeout; + } - if let Some(connection_timeout) = - options_obj.get_optional_int::(global_object, "connectionTimeout")? - { - this.connection_timeout_ms = connection_timeout; - } + if let Some(connection_timeout) = + options_obj.get_optional_int::(global_object, "connectionTimeout")? + { + this.connection_timeout_ms = connection_timeout; + } - if let Some(auto_reconnect) = - options_obj.get_if_property_exists(global_object, "autoReconnect")? - { - this.enable_auto_reconnect = auto_reconnect.to_boolean(); - } + if let Some(auto_reconnect) = + options_obj.get_if_property_exists(global_object, "autoReconnect")? + { + this.enable_auto_reconnect = auto_reconnect.to_boolean(); + } - if let Some(max_retries) = - options_obj.get_optional_int::(global_object, "maxRetries")? - { - this.max_retries = max_retries; - } + if let Some(max_retries) = options_obj.get_optional_int::(global_object, "maxRetries")? { + this.max_retries = max_retries; + } - if let Some(enable_offline_queue) = - options_obj.get_if_property_exists(global_object, "enableOfflineQueue")? - { - this.enable_offline_queue = enable_offline_queue.to_boolean(); - } + if let Some(enable_offline_queue) = + options_obj.get_if_property_exists(global_object, "enableOfflineQueue")? + { + this.enable_offline_queue = enable_offline_queue.to_boolean(); + } - if let Some(enable_auto_pipelining) = - options_obj.get_if_property_exists(global_object, "enableAutoPipelining")? - { - this.enable_auto_pipelining = enable_auto_pipelining.to_boolean(); - } + if let Some(enable_auto_pipelining) = + options_obj.get_if_property_exists(global_object, "enableAutoPipelining")? + { + this.enable_auto_pipelining = enable_auto_pipelining.to_boolean(); + } - if let Some(tls) = options_obj.get_if_property_exists(global_object, "tls")? { - if tls.is_boolean() || tls.is_undefined_or_null() { - this.tls = if tls.to_boolean() { - valkey::TLS::Enabled - } else { - valkey::TLS::None - }; - } else if tls.is_object() { - // SAFETY: `bun_vm()` returns the live per-global VM pointer. - if let Some(ssl_config) = - SSLConfig::from_js(global_object.bun_vm(), global_object, tls)? - { - this.tls = valkey::TLS::Custom(Box::new(ssl_config)); - } else { - return Err(global_object.throw_invalid_argument_type("tls", "tls", "object")); - } + if let Some(tls) = options_obj.get_if_property_exists(global_object, "tls")? { + if tls.is_boolean() || tls.is_undefined_or_null() { + this.tls = if tls.to_boolean() { + valkey::TLS::Enabled } else { - return Err(global_object.throw_invalid_argument_type( - "tls", - "tls", - "boolean or object", - )); + valkey::TLS::None + }; + } else if tls.is_object() { + // SAFETY: `bun_vm()` returns the live per-global VM pointer. + if let Some(ssl_config) = + SSLConfig::from_js(global_object.bun_vm(), global_object, tls)? + { + this.tls = valkey::TLS::Custom(Box::new(ssl_config)); + } else { + return Err(global_object.throw_invalid_argument_type("tls", "tls", "object")); } + } else { + return Err(global_object.throw_invalid_argument_type( + "tls", + "tls", + "boolean or object", + )); } + } - Ok(this) + Ok(this) +} + +impl JSValkeyClient { + #[inline] + pub fn ptr_to_js(ptr: *mut Self, global: &JSGlobalObject) -> JSValue { + Js::to_js(ptr, global) } } + +bun_jsc::impl_js_class_via_generated!(JSValkeyClient => crate::generated_classes::js_RedisClient); diff --git a/src/runtime/valkey_jsc/js_valkey_functions.rs b/src/runtime/valkey_jsc/js_valkey_functions.rs index b63d9eb066b4..399d258868aa 100644 --- a/src/runtime/valkey_jsc/js_valkey_functions.rs +++ b/src/runtime/valkey_jsc/js_valkey_functions.rs @@ -3,32 +3,23 @@ use bun_collections::VecExt as _; use bun_core::OwnedString; use bun_jsc::{ self as jsc, CallFrame, ErrorCode, JSGlobalObject, JSPromise, JSPropertyIterator, JSValue, - JsRef, JsResult, + JsResult, }; -use super::js_valkey::{JSValkeyClient, SubscriptionCtx}; +use super::command::{Args as CommandArgs, Command, Meta as CommandMeta}; +use super::js_valkey::JSValkeyClient; use super::protocol_jsc as protocol; use super::valkey; -use super::valkey_command_body::{Args as CommandArgs, Command, Meta as CommandMeta}; +use bun_valkey::valkey_protocol::RedisError; type Slice = bun_jsc::ZigStringSlice; -/// Reinterpret an ASCII byte-string literal as `&str` for the -/// `throw_invalid_argument_type` family (which take `&'static str`). -/// SAFETY: every command/method name passed to the `cmd_*!` macros is a -/// static ASCII byte-string literal, so it is always valid UTF-8. -#[inline(always)] -const fn bname(b: &'static [u8]) -> &'static str { - // SAFETY: every caller passes a `b"..."` ASCII literal (command/method - // names from the `cmd_*!` macros), which is guaranteed valid UTF-8. - unsafe { core::str::from_utf8_unchecked(b) } -} - // ────────────────────────────────────────────────────────────────────────── // Helpers // ────────────────────────────────────────────────────────────────────────── -fn require_not_subscriber(this: &JSValkeyClient, function_name: &[u8]) -> JsResult<()> { +#[inline] +fn require_not_subscriber(this: &JSValkeyClient, function_name: &str) -> JsResult<()> { if this.is_subscriber() { // `global_object: GlobalRef` derefs safely (BACKREF — VM-owned global outlives client). let global: &JSGlobalObject = &this.global_object; @@ -36,8 +27,7 @@ fn require_not_subscriber(this: &JSValkeyClient, function_name: &[u8]) -> JsResu .err( ErrorCode::REDIS_INVALID_STATE, format_args!( - "RedisClient.prototype.{} cannot be called while in subscriber mode.", - bstr::BStr::new(function_name) + "RedisClient.prototype.{function_name} cannot be called while in subscriber mode.", ), ) .throw()); @@ -45,7 +35,8 @@ fn require_not_subscriber(this: &JSValkeyClient, function_name: &[u8]) -> JsResu Ok(()) } -fn require_subscriber(this: &JSValkeyClient, function_name: &[u8]) -> JsResult<()> { +#[inline] +fn require_subscriber(this: &JSValkeyClient, function_name: &str) -> JsResult<()> { if !this.is_subscriber() { // `global_object: GlobalRef` derefs safely (BACKREF — VM-owned global outlives client). let global: &JSGlobalObject = &this.global_object; @@ -53,8 +44,7 @@ fn require_subscriber(this: &JSValkeyClient, function_name: &[u8]) -> JsResult<( .err( ErrorCode::REDIS_INVALID_STATE, format_args!( - "RedisClient.prototype.{} can only be called while in subscriber mode.", - bstr::BStr::new(function_name) + "RedisClient.prototype.{function_name} can only be called while in subscriber mode.", ), ) .throw()); @@ -62,7 +52,7 @@ fn require_subscriber(this: &JSValkeyClient, function_name: &[u8]) -> JsResult<( Ok(()) } -fn from_js(global: &JSGlobalObject, value: JSValue) -> JsResult> { +fn coerce_redis_arg(global: &JSGlobalObject, value: JSValue) -> JsResult> { if value.is_undefined_or_null() { return Ok(None); } @@ -73,21 +63,50 @@ fn from_js(global: &JSGlobalObject, value: JSValue) -> JsResult JsResult { + coerce_redis_arg(global, value)? + .ok_or_else(|| global.throw_invalid_argument_type(method, label, "string or buffer")) } -/// Shim around `protocol::valkey_error_to_js` that: -/// 1. accepts whatever error type `JSValkeyClient::send` currently returns -/// (presently `crate::Error`) and -/// converts it to `RedisError` so the user-visible error code matches the -/// real failure variant, and -/// 2. wraps the resulting `JSValue` in `Ok` for use in `JsResult` -/// host functions. +/// Convert a trailing varargs slice to `JSArgument`s with a single policy: +/// `undefined`/`null`/unsupported values THROW (never silently skip or truncate). +fn collect_varargs( + global: &JSGlobalObject, + args: &[JSValue], + method: &'static str, + label: &'static str, +) -> JsResult> { + let mut out = Vec::with_capacity(args.len()); + for arg in args { + let Some(v) = coerce_redis_arg(global, *arg)? else { + return Err(global.throw_invalid_argument_type(method, label, "string or buffer")); + }; + out.push(v); + } + Ok(out) +} + +/// Return a rejected `Promise` wrapping the Redis error as a +/// `JsResult` for host functions. #[inline] -fn send_err_to_js(global: &JSGlobalObject, message: &str, err: &crate::Error) -> JsResult { - use bun_valkey::valkey_protocol::RedisError; - let redis_err = err.name().parse().unwrap_or(RedisError::ConnectionClosed); - Ok(protocol::valkey_error_to_js(global, message, redis_err)) +fn send_err_to_js( + global: &JSGlobalObject, + message: impl AsRef<[u8]>, + err: RedisError, +) -> JsResult { + let err_value = protocol::valkey_error_to_js(global, message, err); + Ok(JSPromise::rejected_promise(global, err_value).to_js()) } /// `JSValkeyClient::send` returns a `*mut JSPromise`; route through the @@ -101,25 +120,20 @@ fn promise_to_js(p: *mut JSPromise) -> JSValue { /// `this.send()` it, and convert the result to a `JsResult` — /// `Ok(promise.toJS())` on success, a JS-side Redis error value on failure. /// -/// All 7 `cmd_*!` macros and ~24 hand-written methods (`get`, `getBuffer`, -/// `set`, `incr`, `decr`, `exists`, `expire`, `ttl`, `srem`, `sadd`, -/// `sismember`, `hmget`, `hincrby`, `hset`, `smove`, `publish`, -/// `send_unsubscribe_request_and_cleanup`, …) duplicated this 15-line block -/// byte-identically; the only per-caller variation is the args slice, the -/// `meta` flags, and the error-message prefix. +/// All `cmd_*!` macros and the hand-written methods route through here; the +/// only per-caller variation is the args slice and the `meta` flags. The +/// error message is derived from `command` so it can never disagree with the +/// command actually sent. #[inline] fn send_cmd( this: &JSValkeyClient, global: &JSGlobalObject, - this_js: JSValue, command: &[u8], args: CommandArgs<'_>, meta: CommandMeta, - err_msg: &str, ) -> JsResult { match this.send( global, - this_js, &Command { command, args, @@ -127,7 +141,11 @@ fn send_cmd( }, ) { Ok(p) => Ok(promise_to_js(p)), - Err(err) => send_err_to_js(global, err_msg, &err), + Err(err) => send_err_to_js( + global, + format!("Failed to send {} command", bstr::BStr::new(command)), + err, + ), } } @@ -148,7 +166,7 @@ pub(crate) mod compile { pub(crate) fn test_correct_state( this: &JSValkeyClient, - js_client_prototype_function_name: &[u8], + js_client_prototype_function_name: &str, ) -> JsResult<()> { match REQ { ClientStateRequirement::NotSubscriber => { @@ -165,8 +183,7 @@ pub(crate) mod compile { // cmd_key_varargs! (key: RedisKey, ...args: RedisKey[]), // cmd_key_value! (key: RedisKey, value: RedisValue), // cmd_key_value_value2! (key: RedisKey, value: RedisValue, value2: RedisValue), -// cmd_strings_varargs! (...strings: string[]), -// cmd_key_value_varargs! (key: RedisKey, value: RedisValue, ...args: RedisValue) +// cmd_strings_varargs! (...strings: string[]) macro_rules! cmd_noargs { ($fn_name:ident, $name:literal, $command:literal, $state:ident) => { @@ -174,7 +191,7 @@ macro_rules! cmd_noargs { pub fn $fn_name( this: &Self, global: &JSGlobalObject, - frame: &CallFrame, + _frame: &CallFrame, ) -> JsResult { compile::test_correct_state::<{ compile::ClientStateRequirement::$state }>( this, $name, @@ -182,11 +199,9 @@ macro_rules! cmd_noargs { send_cmd( this, global, - frame.this(), $command.as_bytes(), - CommandArgs::Args(&[]), + CommandArgs::Blobs(&[]), CommandMeta::default(), - concat!("Failed to send ", $command), ) } }; @@ -194,6 +209,16 @@ macro_rules! cmd_noargs { macro_rules! cmd_key { ($fn_name:ident, $name:literal, $command:literal, $arg0_name:literal, $state:ident) => { + cmd_key!( + $fn_name, + $name, + $command, + $arg0_name, + $state, + CommandMeta::default() + ); + }; + ($fn_name:ident, $name:literal, $command:literal, $arg0_name:literal, $state:ident, $meta:expr) => { #[bun_jsc::host_fn(method)] pub fn $fn_name( this: &Self, @@ -204,21 +229,13 @@ macro_rules! cmd_key { this, $name, )?; - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type( - bname($name), - $arg0_name, - "string or buffer", - )); - }; + let key = require_arg(global, frame.argument(0), $name, $arg0_name)?; send_cmd( this, global, - frame.this(), $command.as_bytes(), - CommandArgs::Args(&[key]), - CommandMeta::default(), - concat!("Failed to send ", $command), + CommandArgs::Blobs(&[key]), + $meta, ) } }; @@ -236,35 +253,22 @@ macro_rules! cmd_key_varargs { this, $name, )?; - if frame.argument(0).is_undefined_or_null() { - return Err(global.throw_missing_arguments_value(&[$arg0_name])); - } - + let key = require_arg(global, frame.argument(0), $name, $arg0_name)?; let arguments = frame.arguments(); let mut args: Vec = Vec::with_capacity(arguments.len()); - - for arg in arguments { - if arg.is_undefined_or_null() { - continue; - } - - let Some(another) = from_js(global, *arg)? else { - return Err(global.throw_invalid_argument_type( - bname($name), - "additional arguments", - "string or buffer", - )); - }; - args.push(another); - } + args.push(key); + args.extend(collect_varargs( + global, + &arguments[1..], + $name, + "additional arguments", + )?); send_cmd( this, global, - frame.this(), $command.as_bytes(), - CommandArgs::Args(&args), + CommandArgs::Blobs(&args), CommandMeta::default(), - concat!("Failed to send ", $command), ) } }; @@ -272,6 +276,17 @@ macro_rules! cmd_key_varargs { macro_rules! cmd_key_value { ($fn_name:ident, $name:literal, $command:literal, $arg0_name:literal, $arg1_name:literal, $state:ident) => { + cmd_key_value!( + $fn_name, + $name, + $command, + $arg0_name, + $arg1_name, + $state, + CommandMeta::default() + ); + }; + ($fn_name:ident, $name:literal, $command:literal, $arg0_name:literal, $arg1_name:literal, $state:ident, $meta:expr) => { #[bun_jsc::host_fn(method)] pub fn $fn_name( this: &Self, @@ -282,28 +297,14 @@ macro_rules! cmd_key_value { this, $name, )?; - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type( - bname($name), - $arg0_name, - "string or buffer", - )); - }; - let Some(value) = from_js(global, frame.argument(1))? else { - return Err(global.throw_invalid_argument_type( - bname($name), - $arg1_name, - "string or buffer", - )); - }; + let key = require_arg(global, frame.argument(0), $name, $arg0_name)?; + let value = require_arg(global, frame.argument(1), $name, $arg1_name)?; send_cmd( this, global, - frame.this(), $command.as_bytes(), - CommandArgs::Args(&[key, value]), - CommandMeta::default(), - concat!("Failed to send ", $command), + CommandArgs::Blobs(&[key, value]), + $meta, ) } }; @@ -311,52 +312,18 @@ macro_rules! cmd_key_value { macro_rules! cmd_key_value_value2 { ($fn_name:ident, $name:literal, $command:literal, $arg0_name:literal, $arg1_name:literal, $arg2_name:literal, $state:ident) => { - #[bun_jsc::host_fn(method)] - pub fn $fn_name( - this: &Self, - global: &JSGlobalObject, - frame: &CallFrame, - ) -> JsResult { - compile::test_correct_state::<{ compile::ClientStateRequirement::$state }>( - this, $name, - )?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type( - bname($name), - $arg0_name, - "string or buffer", - )); - }; - let Some(value) = from_js(global, frame.argument(1))? else { - return Err(global.throw_invalid_argument_type( - bname($name), - $arg1_name, - "string or buffer", - )); - }; - let Some(value2) = from_js(global, frame.argument(2))? else { - return Err(global.throw_invalid_argument_type( - bname($name), - $arg2_name, - "string or buffer", - )); - }; - send_cmd( - this, - global, - frame.this(), - $command.as_bytes(), - CommandArgs::Args(&[key, value, value2]), - CommandMeta::default(), - concat!("Failed to send ", $command), - ) - } + cmd_key_value_value2!( + $fn_name, + $name, + $command, + $arg0_name, + $arg1_name, + $arg2_name, + $state, + CommandMeta::default() + ); }; -} - -macro_rules! cmd_strings_varargs { - ($fn_name:ident, $name:literal, $command:literal, $state:ident) => { + ($fn_name:ident, $name:literal, $command:literal, $arg0_name:literal, $arg1_name:literal, $arg2_name:literal, $state:ident, $meta:expr) => { #[bun_jsc::host_fn(method)] pub fn $fn_name( this: &Self, @@ -367,32 +334,21 @@ macro_rules! cmd_strings_varargs { this, $name, )?; - let mut args: Vec = Vec::with_capacity(frame.arguments().len()); - - for arg in frame.arguments() { - let Some(another) = from_js(global, *arg)? else { - return Err(global.throw_invalid_argument_type( - bname($name), - "additional arguments", - "string or buffer", - )); - }; - args.push(another); - } + let key = require_arg(global, frame.argument(0), $name, $arg0_name)?; + let value = require_arg(global, frame.argument(1), $name, $arg1_name)?; + let value2 = require_arg(global, frame.argument(2), $name, $arg2_name)?; send_cmd( this, global, - frame.this(), $command.as_bytes(), - CommandArgs::Args(&args), - CommandMeta::default(), - concat!("Failed to send ", $command), + CommandArgs::Blobs(&[key, value, value2]), + $meta, ) } }; } -macro_rules! cmd_key_value_varargs { +macro_rules! cmd_strings_varargs { ($fn_name:ident, $name:literal, $command:literal, $state:ident) => { #[bun_jsc::host_fn(method)] pub fn $fn_name( @@ -404,30 +360,13 @@ macro_rules! cmd_key_value_varargs { this, $name, )?; - let mut args: Vec = Vec::with_capacity(frame.arguments().len()); - - for arg in frame.arguments() { - if arg.is_undefined_or_null() { - continue; - } - - let Some(another) = from_js(global, *arg)? else { - return Err(global.throw_invalid_argument_type( - bname($name), - "additional arguments", - "string or buffer", - )); - }; - args.push(another); - } + let args = collect_varargs(global, frame.arguments(), $name, "additional arguments")?; send_cmd( this, global, - frame.this(), $command.as_bytes(), - CommandArgs::Args(&args), + CommandArgs::Blobs(&args), CommandMeta::default(), - concat!("Failed to send ", $command), ) } }; @@ -444,94 +383,45 @@ impl JSValkeyClient { let args_array = frame.argument(1); if !args_array.is_object() || !args_array.is_array() { - return Err(global.throw(format_args!("Arguments must be an array"))); + return Err(global.throw_invalid_argument_type("send", "args", "array")); } let mut iter = args_array.array_iterator(global)?; let mut args: Vec = Vec::with_capacity(iter.len as usize); while let Some(arg_js) = iter.next()? { - let Some(v) = from_js(global, arg_js)? else { - return Err(global.throw_invalid_argument_type( - "sendCommand", - "argument", - "string or buffer", - )); - }; - args.push(v); + args.push(require_arg(global, arg_js, "send", "argument")?); } let cmd_str = command.to_utf8_without_ref(); - let mut cmd = Command { - command: cmd_str.slice(), - args: CommandArgs::Args(&args), - meta: CommandMeta::default(), - }; - // Note: reshaped for borrowck (cmd.meta = cmd.meta.check(&cmd)) - let checked_meta = cmd.meta.check(&cmd); - cmd.meta = checked_meta; - // Send command with slices directly - let promise = match this.send(global, frame.this(), &cmd) { - Ok(p) => p, - Err(err) => { - return send_err_to_js(global, "Failed to send command", &err); - } - }; - Ok(promise_to_js(promise)) - } - - #[bun_jsc::host_fn(method)] - pub fn get(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"get")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("get", "key", "string or buffer")); - }; send_cmd( this, global, - frame.this(), - b"GET", - CommandArgs::Args(&[key]), + cmd_str.slice(), + CommandArgs::Blobs(&args), CommandMeta::default(), - "Failed to send GET command", ) } - #[bun_jsc::host_fn(method)] - pub fn get_buffer( - this: &Self, - global: &JSGlobalObject, - frame: &CallFrame, - ) -> JsResult { - require_not_subscriber(this, b"getBuffer")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("getBuffer", "key", "string or buffer")); - }; - send_cmd( - this, - global, - frame.this(), - b"GET", - CommandArgs::Args(&[key]), - CommandMeta::RETURN_AS_BUFFER | CommandMeta::SUPPORTS_AUTO_PIPELINING, - "Failed to send GET command", - ) - } + cmd_key!(get, "get", "GET", "key", NotSubscriber); + cmd_key!( + get_buffer, + "getBuffer", + "GET", + "key", + NotSubscriber, + CommandMeta::RETURN_AS_BUFFER | CommandMeta::SUPPORTS_AUTO_PIPELINING + ); #[bun_jsc::host_fn(method)] pub fn set(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"set")?; + require_not_subscriber(this, "set")?; let args_view = frame.arguments(); let mut args: Vec = Vec::with_capacity(args_view.len()); - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("set", "key", "string or buffer")); - }; - args.push(key); + args.push(require_arg(global, frame.argument(0), "set", "key")?); - let Some(value) = from_js(global, frame.argument(1))? else { + let Some(value) = coerce_redis_arg(global, frame.argument(1))? else { return Err(global.throw_invalid_argument_type( "set", "value", @@ -541,94 +431,40 @@ impl JSValkeyClient { args.push(value); if args_view.len() > 2 { - for arg in &args_view[2..] { - if arg.is_undefined_or_null() { - break; - } - let Some(v) = from_js(global, *arg)? else { - return Err(global.throw_invalid_argument_type( - "set", - "arguments", - "string or buffer", - )); - }; - args.push(v); - } + args.extend(collect_varargs( + global, + &args_view[2..], + "set", + "arguments", + )?); } send_cmd( this, global, - frame.this(), b"SET", - CommandArgs::Args(&args), + CommandArgs::Blobs(&args), CommandMeta::default(), - "Failed to send SET command", - ) - } - - #[bun_jsc::host_fn(method)] - pub fn incr(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"incr")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("incr", "key", "string or buffer")); - }; - send_cmd( - this, - global, - frame.this(), - b"INCR", - CommandArgs::Args(&[key]), - CommandMeta::default(), - "Failed to send INCR command", - ) - } - - #[bun_jsc::host_fn(method)] - pub fn decr(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"decr")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("decr", "key", "string or buffer")); - }; - send_cmd( - this, - global, - frame.this(), - b"DECR", - CommandArgs::Args(&[key]), - CommandMeta::default(), - "Failed to send DECR command", - ) - } - - #[bun_jsc::host_fn(method)] - pub fn exists(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"exists")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("exists", "key", "string or buffer")); - }; - // Send EXISTS command with special Exists type for boolean conversion - send_cmd( - this, - global, - frame.this(), - b"EXISTS", - CommandArgs::Args(&[key]), - CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING, - "Failed to send EXISTS command", ) } + cmd_key!(incr, "incr", "INCR", "key", NotSubscriber); + cmd_key!(decr, "decr", "DECR", "key", NotSubscriber); + cmd_key!( + exists, + "exists", + "EXISTS", + "key", + NotSubscriber, + CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING + ); + // Hand-written (not `cmd_key_value!`) to keep the client-side + // `validate_integer_range` guard + default-0-when-undefined for `seconds`. #[bun_jsc::host_fn(method)] pub fn expire(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"expire")?; + require_not_subscriber(this, "expire")?; - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("expire", "key", "string or buffer")); - }; + let key = require_arg(global, frame.argument(0), "expire", "key")?; let seconds = global.validate_integer_range::( frame.argument(1), @@ -641,76 +477,44 @@ impl JSValkeyClient { }, )?; - // Convert seconds to a string let mut int_buf = bun_core::fmt::ItoaBuf::new(); let seconds_slice = bun_core::fmt::itoa(&mut int_buf, seconds); send_cmd( this, global, - frame.this(), b"EXPIRE", CommandArgs::Raw(&[key.slice(), seconds_slice]), CommandMeta::default(), - "Failed to send EXPIRE command", ) } - #[bun_jsc::host_fn(method)] - pub fn ttl(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"ttl")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("ttl", "key", "string or buffer")); - }; - send_cmd( - this, - global, - frame.this(), - b"TTL", - CommandArgs::Args(&[key]), - CommandMeta::default(), - "Failed to send TTL command", - ) - } + cmd_key!(ttl, "ttl", "TTL", "key", NotSubscriber); // Implement srem (remove value from a set) #[bun_jsc::host_fn(method)] pub fn srem(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"srem")?; + require_not_subscriber(this, "srem")?; let args_view = frame.arguments(); if args_view.len() < 2 { - return Err(global.throw(format_args!("SREM requires at least a key and one member"))); + return Err(global + .err( + ErrorCode::MISSING_ARGS, + format_args!("SREM requires at least a key and one member"), + ) + .throw()); } let mut args: Vec = Vec::with_capacity(args_view.len()); - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("srem", "key", "string or buffer")); - }; - args.push(key); - - for arg in &args_view[1..] { - if arg.is_undefined_or_null() { - break; - } - let Some(value) = from_js(global, *arg)? else { - return Err(global.throw_invalid_argument_type( - "srem", - "member", - "string or buffer", - )); - }; - args.push(value); - } + args.push(require_arg(global, frame.argument(0), "srem", "key")?); + args.extend(collect_varargs(global, &args_view[1..], "srem", "member")?); send_cmd( this, global, - frame.this(), b"SREM", - CommandArgs::Args(&args), + CommandArgs::Blobs(&args), CommandMeta::default(), - "Failed to send SREM command", ) } @@ -721,23 +525,21 @@ impl JSValkeyClient { global: &JSGlobalObject, frame: &CallFrame, ) -> JsResult { - require_not_subscriber(this, b"srandmember")?; + require_not_subscriber(this, "srandmember")?; let args_view = frame.arguments(); let mut args: Vec = Vec::with_capacity(args_view.len()); - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type( - "srandmember", - "key", - "string or buffer", - )); - }; - args.push(key); + args.push(require_arg( + global, + frame.argument(0), + "srandmember", + "key", + )?); // Optional count argument if args_view.len() > 1 && !frame.argument(1).is_undefined_or_null() { - let Some(count_arg) = from_js(global, frame.argument(1))? else { + let Some(count_arg) = coerce_redis_arg(global, frame.argument(1))? else { return Err(global.throw_invalid_argument_type( "srandmember", "count", @@ -749,49 +551,27 @@ impl JSValkeyClient { send_cmd( this, global, - frame.this(), b"SRANDMEMBER", - CommandArgs::Args(&args), + CommandArgs::Blobs(&args), CommandMeta::default(), - "Failed to send SRANDMEMBER command", ) } - // Implement smembers (get all members of a set) - #[bun_jsc::host_fn(method)] - pub fn smembers(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"smembers")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("smembers", "key", "string or buffer")); - }; - send_cmd( - this, - global, - frame.this(), - b"SMEMBERS", - CommandArgs::Args(&[key]), - CommandMeta::default(), - "Failed to send SMEMBERS command", - ) - } + cmd_key!(smembers, "smembers", "SMEMBERS", "key", NotSubscriber); // Implement spop (pop a random member from a set) #[bun_jsc::host_fn(method)] pub fn spop(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"spop")?; + require_not_subscriber(this, "spop")?; let args_view = frame.arguments(); let mut args: Vec = Vec::with_capacity(args_view.len()); - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("spop", "key", "string or buffer")); - }; - args.push(key); + args.push(require_arg(global, frame.argument(0), "spop", "key")?); // Optional count argument if args_view.len() > 1 && !frame.argument(1).is_undefined_or_null() { - let Some(count_arg) = from_js(global, frame.argument(1))? else { + let Some(count_arg) = coerce_redis_arg(global, frame.argument(1))? else { return Err(global.throw_invalid_argument_type( "spop", "count", @@ -803,202 +583,125 @@ impl JSValkeyClient { send_cmd( this, global, - frame.this(), b"SPOP", - CommandArgs::Args(&args), + CommandArgs::Blobs(&args), CommandMeta::default(), - "Failed to send SPOP command", ) } // Implement sadd (add member to a set) #[bun_jsc::host_fn(method)] pub fn sadd(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"sadd")?; + require_not_subscriber(this, "sadd")?; let args_view = frame.arguments(); if args_view.len() < 2 { - return Err(global.throw(format_args!("SADD requires at least a key and one member"))); + return Err(global + .err( + ErrorCode::MISSING_ARGS, + format_args!("SADD requires at least a key and one member"), + ) + .throw()); } let mut args: Vec = Vec::with_capacity(args_view.len()); - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("sadd", "key", "string or buffer")); - }; - args.push(key); - - for arg in &args_view[1..] { - if arg.is_undefined_or_null() { - break; - } - let Some(value) = from_js(global, *arg)? else { - return Err(global.throw_invalid_argument_type( - "sadd", - "member", - "string or buffer", - )); - }; - args.push(value); - } + args.push(require_arg(global, frame.argument(0), "sadd", "key")?); + args.extend(collect_varargs(global, &args_view[1..], "sadd", "member")?); send_cmd( this, global, - frame.this(), b"SADD", - CommandArgs::Args(&args), + CommandArgs::Blobs(&args), CommandMeta::default(), - "Failed to send SADD command", ) } - // Implement sismember (check if value is member of a set) - #[bun_jsc::host_fn(method)] - pub fn sismember(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"sismember")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("sismember", "key", "string or buffer")); - }; - let Some(value) = from_js(global, frame.argument(1))? else { - return Err(global.throw_invalid_argument_type( - "sismember", - "value", - "string or buffer", - )); - }; - send_cmd( - this, - global, - frame.this(), - b"SISMEMBER", - CommandArgs::Args(&[key, value]), - CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING, - "Failed to send SISMEMBER command", - ) - } + cmd_key_value!( + sismember, + "sismember", + "SISMEMBER", + "key", + "member", + NotSubscriber, + CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING + ); // Implement hmget (get multiple values from hash) #[bun_jsc::host_fn(method)] pub fn hmget(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"hmget")?; + require_not_subscriber(this, "hmget")?; let args_view = frame.arguments(); if args_view.len() < 2 { - return Err(global.throw(format_args!("HMGET requires at least a key and one field"))); + return Err(global + .err( + ErrorCode::MISSING_ARGS, + format_args!("HMGET requires at least a key and one field"), + ) + .throw()); } let mut args: Vec = Vec::with_capacity(args_view.len()); - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("hmget", "key", "string or buffer")); - }; - args.push(key); + args.push(require_arg(global, frame.argument(0), "hmget", "key")?); let second_arg = frame.argument(1); if second_arg.is_array() { let array_len = second_arg.get_length(global)?; if array_len == 0 { - return Err(global.throw(format_args!("HMGET requires at least one field"))); + return Err(global + .err( + ErrorCode::MISSING_ARGS, + format_args!("HMGET requires at least one field"), + ) + .throw()); } let mut array_iter = second_arg.array_iterator(global)?; while let Some(element) = array_iter.next()? { - let Some(field) = from_js(global, element)? else { - return Err(global.throw_invalid_argument_type( - "hmget", - "field", - "string or buffer", - )); - }; - args.push(field); + args.push(require_arg(global, element, "hmget", "field")?); } } else { - for arg in &args_view[1..] { - if arg.is_undefined_or_null() { - break; - } - let Some(field) = from_js(global, *arg)? else { - return Err(global.throw_invalid_argument_type( - "hmget", - "field", - "string or buffer", - )); - }; - args.push(field); - } + args.extend(collect_varargs(global, &args_view[1..], "hmget", "field")?); } send_cmd( this, global, - frame.this(), b"HMGET", - CommandArgs::Args(&args), + CommandArgs::Blobs(&args), CommandMeta::default(), - "Failed to send HMGET command", ) } - // Implement hincrby (increment hash field by integer value) - #[bun_jsc::host_fn(method)] - pub fn hincrby(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"hincrby")?; - - let key = OwnedString::new(frame.argument(0).to_bun_string(global)?); - let field = OwnedString::new(frame.argument(1).to_bun_string(global)?); - let value = OwnedString::new(frame.argument(2).to_bun_string(global)?); - - let key_slice = key.to_utf8_without_ref(); - let field_slice = field.to_utf8_without_ref(); - let value_slice = value.to_utf8_without_ref(); - - send_cmd( - this, - global, - frame.this(), - b"HINCRBY", - CommandArgs::Slices(&[key_slice, field_slice, value_slice]), - CommandMeta::default(), - "Failed to send HINCRBY command", - ) - } - - // Implement hincrbyfloat (increment hash field by float value) - #[bun_jsc::host_fn(method)] - pub fn hincrbyfloat( - this: &Self, - global: &JSGlobalObject, - frame: &CallFrame, - ) -> JsResult { - require_not_subscriber(this, b"hincrbyfloat")?; - - let key = OwnedString::new(frame.argument(0).to_bun_string(global)?); - let field = OwnedString::new(frame.argument(1).to_bun_string(global)?); - let value = OwnedString::new(frame.argument(2).to_bun_string(global)?); - - let key_slice = key.to_utf8_without_ref(); - let field_slice = field.to_utf8_without_ref(); - let value_slice = value.to_utf8_without_ref(); - - send_cmd( - this, - global, - frame.this(), - b"HINCRBYFLOAT", - CommandArgs::Slices(&[key_slice, field_slice, value_slice]), - CommandMeta::default(), - "Failed to send HINCRBYFLOAT command", - ) - } + cmd_key_value_value2!( + hincrby, + "hincrby", + "HINCRBY", + "key", + "field", + "increment", + NotSubscriber + ); + cmd_key_value_value2!( + hincrbyfloat, + "hincrbyfloat", + "HINCRBYFLOAT", + "key", + "field", + "increment", + NotSubscriber + ); fn hset_impl( this: &Self, global: &JSGlobalObject, frame: &CallFrame, command: &'static [u8], + js_name: &'static str, ) -> JsResult { - require_not_subscriber(this, command)?; + require_not_subscriber(this, js_name)?; let key = OwnedString::new(frame.argument(0).to_bun_string(global)?); @@ -1011,7 +714,7 @@ impl JSValkeyClient { if second_arg.is_object() && !second_arg.is_array() { // Pattern 1: Object/Record - hset(key, {field: value, ...}) let Some(obj) = second_arg.get_object() else { - return Err(global.throw_invalid_argument_type(bname(command), "fields", "object")); + return Err(global.throw_invalid_argument_type(js_name, "fields", "object")); }; let mut object_iter = JSPropertyIterator::init( @@ -1029,17 +732,13 @@ impl JSValkeyClient { let field_slice = field_name.to_utf8(); args.push(field_slice); - let value_str = object_iter.value.to_bun_string(global)?; - args.push(value_str.to_utf8()); - // `to_utf8()` already bumped - // (or copied) the ref the slice needs, so release ours now. - value_str.deref(); + args.push(object_iter.value.to_slice(global)?); } } else if second_arg.is_array() { // Pattern 3: Array - hmset(key, [field, value, ...]) let mut iter = second_arg.array_iterator(global)?; if iter.len % 2 != 0 { - return Err(global.throw(format_args!( + return Err(global.throw_invalid_arguments(format_args!( "Array must have an even number of elements (field-value pairs)" ))); } @@ -1047,32 +746,35 @@ impl JSValkeyClient { args.ensure_total_capacity(1 + iter.len as usize); while let Some(field_js) = iter.next()? { - let field_str = field_js.to_bun_string(global)?; - args.push(field_str.to_utf8()); - field_str.deref(); + args.push(field_js.to_slice(global)?); let Some(value_js) = iter.next()? else { - return Err(global.throw(format_args!( + return Err(global.throw_invalid_arguments(format_args!( "Array must have an even number of elements (field-value pairs)" ))); }; - let value_str = value_js.to_bun_string(global)?; - args.push(value_str.to_utf8()); - value_str.deref(); + args.push(value_js.to_slice(global)?); } } else { // Pattern 2: Variadic - hset(key, field, value, ...) let args_count = frame.arguments_count(); if args_count < 3 { - return Err(global.throw(format_args!( - "HSET requires at least key, field, and value arguments" - ))); + return Err(global + .err( + ErrorCode::MISSING_ARGS, + format_args!( + "{} requires at least key, field, and value arguments", + bstr::BStr::new(command) + ), + ) + .throw()); } let field_value_count = args_count - 1; // Exclude key if !field_value_count.is_multiple_of(2) { - return Err(global.throw(format_args!( - "HSET requires field-value pairs (even number of arguments after key)" + return Err(global.throw_invalid_arguments(format_args!( + "{} requires field-value pairs (even number of arguments after key)", + bstr::BStr::new(command) ))); } @@ -1080,124 +782,84 @@ impl JSValkeyClient { let mut i: u32 = 1; while i < args_count { - let arg_str = frame.argument(i as usize).to_bun_string(global)?; - args.push(arg_str.to_utf8()); - arg_str.deref(); + args.push(frame.argument(i as usize).to_slice(global)?); i += 1; } } if args.len() == 1 { - return Err(global.throw(format_args!("HSET requires at least one field-value pair"))); + return Err(global + .err( + ErrorCode::MISSING_ARGS, + format_args!( + "{} requires at least one field-value pair", + bstr::BStr::new(command) + ), + ) + .throw()); } - let msg = if command == b"HSET" { - "Failed to send HSET command" - } else { - "Failed to send HMSET command" - }; send_cmd( this, global, - frame.this(), command, CommandArgs::Slices(&args), CommandMeta::default(), - msg, ) } #[bun_jsc::host_fn(method)] pub fn hset(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - Self::hset_impl(this, global, frame, b"HSET") + Self::hset_impl(this, global, frame, b"HSET", "hset") } #[bun_jsc::host_fn(method)] pub fn hmset(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - Self::hset_impl(this, global, frame, b"HMSET") + Self::hset_impl(this, global, frame, b"HMSET", "hmset") } - cmd_key_varargs!(hdel, b"hdel", "HDEL", "key", NotSubscriber); - cmd_key_varargs!( - hrandfield, - b"hrandfield", - "HRANDFIELD", + cmd_key_varargs!(hdel, "hdel", "HDEL", "key", NotSubscriber); + cmd_key_varargs!(hrandfield, "hrandfield", "HRANDFIELD", "key", NotSubscriber); + cmd_key_varargs!(hscan, "hscan", "HSCAN", "key", NotSubscriber); + cmd_strings_varargs!(hgetdel, "hgetdel", "HGETDEL", NotSubscriber); + cmd_strings_varargs!(hgetex, "hgetex", "HGETEX", NotSubscriber); + cmd_strings_varargs!(hsetex, "hsetex", "HSETEX", NotSubscriber); + cmd_strings_varargs!(hexpire, "hexpire", "HEXPIRE", NotSubscriber); + cmd_strings_varargs!(hexpireat, "hexpireat", "HEXPIREAT", NotSubscriber); + cmd_strings_varargs!(hexpiretime, "hexpiretime", "HEXPIRETIME", NotSubscriber); + cmd_strings_varargs!(hpersist, "hpersist", "HPERSIST", NotSubscriber); + cmd_strings_varargs!(hpexpire, "hpexpire", "HPEXPIRE", NotSubscriber); + cmd_strings_varargs!(hpexpireat, "hpexpireat", "HPEXPIREAT", NotSubscriber); + cmd_strings_varargs!(hpexpiretime, "hpexpiretime", "HPEXPIRETIME", NotSubscriber); + cmd_strings_varargs!(hpttl, "hpttl", "HPTTL", NotSubscriber); + cmd_strings_varargs!(httl, "httl", "HTTL", NotSubscriber); + + cmd_key_value_value2!( + hsetnx, + "hsetnx", + "HSETNX", "key", - NotSubscriber + "field", + "value", + NotSubscriber, + CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING + ); + cmd_key_value!( + hexists, + "hexists", + "HEXISTS", + "key", + "field", + NotSubscriber, + CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING ); - cmd_key_varargs!(hscan, b"hscan", "HSCAN", "key", NotSubscriber); - cmd_strings_varargs!(hgetdel, b"hgetdel", "HGETDEL", NotSubscriber); - cmd_strings_varargs!(hgetex, b"hgetex", "HGETEX", NotSubscriber); - cmd_strings_varargs!(hsetex, b"hsetex", "HSETEX", NotSubscriber); - cmd_strings_varargs!(hexpire, b"hexpire", "HEXPIRE", NotSubscriber); - cmd_strings_varargs!(hexpireat, b"hexpireat", "HEXPIREAT", NotSubscriber); - cmd_strings_varargs!(hexpiretime, b"hexpiretime", "HEXPIRETIME", NotSubscriber); - cmd_strings_varargs!(hpersist, b"hpersist", "HPERSIST", NotSubscriber); - cmd_strings_varargs!(hpexpire, b"hpexpire", "HPEXPIRE", NotSubscriber); - cmd_strings_varargs!(hpexpireat, b"hpexpireat", "HPEXPIREAT", NotSubscriber); - cmd_strings_varargs!(hpexpiretime, b"hpexpiretime", "HPEXPIRETIME", NotSubscriber); - cmd_strings_varargs!(hpttl, b"hpttl", "HPTTL", NotSubscriber); - cmd_strings_varargs!(httl, b"httl", "HTTL", NotSubscriber); - - #[bun_jsc::host_fn(method)] - pub fn hsetnx(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"hsetnx")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("hsetnx", "key", "string or buffer")); - }; - let Some(field) = from_js(global, frame.argument(1))? else { - return Err(global.throw_invalid_argument_type("hsetnx", "field", "string or buffer")); - }; - let Some(value) = from_js(global, frame.argument(2))? else { - return Err(global.throw_invalid_argument_type("hsetnx", "value", "string or buffer")); - }; - send_cmd( - this, - global, - frame.this(), - b"HSETNX", - CommandArgs::Args(&[key, field, value]), - CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING, - "Failed to send HSETNX command", - ) - } - - #[bun_jsc::host_fn(method)] - pub fn hexists(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"hexists")?; - - let Some(key) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("hexists", "key", "string or buffer")); - }; - - let Some(field) = from_js(global, frame.argument(1))? else { - return Err(global.throw_invalid_argument_type("hexists", "field", "string or buffer")); - }; - send_cmd( - this, - global, - frame.this(), - b"HEXISTS", - CommandArgs::Args(&[key, field]), - CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING, - "Failed to send HEXISTS command", - ) - } // Implement ping (send a PING command with an optional message) #[bun_jsc::host_fn(method)] pub fn ping(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { let message: Option = if !frame.argument(0).is_undefined_or_null() { // Only use the first argument if provided, ignore any additional arguments - let Some(m) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type( - "ping", - "message", - "string or buffer", - )); - }; - Some(m) + Some(require_arg(global, frame.argument(0), "ping", "message")?) } else { None }; @@ -1208,32 +870,30 @@ impl JSValkeyClient { send_cmd( this, global, - frame.this(), b"PING", - CommandArgs::Args(args_slice), + CommandArgs::Blobs(args_slice), CommandMeta::default(), - "Failed to send PING command", ) } - cmd_key!(bitcount, b"bitcount", "BITCOUNT", "key", NotSubscriber); - cmd_strings_varargs!(blmove, b"blmove", "BLMOVE", NotSubscriber); - cmd_strings_varargs!(blmpop, b"blmpop", "BLMPOP", NotSubscriber); - cmd_strings_varargs!(blpop, b"blpop", "BLPOP", NotSubscriber); - cmd_strings_varargs!(brpop, b"brpop", "BRPOP", NotSubscriber); + cmd_key!(bitcount, "bitcount", "BITCOUNT", "key", NotSubscriber); + cmd_strings_varargs!(blmove, "blmove", "BLMOVE", NotSubscriber); + cmd_strings_varargs!(blmpop, "blmpop", "BLMPOP", NotSubscriber); + cmd_strings_varargs!(blpop, "blpop", "BLPOP", NotSubscriber); + cmd_strings_varargs!(brpop, "brpop", "BRPOP", NotSubscriber); cmd_key_value_value2!( brpoplpush, - b"brpoplpush", + "brpoplpush", "BRPOPLPUSH", "source", "destination", "timeout", NotSubscriber ); - cmd_key_value!(getbit, b"getbit", "GETBIT", "key", "offset", NotSubscriber); + cmd_key_value!(getbit, "getbit", "GETBIT", "key", "offset", NotSubscriber); cmd_key_value_value2!( setbit, - b"setbit", + "setbit", "SETBIT", "key", "offset", @@ -1242,7 +902,7 @@ impl JSValkeyClient { ); cmd_key_value_value2!( getrange, - b"getrange", + "getrange", "GETRANGE", "key", "start", @@ -1251,46 +911,40 @@ impl JSValkeyClient { ); cmd_key_value_value2!( setrange, - b"setrange", + "setrange", "SETRANGE", "key", "offset", "value", NotSubscriber ); - cmd_key!(dump, b"dump", "DUMP", "key", NotSubscriber); + cmd_key!(dump, "dump", "DUMP", "key", NotSubscriber); cmd_key_value!( expireat, - b"expireat", + "expireat", "EXPIREAT", "key", "timestamp", NotSubscriber ); - cmd_key!( - expiretime, - b"expiretime", - "EXPIRETIME", - "key", - NotSubscriber - ); - cmd_key!(getdel, b"getdel", "GETDEL", "key", NotSubscriber); - cmd_strings_varargs!(getex, b"getex", "GETEX", NotSubscriber); - cmd_key!(hgetall, b"hgetall", "HGETALL", "key", NotSubscriber); - cmd_key!(hkeys, b"hkeys", "HKEYS", "key", NotSubscriber); - cmd_key!(hlen, b"hlen", "HLEN", "key", NotSubscriber); - cmd_key!(hvals, b"hvals", "HVALS", "key", NotSubscriber); - cmd_key!(keys, b"keys", "KEYS", "key", NotSubscriber); - cmd_key_value!(lindex, b"lindex", "LINDEX", "key", "index", NotSubscriber); - cmd_strings_varargs!(linsert, b"linsert", "LINSERT", NotSubscriber); - cmd_key!(llen, b"llen", "LLEN", "key", NotSubscriber); - cmd_strings_varargs!(lmove, b"lmove", "LMOVE", NotSubscriber); - cmd_strings_varargs!(lmpop, b"lmpop", "LMPOP", NotSubscriber); - cmd_key_varargs!(lpop, b"lpop", "LPOP", "key", NotSubscriber); - cmd_strings_varargs!(lpos, b"lpos", "LPOS", NotSubscriber); + cmd_key!(expiretime, "expiretime", "EXPIRETIME", "key", NotSubscriber); + cmd_key!(getdel, "getdel", "GETDEL", "key", NotSubscriber); + cmd_strings_varargs!(getex, "getex", "GETEX", NotSubscriber); + cmd_key!(hgetall, "hgetall", "HGETALL", "key", NotSubscriber); + cmd_key!(hkeys, "hkeys", "HKEYS", "key", NotSubscriber); + cmd_key!(hlen, "hlen", "HLEN", "key", NotSubscriber); + cmd_key!(hvals, "hvals", "HVALS", "key", NotSubscriber); + cmd_key!(keys, "keys", "KEYS", "key", NotSubscriber); + cmd_key_value!(lindex, "lindex", "LINDEX", "key", "index", NotSubscriber); + cmd_strings_varargs!(linsert, "linsert", "LINSERT", NotSubscriber); + cmd_key!(llen, "llen", "LLEN", "key", NotSubscriber); + cmd_strings_varargs!(lmove, "lmove", "LMOVE", NotSubscriber); + cmd_strings_varargs!(lmpop, "lmpop", "LMPOP", NotSubscriber); + cmd_key_varargs!(lpop, "lpop", "LPOP", "key", NotSubscriber); + cmd_strings_varargs!(lpos, "lpos", "LPOS", NotSubscriber); cmd_key_value_value2!( lrange, - b"lrange", + "lrange", "LRANGE", "key", "start", @@ -1299,7 +953,7 @@ impl JSValkeyClient { ); cmd_key_value_value2!( lrem, - b"lrem", + "lrem", "LREM", "key", "count", @@ -1308,7 +962,7 @@ impl JSValkeyClient { ); cmd_key_value_value2!( lset, - b"lset", + "lset", "LSET", "key", "index", @@ -1317,17 +971,17 @@ impl JSValkeyClient { ); cmd_key_value_value2!( ltrim, - b"ltrim", + "ltrim", "LTRIM", "key", "start", "stop", NotSubscriber ); - cmd_key!(persist, b"persist", "PERSIST", "key", NotSubscriber); + cmd_key!(persist, "persist", "PERSIST", "key", NotSubscriber); cmd_key_value!( pexpire, - b"pexpire", + "pexpire", "PEXPIRE", "key", "milliseconds", @@ -1335,7 +989,7 @@ impl JSValkeyClient { ); cmd_key_value!( pexpireat, - b"pexpireat", + "pexpireat", "PEXPIREAT", "key", "milliseconds-timestamp", @@ -1343,39 +997,39 @@ impl JSValkeyClient { ); cmd_key!( pexpiretime, - b"pexpiretime", + "pexpiretime", "PEXPIRETIME", "key", NotSubscriber ); - cmd_key!(pttl, b"pttl", "PTTL", "key", NotSubscriber); - cmd_noargs!(randomkey, b"randomkey", "RANDOMKEY", NotSubscriber); - cmd_key_varargs!(rpop, b"rpop", "RPOP", "key", NotSubscriber); + cmd_key!(pttl, "pttl", "PTTL", "key", NotSubscriber); + cmd_noargs!(randomkey, "randomkey", "RANDOMKEY", NotSubscriber); + cmd_key_varargs!(rpop, "rpop", "RPOP", "key", NotSubscriber); cmd_key_value!( rpoplpush, - b"rpoplpush", + "rpoplpush", "RPOPLPUSH", "source", "destination", NotSubscriber ); - cmd_strings_varargs!(scan, b"scan", "SCAN", NotSubscriber); - cmd_key!(scard, b"scard", "SCARD", "key", NotSubscriber); - cmd_strings_varargs!(sdiff, b"sdiff", "SDIFF", NotSubscriber); - cmd_strings_varargs!(sdiffstore, b"sdiffstore", "SDIFFSTORE", NotSubscriber); - cmd_strings_varargs!(sinter, b"sinter", "SINTER", NotSubscriber); - cmd_strings_varargs!(sintercard, b"sintercard", "SINTERCARD", NotSubscriber); - cmd_strings_varargs!(sinterstore, b"sinterstore", "SINTERSTORE", NotSubscriber); - cmd_strings_varargs!(smismember, b"smismember", "SMISMEMBER", NotSubscriber); - cmd_strings_varargs!(sscan, b"sscan", "SSCAN", NotSubscriber); - cmd_key!(strlen, b"strlen", "STRLEN", "key", NotSubscriber); - cmd_strings_varargs!(sunion, b"sunion", "SUNION", NotSubscriber); - cmd_strings_varargs!(sunionstore, b"sunionstore", "SUNIONSTORE", NotSubscriber); - cmd_key!(r#type, b"type", "TYPE", "key", NotSubscriber); - cmd_key!(zcard, b"zcard", "ZCARD", "key", NotSubscriber); + cmd_strings_varargs!(scan, "scan", "SCAN", NotSubscriber); + cmd_key!(scard, "scard", "SCARD", "key", NotSubscriber); + cmd_strings_varargs!(sdiff, "sdiff", "SDIFF", NotSubscriber); + cmd_strings_varargs!(sdiffstore, "sdiffstore", "SDIFFSTORE", NotSubscriber); + cmd_strings_varargs!(sinter, "sinter", "SINTER", NotSubscriber); + cmd_strings_varargs!(sintercard, "sintercard", "SINTERCARD", NotSubscriber); + cmd_strings_varargs!(sinterstore, "sinterstore", "SINTERSTORE", NotSubscriber); + cmd_strings_varargs!(smismember, "smismember", "SMISMEMBER", NotSubscriber); + cmd_strings_varargs!(sscan, "sscan", "SSCAN", NotSubscriber); + cmd_key!(strlen, "strlen", "STRLEN", "key", NotSubscriber); + cmd_strings_varargs!(sunion, "sunion", "SUNION", NotSubscriber); + cmd_strings_varargs!(sunionstore, "sunionstore", "SUNIONSTORE", NotSubscriber); + cmd_key!(r#type, "type", "TYPE", "key", NotSubscriber); + cmd_key!(zcard, "zcard", "ZCARD", "key", NotSubscriber); cmd_key_value_value2!( zcount, - b"zcount", + "zcount", "ZCOUNT", "key", "min", @@ -1384,56 +1038,56 @@ impl JSValkeyClient { ); cmd_key_value_value2!( zlexcount, - b"zlexcount", + "zlexcount", "ZLEXCOUNT", "key", "min", "max", NotSubscriber ); - cmd_key_varargs!(zpopmax, b"zpopmax", "ZPOPMAX", "key", NotSubscriber); - cmd_key_varargs!(zpopmin, b"zpopmin", "ZPOPMIN", "key", NotSubscriber); + cmd_key_varargs!(zpopmax, "zpopmax", "ZPOPMAX", "key", NotSubscriber); + cmd_key_varargs!(zpopmin, "zpopmin", "ZPOPMIN", "key", NotSubscriber); cmd_key_varargs!( zrandmember, - b"zrandmember", + "zrandmember", "ZRANDMEMBER", "key", NotSubscriber ); - cmd_strings_varargs!(zrange, b"zrange", "ZRANGE", NotSubscriber); - cmd_strings_varargs!(zrevrange, b"zrevrange", "ZREVRANGE", NotSubscriber); + cmd_strings_varargs!(zrange, "zrange", "ZRANGE", NotSubscriber); + cmd_strings_varargs!(zrevrange, "zrevrange", "ZREVRANGE", NotSubscriber); cmd_strings_varargs!( zrangebyscore, - b"zrangebyscore", + "zrangebyscore", "ZRANGEBYSCORE", NotSubscriber ); cmd_strings_varargs!( zrevrangebyscore, - b"zrevrangebyscore", + "zrevrangebyscore", "ZREVRANGEBYSCORE", NotSubscriber ); cmd_key_varargs!( zrangebylex, - b"zrangebylex", + "zrangebylex", "ZRANGEBYLEX", "key", NotSubscriber ); cmd_key_varargs!( zrevrangebylex, - b"zrevrangebylex", + "zrevrangebylex", "ZREVRANGEBYLEX", "key", NotSubscriber ); - cmd_key_value!(append, b"append", "APPEND", "key", "value", NotSubscriber); - cmd_key_value!(getset, b"getset", "GETSET", "key", "value", NotSubscriber); - cmd_key_value!(hget, b"hget", "HGET", "key", "field", NotSubscriber); + cmd_key_value!(append, "append", "APPEND", "key", "value", NotSubscriber); + cmd_key_value!(getset, "getset", "GETSET", "key", "value", NotSubscriber); + cmd_key_value!(hget, "hget", "HGET", "key", "field", NotSubscriber); cmd_key_value!( incrby, - b"incrby", + "incrby", "INCRBY", "key", "increment", @@ -1441,7 +1095,7 @@ impl JSValkeyClient { ); cmd_key_value!( incrbyfloat, - b"incrbyfloat", + "incrbyfloat", "INCRBYFLOAT", "key", "increment", @@ -1449,21 +1103,21 @@ impl JSValkeyClient { ); cmd_key_value!( decrby, - b"decrby", + "decrby", "DECRBY", "key", "decrement", NotSubscriber ); - cmd_key_value_varargs!(lpush, b"lpush", "LPUSH", NotSubscriber); - cmd_key_value_varargs!(lpushx, b"lpushx", "LPUSHX", NotSubscriber); - cmd_key_value!(pfadd, b"pfadd", "PFADD", "key", "value", NotSubscriber); - cmd_key_value_varargs!(rpush, b"rpush", "RPUSH", NotSubscriber); - cmd_key_value_varargs!(rpushx, b"rpushx", "RPUSHX", NotSubscriber); - cmd_key_value!(setnx, b"setnx", "SETNX", "key", "value", NotSubscriber); + cmd_strings_varargs!(lpush, "lpush", "LPUSH", NotSubscriber); + cmd_strings_varargs!(lpushx, "lpushx", "LPUSHX", NotSubscriber); + cmd_key_value!(pfadd, "pfadd", "PFADD", "key", "value", NotSubscriber); + cmd_strings_varargs!(rpush, "rpush", "RPUSH", NotSubscriber); + cmd_strings_varargs!(rpushx, "rpushx", "RPUSHX", NotSubscriber); + cmd_key_value!(setnx, "setnx", "SETNX", "key", "value", NotSubscriber); cmd_key_value_value2!( setex, - b"setex", + "setex", "SETEX", "key", "seconds", @@ -1472,103 +1126,79 @@ impl JSValkeyClient { ); cmd_key_value_value2!( psetex, - b"psetex", + "psetex", "PSETEX", "key", "milliseconds", "value", NotSubscriber ); - cmd_key_value!(zscore, b"zscore", "ZSCORE", "key", "value", NotSubscriber); + cmd_key_value!(zscore, "zscore", "ZSCORE", "key", "value", NotSubscriber); cmd_key_value_value2!( zincrby, - b"zincrby", + "zincrby", "ZINCRBY", "key", "increment", "member", NotSubscriber ); - cmd_key_value_varargs!(zmscore, b"zmscore", "ZMSCORE", NotSubscriber); - cmd_strings_varargs!(zadd, b"zadd", "ZADD", NotSubscriber); - cmd_strings_varargs!(zscan, b"zscan", "ZSCAN", NotSubscriber); - cmd_strings_varargs!(zdiff, b"zdiff", "ZDIFF", NotSubscriber); - cmd_strings_varargs!(zdiffstore, b"zdiffstore", "ZDIFFSTORE", NotSubscriber); - cmd_strings_varargs!(zinter, b"zinter", "ZINTER", NotSubscriber); - cmd_strings_varargs!(zintercard, b"zintercard", "ZINTERCARD", NotSubscriber); - cmd_strings_varargs!(zinterstore, b"zinterstore", "ZINTERSTORE", NotSubscriber); - cmd_strings_varargs!(zunion, b"zunion", "ZUNION", NotSubscriber); - cmd_strings_varargs!(zunionstore, b"zunionstore", "ZUNIONSTORE", NotSubscriber); - cmd_strings_varargs!(zmpop, b"zmpop", "ZMPOP", NotSubscriber); - cmd_strings_varargs!(bzmpop, b"bzmpop", "BZMPOP", NotSubscriber); - cmd_strings_varargs!(bzpopmin, b"bzpopmin", "BZPOPMIN", NotSubscriber); - cmd_strings_varargs!(bzpopmax, b"bzpopmax", "BZPOPMAX", NotSubscriber); - cmd_key_varargs!(del, b"del", "DEL", "key", NotSubscriber); - cmd_key_varargs!(mget, b"mget", "MGET", "key", NotSubscriber); - cmd_strings_varargs!(mset, b"mset", "MSET", NotSubscriber); - cmd_strings_varargs!(msetnx, b"msetnx", "MSETNX", NotSubscriber); - cmd_strings_varargs!(script, b"script", "SCRIPT", NotSubscriber); - cmd_strings_varargs!(select, b"select", "SELECT", NotSubscriber); + cmd_strings_varargs!(zmscore, "zmscore", "ZMSCORE", NotSubscriber); + cmd_strings_varargs!(zadd, "zadd", "ZADD", NotSubscriber); + cmd_strings_varargs!(zscan, "zscan", "ZSCAN", NotSubscriber); + cmd_strings_varargs!(zdiff, "zdiff", "ZDIFF", NotSubscriber); + cmd_strings_varargs!(zdiffstore, "zdiffstore", "ZDIFFSTORE", NotSubscriber); + cmd_strings_varargs!(zinter, "zinter", "ZINTER", NotSubscriber); + cmd_strings_varargs!(zintercard, "zintercard", "ZINTERCARD", NotSubscriber); + cmd_strings_varargs!(zinterstore, "zinterstore", "ZINTERSTORE", NotSubscriber); + cmd_strings_varargs!(zunion, "zunion", "ZUNION", NotSubscriber); + cmd_strings_varargs!(zunionstore, "zunionstore", "ZUNIONSTORE", NotSubscriber); + cmd_strings_varargs!(zmpop, "zmpop", "ZMPOP", NotSubscriber); + cmd_strings_varargs!(bzmpop, "bzmpop", "BZMPOP", NotSubscriber); + cmd_strings_varargs!(bzpopmin, "bzpopmin", "BZPOPMIN", NotSubscriber); + cmd_strings_varargs!(bzpopmax, "bzpopmax", "BZPOPMAX", NotSubscriber); + cmd_key_varargs!(del, "del", "DEL", "key", NotSubscriber); + cmd_key_varargs!(mget, "mget", "MGET", "key", NotSubscriber); + cmd_strings_varargs!(mset, "mset", "MSET", NotSubscriber); + cmd_strings_varargs!(msetnx, "msetnx", "MSETNX", NotSubscriber); + cmd_strings_varargs!(script, "script", "SCRIPT", NotSubscriber); + cmd_strings_varargs!(select, "select", "SELECT", NotSubscriber); cmd_key_value!( spublish, - b"spublish", + "spublish", "SPUBLISH", "channel", "message", NotSubscriber ); - #[bun_jsc::host_fn(method)] - pub fn smove(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"smove")?; - - let Some(source) = from_js(global, frame.argument(0))? else { - return Err(global.throw_invalid_argument_type("smove", "source", "string or buffer")); - }; - let Some(destination) = from_js(global, frame.argument(1))? else { - return Err(global.throw_invalid_argument_type( - "smove", - "destination", - "string or buffer", - )); - }; - let Some(member) = from_js(global, frame.argument(2))? else { - return Err(global.throw_invalid_argument_type("smove", "member", "string or buffer")); - }; - send_cmd( - this, - global, - frame.this(), - b"SMOVE", - CommandArgs::Args(&[source, destination, member]), - CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING, - "Failed to send SMOVE command", - ) - } + cmd_key_value_value2!( + smove, + "smove", + "SMOVE", + "source", + "destination", + "member", + NotSubscriber, + CommandMeta::RETURN_AS_BOOL | CommandMeta::SUPPORTS_AUTO_PIPELINING + ); cmd_key_value_value2!( substr, - b"substr", + "substr", "SUBSTR", "key", "start", "end", NotSubscriber ); - cmd_key_value!( - hstrlen, - b"hstrlen", - "HSTRLEN", - "key", - "field", - NotSubscriber - ); - cmd_key_varargs!(zrank, b"zrank", "ZRANK", "key", NotSubscriber); - cmd_strings_varargs!(zrangestore, b"zrangestore", "ZRANGESTORE", NotSubscriber); - cmd_key_varargs!(zrem, b"zrem", "ZREM", "key", NotSubscriber); + cmd_key_value!(hstrlen, "hstrlen", "HSTRLEN", "key", "field", NotSubscriber); + cmd_key_varargs!(zrank, "zrank", "ZRANK", "key", NotSubscriber); + cmd_strings_varargs!(zrangestore, "zrangestore", "ZRANGESTORE", NotSubscriber); + cmd_key_varargs!(zrem, "zrem", "ZREM", "key", NotSubscriber); cmd_key_value_value2!( zremrangebylex, - b"zremrangebylex", + "zremrangebylex", "ZREMRANGEBYLEX", "key", "min", @@ -1577,7 +1207,7 @@ impl JSValkeyClient { ); cmd_key_value_value2!( zremrangebyrank, - b"zremrangebyrank", + "zremrangebyrank", "ZREMRANGEBYRANK", "key", "start", @@ -1586,61 +1216,38 @@ impl JSValkeyClient { ); cmd_key_value_value2!( zremrangebyscore, - b"zremrangebyscore", + "zremrangebyscore", "ZREMRANGEBYSCORE", "key", "min", "max", NotSubscriber ); - cmd_key_varargs!(zrevrank, b"zrevrank", "ZREVRANK", "key", NotSubscriber); - cmd_strings_varargs!(psubscribe, b"psubscribe", "PSUBSCRIBE", DontCare); - cmd_strings_varargs!(punsubscribe, b"punsubscribe", "PUNSUBSCRIBE", DontCare); - cmd_strings_varargs!(pubsub, b"pubsub", "PUBSUB", DontCare); - cmd_strings_varargs!(copy, b"copy", "COPY", NotSubscriber); - cmd_key_varargs!(unlink, b"unlink", "UNLINK", "key", NotSubscriber); - cmd_key_varargs!(touch, b"touch", "TOUCH", "key", NotSubscriber); - cmd_key_value!(rename, b"rename", "RENAME", "key", "newkey", NotSubscriber); + cmd_key_varargs!(zrevrank, "zrevrank", "ZREVRANK", "key", NotSubscriber); + cmd_strings_varargs!(psubscribe, "psubscribe", "PSUBSCRIBE", DontCare); + cmd_strings_varargs!(punsubscribe, "punsubscribe", "PUNSUBSCRIBE", DontCare); + cmd_strings_varargs!(pubsub, "pubsub", "PUBSUB", DontCare); + cmd_strings_varargs!(copy, "copy", "COPY", NotSubscriber); + cmd_key_varargs!(unlink, "unlink", "UNLINK", "key", NotSubscriber); + cmd_key_varargs!(touch, "touch", "TOUCH", "key", NotSubscriber); + cmd_key_value!(rename, "rename", "RENAME", "key", "newkey", NotSubscriber); cmd_key_value!( renamenx, - b"renamenx", + "renamenx", "RENAMENX", "key", "newkey", NotSubscriber ); - #[bun_jsc::host_fn(method)] - pub fn publish(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - require_not_subscriber(this, b"publish")?; - - let args_view = frame.arguments(); - let mut args: Vec = Vec::with_capacity(args_view.len()); - - let arg0 = frame.argument(0); - if !arg0.is_string() { - return Err(global.throw_invalid_argument_type("publish", "channel", "string")); - } - let channel = from_js(global, arg0)?.expect("unreachable"); - - args.push(channel); - - let arg1 = frame.argument(1); - if !arg1.is_string() { - return Err(global.throw_invalid_argument_type("publish", "message", "string")); - } - let message = from_js(global, arg1)?.expect("unreachable"); - args.push(message); - send_cmd( - this, - global, - frame.this(), - b"PUBLISH", - CommandArgs::Args(&args), - CommandMeta::default(), - "Failed to send PUBLISH command", - ) - } + cmd_key_value!( + publish, + "publish", + "PUBLISH", + "channel", + "message", + NotSubscriber + ); #[bun_jsc::host_fn(method)] pub fn subscribe(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult { @@ -1651,6 +1258,7 @@ impl JSValkeyClient { let [channel_or_many, handler_callback] = frame.arguments_as_array::<2>(); let mut redis_channels: Vec = Vec::with_capacity(1); + let mut inserted_channels: Vec = Vec::with_capacity(1); if !handler_callback.is_callable() { return Err(global.throw_invalid_argument_type("subscribe", "listener", "function")); @@ -1658,16 +1266,20 @@ impl JSValkeyClient { // The first argument given is the channel or may be an array of channels. if channel_or_many.is_array() { - if channel_or_many.get_length(global)? == 0 { - return Err(global.throw_invalid_arguments(format_args!( - "subscribe requires at least one channel" - ))); + let len = channel_or_many.get_length(global)?; + if len == 0 { + return Err(global + .err( + ErrorCode::MISSING_ARGS, + format_args!("subscribe requires at least one channel"), + ) + .throw()); } - redis_channels.ensure_total_capacity(channel_or_many.get_length(global)? as usize); + redis_channels.ensure_total_capacity(len as usize); let mut array_iter = channel_or_many.array_iterator(global)?; while let Some(channel_arg) = array_iter.next()? { - let Some(channel) = from_js(global, channel_arg)? else { + let Some(channel) = coerce_redis_arg(global, channel_arg)? else { return Err(global.throw_invalid_argument_type( "subscribe", "channel", @@ -1678,28 +1290,26 @@ impl JSValkeyClient { // What we do here is add our receive handler. Notice that this doesn't really do anything until the // "SUBSCRIBE" command is sent to redis and we get a response. - // - // This is less-than-ideal, still, because this assumes a happy path. What happens if - // the SUBSCRIBE command fails? We have no way to roll back the addition of the - // handler. - this._subscription_ctx.get().upsert_receive_handler( + this.subscription_ctx.get().upsert_receive_handler( global, channel_arg, handler_callback, )?; + inserted_channels.push(channel_arg); } } else if channel_or_many.is_string() { // It is a single string channel - let Some(channel) = from_js(global, channel_or_many)? else { + let Some(channel) = coerce_redis_arg(global, channel_or_many)? else { return Err(global.throw_invalid_argument_type("subscribe", "channel", "string")); }; redis_channels.push(channel); - this._subscription_ctx.get().upsert_receive_handler( + this.subscription_ctx.get().upsert_receive_handler( global, channel_or_many, handler_callback, )?; + inserted_channels.push(channel_or_many); } else { return Err(global.throw_invalid_argument_type( "subscribe", @@ -1710,43 +1320,26 @@ impl JSValkeyClient { let command = Command { command: b"SUBSCRIBE", - args: CommandArgs::Args(&redis_channels), + args: CommandArgs::Blobs(&redis_channels), meta: CommandMeta::default() | CommandMeta::SUBSCRIPTION_REQUEST, }; - let promise = match this.send(global, frame.this(), &command) { + let promise = match this.send(global, &command) { Ok(p) => p, Err(err) => { - // If we catch an error, we need to clean up any handlers we may have added and fall out of subscription mode - this._subscription_ctx - .get() - .clear_all_receive_handlers(global)?; - return send_err_to_js(global, "Failed to send SUBSCRIBE command", &err); + for ch in &inserted_channels { + let _ = this.subscription_ctx.get().remove_receive_handler( + global, + *ch, + handler_callback, + )?; + } + return send_err_to_js(global, "Failed to send SUBSCRIBE command", err); } }; Ok(promise_to_js(promise)) } - /// Send redis the UNSUBSCRIBE RESP command and clean up anything necessary after the unsubscribe commoand. - /// - /// The subscription context must exist when calling this function. - fn send_unsubscribe_request_and_cleanup( - this: &Self, - this_js: JSValue, - global: &JSGlobalObject, - redis_channels: &[JSArgument], - ) -> JsResult { - send_cmd( - this, - global, - this_js, - b"UNSUBSCRIBE", - CommandArgs::Args(redis_channels), - CommandMeta::default(), - "Failed to send UNSUBSCRIBE command", - ) - } - #[bun_jsc::host_fn(method)] pub fn unsubscribe( this: &Self, @@ -1758,7 +1351,7 @@ impl JSValkeyClient { let _guard = this.ref_scope(); // Check if we're in subscription mode - require_subscriber(this, b"unsubscribe")?; + require_subscriber(this, "unsubscribe")?; let args_view = frame.arguments(); @@ -1766,28 +1359,25 @@ impl JSValkeyClient { // If no arguments, unsubscribe from all channels if args_view.is_empty() { - this._subscription_ctx - .get() - .clear_all_receive_handlers(global)?; - return Self::send_unsubscribe_request_and_cleanup( - this, - frame.this(), - global, - &redis_channels, - ); + let command = Command { + command: b"UNSUBSCRIBE", + args: CommandArgs::Blobs(&redis_channels), + meta: CommandMeta::default() | CommandMeta::SUBSCRIPTION_REQUEST, + }; + return match this.send(global, &command) { + Ok(p) => { + this.subscription_ctx + .get() + .clear_all_receive_handlers(global)?; + Ok(promise_to_js(p)) + } + Err(err) => send_err_to_js(global, "Failed to send UNSUBSCRIBE command", err), + }; } // The first argument can be a channel or an array of channels let channel_or_many = frame.argument(0); - // Get the subscription context - if !this._subscription_ctx.get().is_subscriber { - return Ok(JSPromise::resolved_promise_value( - global, - JSValue::UNDEFINED, - )); - } - // Two arguments means .unsubscribe(channel, listener) is invoked. if frame.arguments().len() == 2 { // In this case, the first argument is a channel string and the second @@ -1811,12 +1401,12 @@ impl JSValkeyClient { // unsubscribe from. This s important since this list is used to send // the UNSUBSCRIBE command to redis. Without this, we would end up // unsubscribing from all channels. - let Some(ch) = from_js(global, channel)? else { + let Some(ch) = coerce_redis_arg(global, channel)? else { return Err(global.throw_invalid_argument_type("unsubscribe", "channel", "string")); }; redis_channels.push(ch); - let remaining_listeners = match this._subscription_ctx.get().remove_receive_handler( + let remaining_listeners = match this.subscription_ctx.get().remove_receive_handler( global, channel, listener_cb, @@ -1836,12 +1426,22 @@ impl JSValkeyClient { // In this case, we only want to send the unsubscribe command to redis if there are no more listeners for this // channel. if remaining_listeners == 0 { - return Self::send_unsubscribe_request_and_cleanup( - this, - frame.this(), - global, - &redis_channels, - ); + let command = Command { + command: b"UNSUBSCRIBE", + args: CommandArgs::Blobs(&redis_channels), + meta: CommandMeta::default() | CommandMeta::SUBSCRIPTION_REQUEST, + }; + return match this.send(global, &command) { + Ok(p) => Ok(promise_to_js(p)), + Err(err) => { + this.subscription_ctx.get().upsert_receive_handler( + global, + channel, + listener_cb, + )?; + send_err_to_js(global, "Failed to send UNSUBSCRIBE command", err) + } + }; } // Otherwise, in order to keep the API consistent, we need to return a resolved promise. @@ -1851,20 +1451,24 @@ impl JSValkeyClient { )); } + let mut cleared_channels: Vec = Vec::with_capacity(1); + if channel_or_many.is_array() { - if channel_or_many.get_length(global)? == 0 { - return Err(global.throw_invalid_arguments(format_args!( - "unsubscribe requires at least one channel" - ))); + let len = channel_or_many.get_length(global)?; + if len == 0 { + return Err(global + .err( + ErrorCode::MISSING_ARGS, + format_args!("unsubscribe requires at least one channel"), + ) + .throw()); } - redis_channels.reserve( - (channel_or_many.get_length(global)? as usize).saturating_sub(redis_channels.len()), - ); + redis_channels.reserve((len as usize).saturating_sub(redis_channels.len())); // It is an array, so let's iterate over it let mut array_iter = channel_or_many.array_iterator(global)?; while let Some(channel_arg) = array_iter.next()? { - let Some(channel) = from_js(global, channel_arg)? else { + let Some(channel) = coerce_redis_arg(global, channel_arg)? else { return Err(global.throw_invalid_argument_type( "unsubscribe", "channel", @@ -1872,21 +1476,15 @@ impl JSValkeyClient { )); }; redis_channels.push(channel); - // Clear the handlers for this channel - this._subscription_ctx - .get() - .clear_receive_handlers(global, channel_arg)?; + cleared_channels.push(channel_arg); } } else if channel_or_many.is_string() { // It is a single string channel - let Some(channel) = from_js(global, channel_or_many)? else { + let Some(channel) = coerce_redis_arg(global, channel_or_many)? else { return Err(global.throw_invalid_argument_type("unsubscribe", "channel", "string")); }; redis_channels.push(channel); - // Clear the handlers for this channel - this._subscription_ctx - .get() - .clear_receive_handlers(global, channel_or_many)?; + cleared_channels.push(channel_or_many); } else { return Err(global.throw_invalid_argument_type( "unsubscribe", @@ -1895,8 +1493,22 @@ impl JSValkeyClient { )); } - // Now send the unsubscribe command and clean up if necessary - Self::send_unsubscribe_request_and_cleanup(this, frame.this(), global, &redis_channels) + let command = Command { + command: b"UNSUBSCRIBE", + args: CommandArgs::Blobs(&redis_channels), + meta: CommandMeta::default() | CommandMeta::SUBSCRIPTION_REQUEST, + }; + match this.send(global, &command) { + Ok(p) => { + for ch in &cleared_channels { + this.subscription_ctx + .get() + .clear_receive_handlers(global, *ch)?; + } + Ok(promise_to_js(p)) + } + Err(err) => send_err_to_js(global, "Failed to send UNSUBSCRIBE command", err), + } } #[bun_jsc::host_fn(method)] @@ -1904,15 +1516,11 @@ impl JSValkeyClient { let _ = frame; let new_client_ptr = this.clone_without_connecting(global)?; + let new_client_js = JSValkeyClient::ptr_to_js(new_client_ptr, global); + JSValkeyClient::bind_js(new_client_ptr, new_client_js); // SAFETY: clone_without_connecting returns a freshly allocated, leaked // JSValkeyClient (heap::alloc); valid for the rest of this scope. let new_client: &JSValkeyClient = unsafe { &*new_client_ptr }; - - let new_client_js = JSValkeyClient::ptr_to_js(new_client_ptr, global); - new_client.this_value.set(JsRef::init_weak(new_client_js)); - new_client - ._subscription_ctx - .set(SubscriptionCtx::init(new_client)?); // If the original client is already connected and not manually closed, start connecting the new client. if this.client.get().status == valkey::Status::Connected && !this.client.get().flags.is_manually_closed @@ -1927,16 +1535,4 @@ impl JSValkeyClient { Ok(JSPromise::resolved_promise_value(global, new_client_js)) } - - // script(subcommand: "LOAD", script: RedisValue) - // select(index: number | string) - // spublish(shardchannel: RedisValue, message: RedisValue) - // smove(source: RedisKey, destination: RedisKey, member: RedisValue) - // substr(key: RedisKey, start: number, end: number)` // Deprecated alias for getrang - // hstrlen(key: RedisKey, field: RedisValue) - // zrank(key: RedisKey, member: RedisValue) - // zrevrank(key: RedisKey, member: RedisValue) - // zscore(key: RedisKey, member: RedisValue) - - // cluster(subcommand: "KEYSLOT", key: RedisKey) } diff --git a/src/runtime/valkey_jsc/mod.rs b/src/runtime/valkey_jsc/mod.rs index c1490f533b49..d3af46f235d9 100644 --- a/src/runtime/valkey_jsc/mod.rs +++ b/src/runtime/valkey_jsc/mod.rs @@ -6,81 +6,10 @@ //! methods in [`js_valkey_functions`]. RESP wire-format parsing is in the //! lower `bun_valkey` crate; this module owns the JSC-touching half. -use crate::jsc::{JSGlobalObject, JSValue}; +pub mod command; +pub mod js_valkey; +pub mod js_valkey_functions; +pub mod protocol_jsc; +pub mod valkey; -// ─── submodules ────────────────────────────────────────────────────────────── - -#[path = "valkey.rs"] -pub mod valkey; // ValkeyClient state machine, fail/reject paths, auto-flush - -#[path = "js_valkey.rs"] -pub mod js_valkey; // JSValkeyClient host fns, SocketHandler, constructor - -#[path = "js_valkey_functions.rs"] -pub mod js_valkey_functions; // 200+ prototype methods (get/set/hget/…) - -#[path = "ValkeyCommand.rs"] -pub mod valkey_command_body; // Command::serialize, Promise::resolve/reject - -pub mod valkey_context { - /// Per-VM Valkey state. Empty: connections link into - /// `RareData.valkey_group` / `valkey_tls_group` directly, and the - /// default-TLS `SSL_CTX` is `RareData.defaultClientSslCtx()`. - #[derive(Default)] - pub struct ValkeyContext; -} - -#[path = "protocol_jsc.rs"] -pub mod protocol_jsc; // RESPValue → JSValue, RedisError → JS Error - -#[path = "index.rs"] -pub mod index; - -// ─── back-compat aliases ───────────────────────────────────────────────────── -// Sibling files were written against `*_body` module names (`valkey.rs` -// imports `super::js_valkey_body`); keep the alias so it doesn't need to churn. -pub use self::js_valkey as js_valkey_body; - -// ─── public re-exports ─────────────────────────────────────────────────────── pub use js_valkey::JSValkeyClient; -pub use protocol_jsc::{ - ToJSOptions, resp_value_to_js, resp_value_to_js_with_options, valkey_error_to_js, -}; -pub use valkey::{Options, Protocol, Status, ValkeyClient}; -pub use valkey_context::ValkeyContext; - -// ── ValkeyCommand ──────────────────────────────────────────────────────────── -// `ValkeyCommand` is both a namespace -// *and* the `Command` type. Expose a `valkey_command` module that re-exports -// the body's items so `command::PromisePair` / `command::Entry` resolve, and -// alias it as `ValkeyCommand` for callers that use that spelling. -pub mod valkey_command { - pub use super::valkey_command_body::{Entry, Meta, Promise, PromisePair, entry, promise_pair}; - // `index.rs` re-exports `super::valkey_command::ValkeyCommand`. - pub use super::ValkeyCommand; -} -pub use valkey_command as ValkeyCommand; - -// ── JsClass wiring (codegen name = "RedisClient", see valkey.classes.ts) ──── -// `generate-classes.ts` emits the `RedisClient__{fromJS,fromJSDirect,create, -// getConstructor}` externs plus safe wrappers as **free functions** in -// `crate::generated_classes::js_RedisClient` (one `js_` submodule per -// class). The codegen's `RedisClient` *type* re-export resolves back to -// `JSValkeyClient` itself (via `valkey::RedisClient`), so the wrapper pointee -// is already `*mut JSValkeyClient` — no cast required. Route through the -// submodule instead of redeclaring the externs here; a second `extern "C"` -// block would trip `clashing_extern_declarations`. -use crate::generated_classes::js_RedisClient; - -impl JSValkeyClient { - /// Wrap an already-heap-allocated client pointer in its JS object. - /// Ownership transfers to the C++ wrapper (freed via `finalize`). - #[inline] - pub fn ptr_to_js(ptr: *mut Self, global: &JSGlobalObject) -> JSValue { - // `ptr` was produced by `JSValkeyClient::new` (heap-allocated) and is - // hereby owned by the JS wrapper. - js_RedisClient::to_js(ptr, global) - } -} - -bun_jsc::impl_js_class_via_generated!(JSValkeyClient => crate::generated_classes::js_RedisClient); diff --git a/src/runtime/valkey_jsc/protocol_jsc.rs b/src/runtime/valkey_jsc/protocol_jsc.rs index a26a981fd162..026deb952f1e 100644 --- a/src/runtime/valkey_jsc/protocol_jsc.rs +++ b/src/runtime/valkey_jsc/protocol_jsc.rs @@ -3,11 +3,9 @@ //! `valkey/`; only the `JSGlobalObject`/`JSValue`-touching conversions live //! here so `valkey/` is JSC-free. -use crate::jsc::{Error as JscError, JSGlobalObject, JSValue, JsError, JsResult, bun_string_jsc}; +use crate::jsc::{ErrorCode, JSGlobalObject, JSValue, JsResult, bun_string_jsc}; use bun_valkey::valkey_protocol::{RESPValue, RedisError}; -// keep `protocol` referenced for sibling drafts - /// All callers always provide a message, so the parameter /// is `impl AsRef<[u8]>` to accept `&str`, `&[u8]`, `&[u8; N]`, `&Box<[u8]>` /// uniformly without forcing `Some(..)` at every call site. @@ -16,52 +14,47 @@ pub fn valkey_error_to_js( message: impl AsRef<[u8]>, err: RedisError, ) -> JSValue { - let error_code: JscError = match err { - RedisError::ConnectionClosed => JscError::REDIS_CONNECTION_CLOSED, - RedisError::InvalidResponse => JscError::REDIS_INVALID_RESPONSE, - RedisError::InvalidBulkString => JscError::REDIS_INVALID_BULK_STRING, - RedisError::InvalidArray => JscError::REDIS_INVALID_ARRAY, - RedisError::InvalidInteger => JscError::REDIS_INVALID_INTEGER, - RedisError::InvalidSimpleString => JscError::REDIS_INVALID_SIMPLE_STRING, - RedisError::InvalidErrorString => JscError::REDIS_INVALID_ERROR_STRING, + let error_code: ErrorCode = match err { + RedisError::ConnectionClosed => ErrorCode::REDIS_CONNECTION_CLOSED, + RedisError::InvalidResponse => ErrorCode::REDIS_INVALID_RESPONSE, + RedisError::InvalidBulkString => ErrorCode::REDIS_INVALID_BULK_STRING, + RedisError::InvalidInteger => ErrorCode::REDIS_INVALID_INTEGER, RedisError::InvalidDouble | RedisError::InvalidBoolean - | RedisError::InvalidNull | RedisError::InvalidMap | RedisError::InvalidSet - | RedisError::InvalidBigNumber | RedisError::InvalidVerbatimString | RedisError::InvalidBlobError | RedisError::InvalidAttribute - | RedisError::InvalidPush => JscError::REDIS_INVALID_RESPONSE, - RedisError::AuthenticationFailed => JscError::REDIS_AUTHENTICATION_FAILED, - RedisError::InvalidCommand => JscError::REDIS_INVALID_COMMAND, - RedisError::InvalidArgument => JscError::REDIS_INVALID_ARGUMENT, - RedisError::UnsupportedProtocol => JscError::REDIS_INVALID_RESPONSE, - RedisError::InvalidResponseType => JscError::REDIS_INVALID_RESPONSE_TYPE, - RedisError::ConnectionTimeout => JscError::REDIS_CONNECTION_TIMEOUT, - RedisError::IdleTimeout => JscError::REDIS_IDLE_TIMEOUT, - RedisError::NestingDepthExceeded => JscError::REDIS_INVALID_RESPONSE, - RedisError::LineTooLong => JscError::REDIS_INVALID_RESPONSE, - RedisError::JSError => return global.take_exception(JsError::Thrown), - RedisError::OutOfMemory => { - let _ = global.throw_out_of_memory(); - return global.take_exception(JsError::Thrown); - } - RedisError::JSTerminated => return global.take_exception(JsError::Terminated), + | RedisError::InvalidPush => ErrorCode::REDIS_INVALID_RESPONSE, + RedisError::AuthenticationFailed => ErrorCode::REDIS_AUTHENTICATION_FAILED, + RedisError::ServerError => ErrorCode::REDIS_SERVER_ERROR, + RedisError::InvalidCommand => ErrorCode::REDIS_INVALID_COMMAND, + RedisError::InvalidArgument => ErrorCode::REDIS_INVALID_ARGUMENT, + RedisError::UnsupportedProtocol => ErrorCode::REDIS_INVALID_RESPONSE, + RedisError::InvalidResponseType => ErrorCode::REDIS_INVALID_RESPONSE_TYPE, + RedisError::ConnectionTimeout => ErrorCode::REDIS_CONNECTION_TIMEOUT, + RedisError::IdleTimeout => ErrorCode::REDIS_IDLE_TIMEOUT, + RedisError::NestingDepthExceeded => ErrorCode::REDIS_INVALID_RESPONSE, + RedisError::LineTooLong => ErrorCode::REDIS_INVALID_RESPONSE, + RedisError::OutOfMemory => return global.create_out_of_memory_error(), }; let msg = message.as_ref(); - if !msg.is_empty() { - return error_code.fmt(global, format_args!("{}", bstr::BStr::new(msg))); + let tag: &'static str = err.into(); + if msg.is_empty() { + error_code.fmt(global, format_args!("Valkey error: {tag}")) + } else if matches!(error_code, ErrorCode::REDIS_INVALID_RESPONSE) { + // Several parser variants collapse into this one code; append the + // variant name so InvalidDouble/NestingDepthExceeded/etc. are + // distinguishable. Other codes are already 1:1 with their variant. + error_code.fmt(global, format_args!("{}: {tag}", bstr::BStr::new(msg))) + } else { + error_code.fmt(global, format_args!("{}", bstr::BStr::new(msg))) } - error_code.fmt( - global, - format_args!("Valkey error: {}", <&'static str>::from(err)), - ) } -pub fn resp_value_to_js(this: &mut RESPValue, global: &JSGlobalObject) -> JsResult { +pub fn resp_value_to_js(this: RESPValue, global: &JSGlobalObject) -> JsResult { resp_value_to_js_with_options(this, global, ToJSOptions::default()) } @@ -72,35 +65,30 @@ pub struct ToJSOptions { fn valkey_str_to_js_value( global: &JSGlobalObject, - str: &mut Box<[u8]>, + str: Box<[u8]>, options: ToJSOptions, ) -> JsResult { if options.return_as_buffer { // The parser's payload is an owned allocation that is only converted // once; adopt it as the Buffer backing store instead of copying it // into a fresh ArrayBuffer. - Ok(JSValue::create_buffer_from_box( - global, - core::mem::take(str), - )) + Ok(JSValue::create_buffer_from_box(global, str)) } else { - bun_string_jsc::create_utf8_for_js(global, str) + bun_string_jsc::create_utf8_for_js(global, &str) } } pub fn resp_value_to_js_with_options( - this: &mut RESPValue, + this: RESPValue, global: &JSGlobalObject, options: ToJSOptions, ) -> JsResult { match this { RESPValue::SimpleString(str) => valkey_str_to_js_value(global, str, options), - RESPValue::Error(str) => Ok(valkey_error_to_js( - global, - &**str, - RedisError::InvalidResponse, - )), - RESPValue::Integer(int) => Ok(JSValue::js_number(*int as f64)), + RESPValue::Error(str) | RESPValue::BlobError(str) => { + Ok(valkey_error_to_js(global, &*str, RedisError::ServerError)) + } + RESPValue::Integer(int) => Ok(JSValue::js_number(int as f64)), RESPValue::BulkString(maybe_str) => { if let Some(str) = maybe_str { valkey_str_to_js_value(global, str, options) @@ -108,43 +96,34 @@ pub fn resp_value_to_js_with_options( Ok(JSValue::NULL) } } - RESPValue::Array(array) => { - JSValue::create_array_from_iter(global, array.iter_mut(), |item| { + RESPValue::Array(items) | RESPValue::Set(items) => { + JSValue::create_array_from_iter(global, items.into_iter(), |item| { resp_value_to_js_with_options(item, global, options) }) } RESPValue::Null => Ok(JSValue::NULL), - RESPValue::Double(d) => Ok(JSValue::js_number(*d)), - RESPValue::Boolean(b) => Ok(JSValue::from(*b)), - RESPValue::BlobError(str) => Ok(valkey_error_to_js( - global, - &**str, - RedisError::InvalidBlobError, - )), + RESPValue::Double(d) => Ok(JSValue::js_number(d)), + RESPValue::Boolean(b) => Ok(JSValue::from(b)), RESPValue::VerbatimString(verbatim) => { - valkey_str_to_js_value(global, &mut verbatim.content, options) + valkey_str_to_js_value(global, verbatim.content, options) } RESPValue::Map(entries) => { let js_obj = JSValue::create_empty_object_with_null_prototype(global); - for entry in entries.iter_mut() { + for entry in entries.into_iter() { let js_key = - resp_value_to_js_with_options(&mut entry.key, global, ToJSOptions::default())?; + resp_value_to_js_with_options(entry.key, global, ToJSOptions::default())?; // Route through `put_to_property_key`, which performs // index-vs-string property dispatch on the JSValue key. - let _ = js_key.to_bun_string(global)?; // preserve toString side-effect/exception path - let js_value = resp_value_to_js_with_options(&mut entry.value, global, options)?; + let js_value = resp_value_to_js_with_options(entry.value, global, options)?; JSValue::put_to_property_key(js_obj, global, js_key, js_value)?; } Ok(js_obj) } - RESPValue::Set(set) => JSValue::create_array_from_iter(global, set.iter_mut(), |item| { - resp_value_to_js_with_options(item, global, options) - }), RESPValue::Attribute(attribute) => { // For now, we just return the value and ignore attributes // In the future, we could attach the attributes as a hidden property - resp_value_to_js_with_options(&mut attribute.value, global, options) + resp_value_to_js_with_options(*attribute.value, global, options) } RESPValue::Push(push) => { let js_obj = JSValue::create_empty_object_with_null_prototype(global); @@ -155,21 +134,13 @@ pub fn resp_value_to_js_with_options( // Add the data as an array let data_array = - JSValue::create_array_from_iter(global, push.data.iter_mut(), |item| { + JSValue::create_array_from_iter(global, push.data.into_iter(), |item| { resp_value_to_js_with_options(item, global, options) })?; js_obj.put(global, b"data", data_array); Ok(js_obj) } - RESPValue::BigNumber(str) => { - // Try to parse as number if possible - if let Ok(int) = bun_core::fmt::parse_int::(str, 10) { - Ok(JSValue::js_number(int as f64)) - } else { - // If it doesn't fit in an i64, return as string - bun_string_jsc::create_utf8_for_js(global, str) - } - } + RESPValue::BigNumber(str) => valkey_str_to_js_value(global, str, options), } } diff --git a/src/runtime/valkey_jsc/valkey.classes.ts b/src/runtime/valkey_jsc/valkey.classes.ts index 3e2cf711c81b..1721b9af890b 100644 --- a/src/runtime/valkey_jsc/valkey.classes.ts +++ b/src/runtime/valkey_jsc/valkey.classes.ts @@ -1,9 +1,190 @@ import { define } from "../../codegen/class-definitions"; +// [jsName, length] — `length` is the JS-visible `function.length` (count of +// required positional params per the Redis command docs, before the first +// optional/rest). It is NOT the Rust-side validation minimum. +const commands: ReadonlyArray = [ + ["get", 1], + ["getBuffer", 1], + ["set", 2], + ["del", 1], + ["incr", 1], + ["incrby", 2], + ["incrbyfloat", 2], + ["decr", 1], + ["decrby", 2], + ["exists", 1], + ["expire", 2], + ["expireat", 2], + ["pexpire", 2], + ["ttl", 1], + ["hmset", 2], + ["hset", 2], + ["hsetnx", 3], + ["hget", 2], + ["hmget", 2], + ["hdel", 2], + ["hexists", 2], + ["hrandfield", 1], + ["hscan", 2], + ["hgetdel", 2], + ["hgetex", 2], + ["hsetex", 3], + ["hexpire", 3], + ["hexpireat", 3], + ["hexpiretime", 2], + ["hpersist", 2], + ["hpexpire", 3], + ["hpexpireat", 3], + ["hpexpiretime", 2], + ["hpttl", 2], + ["httl", 2], + ["sismember", 2], + ["sadd", 2], + ["srem", 2], + ["smembers", 1], + ["srandmember", 1], + ["spop", 1], + ["hincrby", 3], + ["hincrbyfloat", 3], + ["bitcount", 1], + ["blmove", 5], + ["blmpop", 3], + ["blpop", 2], + ["brpop", 2], + ["brpoplpush", 3], + ["getbit", 2], + ["setbit", 3], + ["getrange", 3], + ["setrange", 3], + ["dump", 1], + ["expiretime", 1], + ["getdel", 1], + ["getex", 0], + ["hgetall", 1], + ["hkeys", 1], + ["hlen", 1], + ["hvals", 1], + ["keys", 1], + ["lindex", 2], + ["linsert", 4], + ["llen", 1], + ["lmove", 4], + ["lmpop", 2], + ["lpop", 1], + ["lpos", 2], + ["lrange", 3], + ["lrem", 3], + ["lset", 3], + ["ltrim", 3], + ["persist", 1], + ["pexpireat", 2], + ["pexpiretime", 1], + ["pttl", 1], + ["randomkey", 0], + ["rpop", 1], + ["rpoplpush", 2], + ["scan", 0], + ["scard", 1], + ["sdiff", 1], + ["sdiffstore", 2], + ["sinter", 1], + ["sintercard", 1], + ["sinterstore", 2], + ["smismember", 2], + ["sscan", 2], + ["strlen", 1], + ["sunion", 1], + ["sunionstore", 2], + ["type", 1], + ["zcard", 1], + ["zcount", 3], + ["zlexcount", 3], + ["zpopmax", 1], + ["zpopmin", 1], + ["zrandmember", 1], + ["zrange", 3], + ["zrangebylex", 3], + ["zrangebyscore", 3], + ["zrangestore", 4], + ["zrem", 2], + ["zremrangebylex", 3], + ["zremrangebyrank", 3], + ["zremrangebyscore", 3], + ["zrevrange", 3], + ["zrevrangebylex", 3], + ["zrevrangebyscore", 3], + ["append", 2], + ["getset", 2], + ["lpush", 0], + ["lpushx", 0], + ["pfadd", 2], + ["rpush", 0], + ["rpushx", 0], + ["setnx", 2], + ["setex", 3], + ["psetex", 3], + ["zscore", 2], + ["zincrby", 3], + ["zmscore", 0], + ["zadd", 3], + ["zscan", 2], + ["zdiff", 1], + ["zdiffstore", 2], + ["zinter", 2], + ["zintercard", 1], + ["zinterstore", 3], + ["zunion", 2], + ["zunionstore", 3], + ["zmpop", 2], + ["bzmpop", 3], + ["bzpopmin", 2], + ["bzpopmax", 2], + ["mget", 1], + ["mset", 0], + ["msetnx", 0], + ["ping", 0], + ["publish", 2], + ["script", 0], + ["select", 0], + ["spublish", 2], + ["smove", 3], + ["substr", 3], + ["hstrlen", 2], + ["zrank", 1], + ["zrevrank", 1], + ["subscribe", 2], + ["psubscribe", 0], + ["unsubscribe", 0], + ["punsubscribe", 0], + ["pubsub", 0], + ["copy", 0], + ["unlink", 1], + ["touch", 1], + ["rename", 2], + ["renamenx", 2], +]; + +const reserved = new Set([ + "connected", + "onconnect", + "onclose", + "bufferedAmount", + "connect", + "close", + "send", + "duplicate", +]); +for (const [name] of commands) { + if (reserved.has(name)) + throw new Error(`valkey.classes.ts: command '${name}' collides with a reserved prototype slot`); +} + +const commandProto = Object.fromEntries(commands.map(([name, length]) => [name, { fn: name, length }])); + export default [ define({ name: "RedisClient", - // R-2 Phase 2: user impls take `&self`; emit `this: &T` shims. sharedThis: true, construct: true, constructNeedsThis: true, @@ -13,6 +194,7 @@ export default [ JSType: "0b11101110", memoryCost: true, proto: { + // Lifecycle / accessors connected: { getter: "getConnected", }, @@ -29,59 +211,6 @@ export default [ bufferedAmount: { getter: "getBufferedAmount", }, - // Valkey commands - get: { - fn: "get", - length: 1, - }, - getBuffer: { - fn: "getBuffer", - length: 1, - }, - set: { - fn: "set", - length: 2, - }, - del: { - fn: "del", - length: 1, - }, - incr: { - fn: "incr", - length: 1, - }, - incrby: { - fn: "incrby", - length: 2, - }, - incrbyfloat: { - fn: "incrbyfloat", - length: 2, - }, - decr: { - fn: "decr", - length: 1, - }, - decrby: { - fn: "decrby", - length: 2, - }, - exists: { - fn: "exists", - length: 1, - }, - expire: { - fn: "expire", - length: 2, - }, - expireat: { - fn: "expireat", - length: 2, - }, - pexpire: { - fn: "pexpire", - length: 2, - }, connect: { fn: "jsConnect", length: 0, @@ -94,495 +223,12 @@ export default [ fn: "jsSend", length: 2, }, - ttl: { - fn: "ttl", - length: 1, - }, - hmset: { - fn: "hmset", - length: 2, - }, - hset: { - fn: "hset", - length: 2, - }, - hsetnx: { - fn: "hsetnx", - length: 3, - }, - hget: { - fn: "hget", - length: 2, - }, - hmget: { - fn: "hmget", - length: 2, - }, - hdel: { - fn: "hdel", - length: 2, - }, - hexists: { - fn: "hexists", - length: 2, - }, - hrandfield: { - fn: "hrandfield", - length: 1, - }, - hscan: { - fn: "hscan", - length: 2, - }, - hgetdel: { - fn: "hgetdel", - length: 2, - }, - hgetex: { - fn: "hgetex", - length: 2, - }, - hsetex: { - fn: "hsetex", - length: 3, - }, - hexpire: { - fn: "hexpire", - length: 3, - }, - hexpireat: { - fn: "hexpireat", - length: 3, - }, - hexpiretime: { - fn: "hexpiretime", - length: 2, - }, - hpersist: { - fn: "hpersist", - length: 2, - }, - hpexpire: { - fn: "hpexpire", - length: 3, - }, - hpexpireat: { - fn: "hpexpireat", - length: 3, - }, - hpexpiretime: { - fn: "hpexpiretime", - length: 2, - }, - hpttl: { - fn: "hpttl", - length: 2, - }, - httl: { - fn: "httl", - length: 2, - }, - sismember: { - fn: "sismember", - length: 2, - }, - sadd: { - fn: "sadd", - length: 2, - }, - srem: { - fn: "srem", - length: 2, - }, - smembers: { - fn: "smembers", - length: 1, - }, - srandmember: { - fn: "srandmember", - length: 1, - }, - spop: { - fn: "spop", - length: 1, - }, - hincrby: { - fn: "hincrby", - length: 3, - }, - hincrbyfloat: { - fn: "hincrbyfloat", - length: 3, - }, - bitcount: { - fn: "bitcount", - }, - blmove: { - fn: "blmove", - length: 5, - }, - blmpop: { - fn: "blmpop", - length: 3, - }, - blpop: { - fn: "blpop", - length: 2, - }, - brpop: { - fn: "brpop", - length: 2, - }, - brpoplpush: { - fn: "brpoplpush", - length: 3, - }, - getbit: { - fn: "getbit", - length: 2, - }, - setbit: { - fn: "setbit", - length: 3, - }, - getrange: { - fn: "getrange", - length: 3, - }, - setrange: { - fn: "setrange", - length: 3, - }, - dump: { - fn: "dump", - }, - expiretime: { - fn: "expiretime", - }, - getdel: { - fn: "getdel", - }, - getex: { - fn: "getex", - }, - hgetall: { - fn: "hgetall", - }, - hkeys: { - fn: "hkeys", - }, - hlen: { - fn: "hlen", - }, - hvals: { - fn: "hvals", - }, - keys: { - fn: "keys", - }, - lindex: { - fn: "lindex", - length: 2, - }, - linsert: { - fn: "linsert", - length: 4, - }, - llen: { - fn: "llen", - }, - lmove: { - fn: "lmove", - length: 4, - }, - lmpop: { - fn: "lmpop", - length: 2, - }, - lpop: { - fn: "lpop", - }, - lpos: { - fn: "lpos", - length: 2, - }, - lrange: { - fn: "lrange", - length: 3, - }, - lrem: { - fn: "lrem", - length: 3, - }, - lset: { - fn: "lset", - length: 3, - }, - ltrim: { - fn: "ltrim", - length: 3, - }, - persist: { - fn: "persist", - }, - pexpireat: { - fn: "pexpireat", - length: 2, - }, - pexpiretime: { - fn: "pexpiretime", - }, - pttl: { - fn: "pttl", - }, - randomkey: { - fn: "randomkey", + duplicate: { + fn: "duplicate", length: 0, }, - rpop: { - fn: "rpop", - }, - rpoplpush: { - fn: "rpoplpush", - length: 2, - }, - scan: { - fn: "scan", - }, - scard: { - fn: "scard", - }, - sdiff: { - fn: "sdiff", - length: 1, - }, - sdiffstore: { - fn: "sdiffstore", - length: 2, - }, - sinter: { - fn: "sinter", - length: 1, - }, - sintercard: { - fn: "sintercard", - length: 1, - }, - sinterstore: { - fn: "sinterstore", - length: 2, - }, - smismember: { - fn: "smismember", - length: 2, - }, - sscan: { - fn: "sscan", - length: 2, - }, - strlen: { - fn: "strlen", - }, - sunion: { - fn: "sunion", - length: 1, - }, - sunionstore: { - fn: "sunionstore", - length: 2, - }, - type: { - fn: "type", - length: 1, - }, - zcard: { - fn: "zcard", - }, - zcount: { - fn: "zcount", - length: 3, - }, - zlexcount: { - fn: "zlexcount", - length: 3, - }, - zpopmax: { - fn: "zpopmax", - }, - zpopmin: { - fn: "zpopmin", - }, - zrandmember: { - fn: "zrandmember", - }, - zrange: { - fn: "zrange", - length: 3, - }, - zrangebylex: { - fn: "zrangebylex", - length: 3, - }, - zrangebyscore: { - fn: "zrangebyscore", - length: 3, - }, - zrangestore: { - fn: "zrangestore", - length: 4, - }, - zrem: { - fn: "zrem", - length: 2, - }, - zremrangebylex: { - fn: "zremrangebylex", - length: 3, - }, - zremrangebyrank: { - fn: "zremrangebyrank", - length: 3, - }, - zremrangebyscore: { - fn: "zremrangebyscore", - length: 3, - }, - zrevrange: { - fn: "zrevrange", - length: 3, - }, - zrevrangebylex: { - fn: "zrevrangebylex", - length: 3, - }, - zrevrangebyscore: { - fn: "zrevrangebyscore", - length: 3, - }, - append: { - fn: "append", - }, - getset: { - fn: "getset", - }, - lpush: { - fn: "lpush", - }, - lpushx: { - fn: "lpushx", - }, - pfadd: { - fn: "pfadd", - }, - rpush: { - fn: "rpush", - }, - rpushx: { - fn: "rpushx", - }, - setnx: { - fn: "setnx", - }, - setex: { - fn: "setex", - length: 3, - }, - psetex: { - fn: "psetex", - length: 3, - }, - zscore: { - fn: "zscore", - }, - zincrby: { - fn: "zincrby", - length: 3, - }, - zmscore: { - fn: "zmscore", - }, - zadd: { - fn: "zadd", - length: 3, - }, - zscan: { - fn: "zscan", - length: 2, - }, - zdiff: { - fn: "zdiff", - length: 1, - }, - zdiffstore: { - fn: "zdiffstore", - length: 2, - }, - zinter: { - fn: "zinter", - length: 2, - }, - zintercard: { - fn: "zintercard", - length: 1, - }, - zinterstore: { - fn: "zinterstore", - length: 3, - }, - zunion: { - fn: "zunion", - length: 2, - }, - zunionstore: { - fn: "zunionstore", - length: 3, - }, - zmpop: { - fn: "zmpop", - length: 2, - }, - bzmpop: { - fn: "bzmpop", - length: 3, - }, - bzpopmin: { - fn: "bzpopmin", - length: 2, - }, - bzpopmax: { - fn: "bzpopmax", - length: 2, - }, - mget: { - fn: "mget", - }, - mset: { - fn: "mset", - }, - msetnx: { - fn: "msetnx", - }, - ping: { fn: "ping" }, - publish: { fn: "publish" }, - script: { fn: "script" }, - select: { fn: "select" }, - spublish: { fn: "spublish" }, - smove: { fn: "smove" }, - substr: { fn: "substr" }, - hstrlen: { fn: "hstrlen" }, - zrank: { fn: "zrank" }, - zrevrank: { fn: "zrevrank" }, - subscribe: { fn: "subscribe" }, - duplicate: { fn: "duplicate" }, - psubscribe: { fn: "psubscribe" }, - unsubscribe: { fn: "unsubscribe" }, - punsubscribe: { fn: "punsubscribe" }, - pubsub: { fn: "pubsub" }, - copy: { fn: "copy" }, - unlink: { fn: "unlink" }, - touch: { fn: "touch" }, - rename: { fn: "rename", length: 2 }, - renamenx: { fn: "renamenx", length: 2 }, + // Valkey commands + ...commandProto, }, values: ["onconnect", "onclose", "connectionPromise", "hello", "subscriptionCallbackMap"], }), diff --git a/src/runtime/valkey_jsc/valkey.rs b/src/runtime/valkey_jsc/valkey.rs index 8af08821c23f..1a2d914ac767 100644 --- a/src/runtime/valkey_jsc/valkey.rs +++ b/src/runtime/valkey_jsc/valkey.rs @@ -4,18 +4,18 @@ use bun_collections::VecExt; // This file contains the core Valkey client implementation with protocol handling use bun_collections::OffsetByteList; +use bun_core::UnwrapOrOom; use bun_jsc::virtual_machine::VirtualMachine; use bun_jsc::{GlobalRef, JSGlobalObject, JSPromise, JSValue, JsResult}; use bun_uws::{self as uws, AnySocket, SocketGroup, SocketKind, SslCtx}; use bun_valkey::valkey_protocol as protocol; use bun_valkey::valkey_protocol::{RESPValue, RedisError}; -use super::js_valkey_body::JSValkeyClient; -use super::protocol_jsc::{resp_value_to_js, valkey_error_to_js}; -use super::valkey_command_body as command; -use super::valkey_command_body::{Args, Command}; - -pub use super::valkey_context as ValkeyContext; +use super::command; +use super::command::{Args, Command}; +use super::js_valkey::JSValkeyClient; +use super::protocol_jsc::valkey_error_to_js; +use crate::webcore::{AutoFlusher, HasAutoFlusher}; /// Codegen target name. `valkey.classes.ts` declares `name: "RedisClient"`, so /// `generate-classes.ts` resolves the native backing struct to @@ -23,19 +23,13 @@ pub use super::valkey_context as ValkeyContext; /// `RedisClient::method(…)` thunks against it. The actual host type is /// `JSValkeyClient` (sibling `js_valkey.rs`); re-export it under the codegen /// spelling here so the generated `pub use` and prototype thunks resolve. -pub use super::js_valkey_body::JSValkeyClient as RedisClient; - -type JsTerminated = bun_jsc::JsResult; +pub use super::js_valkey::JSValkeyClient as RedisClient; bun_output::define_scoped_log!(debug, Redis, visible); /// Connection flags to track Valkey client state pub struct ConnectionFlags { - // These flags could be refactored into an enumerated state machine, which - // would read more naturally than a bag of booleans. - pub is_authenticated: bool, pub is_manually_closed: bool, - pub is_selecting_db_internal: bool, pub enable_offline_queue: bool, pub needs_to_open_socket: bool, pub enable_auto_reconnect: bool, @@ -58,9 +52,7 @@ pub struct ConnectionFlags { impl Default for ConnectionFlags { fn default() -> Self { Self { - is_authenticated: false, is_manually_closed: false, - is_selecting_db_internal: false, enable_offline_queue: true, needs_to_open_socket: true, enable_auto_reconnect: true, @@ -81,15 +73,15 @@ pub enum Status { Connected, } -impl Status { - #[inline] - pub fn is_active(self) -> bool { - matches!(self, Status::Connected | Status::Connecting) - } +/// Response-dispatch state for the HELLO/SELECT handshake. +#[derive(Copy, Clone, Eq, PartialEq, Default)] +pub enum Handshake { + #[default] + AwaitingHello, + SelectingDb, + Ready, } -pub use super::valkey_command_body as Command_; - /// Valkey protocol types (standalone, TLS, Unix socket) #[derive(Copy, Clone, Eq, PartialEq)] pub enum Protocol { @@ -144,13 +136,20 @@ impl TLS { _ => false, } } + + #[inline] + pub fn is_none(&self) -> bool { + matches!(self, TLS::None) + } } -// Call sites only ever compare against `TLS::None` / `TLS::Enabled`; `SSLConfig` -// doesn't (and shouldn't) implement `PartialEq`, so compare by discriminant. -impl PartialEq for TLS { - fn eq(&self, other: &Self) -> bool { - core::mem::discriminant(self) == core::mem::discriminant(other) +impl Clone for TLS { + fn clone(&self) -> Self { + match self { + TLS::None => TLS::None, + TLS::Enabled => TLS::Enabled, + TLS::Custom(c) => TLS::Custom(c.clone()), + } } } @@ -162,7 +161,6 @@ pub struct Options { pub max_retries: u32, pub enable_offline_queue: bool, pub enable_auto_pipelining: bool, - pub enable_debug_logging: bool, pub tls: TLS, } @@ -175,26 +173,22 @@ impl Default for Options { enable_auto_reconnect: true, max_retries: 20, enable_offline_queue: true, - enable_auto_pipelining: true, - enable_debug_logging: false, + enable_auto_pipelining: + !bun_core::env_var::feature_flag::BUN_FEATURE_FLAG_DISABLE_REDIS_AUTO_PIPELINING + .get() + .unwrap_or(false), tls: TLS::None, } } } +#[derive(Clone)] pub enum Address { Unix(Box<[u8]>), Host { host: Box<[u8]>, port: u16 }, } impl Address { - pub(crate) fn hostname(&self) -> &[u8] { - match self { - Address::Unix(unix_addr) => unix_addr, - Address::Host { host, .. } => host, - } - } - /// Open a TCP/TLS/Unix socket via /// `uws::Socket{TLS,TCP}::connect_*_group`. /// @@ -251,6 +245,7 @@ impl Address { pub struct ValkeyClient { pub socket: AnySocket, pub status: Status, + pub handshake: Handshake, // Buffer management pub write_buffer: OffsetByteList, @@ -259,27 +254,22 @@ pub struct ValkeyClient { pub reply_scanner: protocol::ReplyScanner, /// In-flight commands, after the data has been written to the network socket - pub in_flight: command::promise_pair::Queue, + pub in_flight: command::PromiseQueue, /// Commands that are waiting to be sent to the server. When pipelining is implemented, this usually will be empty. - pub queue: command::entry::Queue, + pub queue: command::EntryQueue, // Connection parameters - // `connection_strings` is retained because `js_valkey.rs` still slices it - // when constructing/duplicating clients. pub password: Box<[u8]>, pub username: Box<[u8]>, pub database: u32, pub address: Address, - pub protocol: Protocol, - - pub connection_strings: Box<[u8]>, // TLS support pub tls: TLS, // Timeout and reconnection management - pub idle_timeout_interval_ms: u32, + pub idle_timeout_ms: u32, pub connection_timeout_ms: u32, pub retry_attempts: u32, pub max_retries: u32, // Maximum retry attempts @@ -292,22 +282,16 @@ pub struct ValkeyClient { pub vm: &'static VirtualMachine, } -#[derive(Copy, Clone, Eq, PartialEq)] -enum SubscribeHandled { - Handled, - Fallthrough, -} - pub(crate) struct DeferredFailure { message: Box<[u8]>, err: RedisError, global_this: GlobalRef, - in_flight: command::promise_pair::Queue, - queue: command::entry::Queue, + in_flight: command::PromiseQueue, + queue: command::EntryQueue, } impl DeferredFailure { - pub(crate) fn run(self) -> JsTerminated<()> { + pub(crate) fn run(self) -> JsResult<()> { debug!("running deferred failure"); let mut this = self; let err = valkey_error_to_js(&this.global_this, &*this.message, this.err); @@ -335,12 +319,6 @@ impl DeferredFailure { } } -/// Read the parser's current byte offset. -#[inline] -fn reader_pos(reader: &protocol::ValkeyReader<'_>) -> usize { - reader.pos() -} - // SAFETY: `ValkeyClient` lives at `JSValkeyClient.client` (intrusive embed via // `container_of`). `JsCell` is `#[repr(transparent)]`, so the // field offset is unchanged. R-2: shared `&` only — every `JSValkeyClient` @@ -351,9 +329,8 @@ impl ValkeyClient { /// Clean up resources used by the Valkey client // Cannot be `Drop` — takes a JSGlobalObject param and has JS side effects. pub fn shutdown(&mut self, global_object_or_finalizing: Option<&JSGlobalObject>) { - let mut pending = - core::mem::replace(&mut self.in_flight, command::promise_pair::Queue::init()); - let mut commands = core::mem::replace(&mut self.queue, command::entry::Queue::init()); + let mut pending = core::mem::replace(&mut self.in_flight, command::PromiseQueue::init()); + let mut commands = core::mem::replace(&mut self.queue, command::EntryQueue::init()); if let Some(global_this) = global_object_or_finalizing { let object = valkey_error_to_js( @@ -361,52 +338,39 @@ impl ValkeyClient { b"Connection closed", RedisError::ConnectionClosed, ); - while let Some(mut pair) = pending.read_item() { + while let Some(mut promise) = pending.read_item() { // Any exception from the reject is swallowed so // every remaining pending command still gets rejected at shutdown. - let _ = pair.reject_command(global_this, object); + let _ = promise.reject(global_this, object); } while let Some(mut offline_cmd) = commands.read_item() { // Same as above: swallow reject exceptions so the whole queue drains. - let _ = offline_cmd.promise.reject(global_this, Ok(object)); - // Note: `offline_cmd.deinit()` — Entry/Box<[u8]> drops automatically. + let _ = offline_cmd.promise.reject(global_this, object); } } else { // finalizing. we can't call into JS. while let Some(pair) = pending.read_item() { - // Note: `pair.promise.deinit()` — JSPromiseStrong drops automatically. drop(pair); } while let Some(offline_cmd) = commands.read_item() { - // Note: `offline_cmd.promise.deinit()` / `offline_cmd.deinit()` — - // JSPromiseStrong / Box<[u8]> drop automatically. drop(offline_cmd); } } - // Note: `allocator.free(connection_strings)` and `write_buffer/read_buffer.deinit()` - // and `tls.deinit()` are handled by Drop on the owning fields. Only the side-effecting - // unregister remains explicit. drop(pending); drop(commands); self.unregister_auto_flusher(); } // ** Auto-pipelining ** - fn register_auto_flusher(&mut self, vm: &VirtualMachine) { - if !self.auto_flusher.registered.get() { - AutoFlusher::register_deferred_microtask_with_type_unchecked::(self, vm); - self.auto_flusher.registered.set(true); - } + fn register_auto_flusher(&mut self) { + AutoFlusher::register_deferred_microtask_with_type::(self, self.vm); } fn unregister_auto_flusher(&mut self) { - if self.auto_flusher.registered.get() { - AutoFlusher::unregister_deferred_microtask_with_type::(self, self.vm); - self.auto_flusher.registered.set(false); - } + AutoFlusher::unregister_deferred_microtask_with_type::(self, self.vm); } // Drain auto-pipelined commands @@ -417,17 +381,17 @@ impl ValkeyClient { return false; } - self.ref_(); + let _guard = self.parent().ref_scope(); // Start draining the command queue let mut total_bytelength: usize = 0; // We compute the count first, then drain by `read_item`. let pipelineable_count: usize = { - let to_process = self.queue.readable_slice(0); let mut total: usize = 0; - for command in to_process { + for command in self.queue.iter() { if !command + .promise .meta .contains(command::Meta::SUPPORTS_AUTO_PIPELINING) { @@ -444,27 +408,19 @@ impl ValkeyClient { .ensure_unused_capacity(total_bytelength); for _ in 0..pipelineable_count { let cmd = self.queue.read_item().expect("count was precomputed"); - self.in_flight - .write_item(command::PromisePair { - meta: cmd.meta, - promise: cmd.promise, - }) - .unwrap_or_oom(); + self.in_flight.write_item(cmd.promise).unwrap_or_oom(); self.write_buffer .write(&cmd.serialized_data) .unwrap_or_oom(); - // Free the serialized data since we've copied it to the write buffer - // Note: `allocator.free(command.serialized_data)` — Box<[u8]> drops here. } - let _ = self.flush_data(); + self.flush_data(); - let have_more = self.queue.readable_length() > 0; + // Stay registered only if this pass actually drained something and more + // remains; otherwise `send_next_command` / `enqueue` will re-register + // when the queue becomes drainable again. + let have_more = pipelineable_count > 0 && self.queue.readable_length() > 0; self.auto_flusher.registered.set(have_more); - - self.deref(); - - // Return true if we should schedule another flush have_more } // ** End of auto-pipelining ** @@ -475,7 +431,7 @@ impl ValkeyClient { return 0; } match self.status { - Status::Connected => self.idle_timeout_interval_ms, + Status::Connected => self.idle_timeout_ms, _ => self.connection_timeout_ms, } } @@ -512,31 +468,34 @@ impl ValkeyClient { delay } - /// Reject all pending commands with an error + /// Reject all pending commands with an error. Both queues are drained + /// unconditionally; the first error (if any) is returned afterwards so no + /// promise is left forever-pending on an early `?` bailout. fn reject_all_pending_commands( - pending_ptr: &mut command::promise_pair::Queue, - entries_ptr: &mut command::entry::Queue, + pending_ptr: &mut command::PromiseQueue, + entries_ptr: &mut command::EntryQueue, global_this: &JSGlobalObject, jsvalue: JSValue, - ) -> JsTerminated<()> { - let mut pending = core::mem::replace(pending_ptr, command::promise_pair::Queue::init()); - let mut entries = core::mem::replace(entries_ptr, command::entry::Queue::init()); - // Note: `defer pending.deinit()` / `defer entries.deinit()` — handled by Drop. - - // Reject commands in the command queue - while let Some(mut command_pair) = pending.read_item() { - command_pair.reject_command(global_this, jsvalue)?; + ) -> JsResult<()> { + let mut pending = core::mem::replace(pending_ptr, command::PromiseQueue::init()); + let mut entries = core::mem::replace(entries_ptr, command::EntryQueue::init()); + let mut first_err: JsResult<()> = Ok(()); + + while let Some(mut promise) = pending.read_item() { + if let Err(e) = promise.reject(global_this, jsvalue) { + first_err = first_err.and(Err(e.into())); + } } - // Reject commands in the offline queue while let Some(mut cmd) = entries.read_item() { - // Note: `defer cmd.deinit(allocator)` — Entry should impl Drop. - cmd.promise.reject(global_this, Ok(jsvalue))?; + if let Err(e) = cmd.promise.reject(global_this, jsvalue) { + first_err = first_err.and(Err(e.into())); + } } - Ok(()) + first_err } - fn reject_in_flight_commands(&mut self, message: &[u8], err: RedisError) -> JsTerminated<()> { + fn reject_in_flight_commands(&mut self, message: &[u8], err: RedisError) -> JsResult<()> { if self.in_flight.readable_length() == 0 { return Ok(()); } @@ -547,11 +506,8 @@ impl ValkeyClient { message: Box::<[u8]>::from(message), err, global_this: GlobalRef::from(vm.global()), - in_flight: core::mem::replace( - &mut self.in_flight, - command::promise_pair::Queue::init(), - ), - queue: command::entry::Queue::init(), + in_flight: core::mem::replace(&mut self.in_flight, command::PromiseQueue::init()), + queue: command::EntryQueue::init(), }); deferred_failure.enqueue(); return Ok(()); @@ -559,26 +515,25 @@ impl ValkeyClient { let global_this = self.global_object(); let jsvalue = valkey_error_to_js(&global_this, message, err); - let mut entries = command::entry::Queue::init(); + let mut entries = command::EntryQueue::init(); Self::reject_all_pending_commands(&mut self.in_flight, &mut entries, &global_this, jsvalue) } /// Flush pending data to the socket - pub fn flush_data(&mut self) -> bool { + pub fn flush_data(&mut self) { let chunk = self.write_buffer.remaining(); if chunk.is_empty() { - return false; + return; } let wrote = self.socket.write(chunk); if wrote > 0 { self.write_buffer .consume(u32::try_from(wrote).expect("int cast")); } - self.write_buffer.len() > 0 } /// Mark the connection as failed with error message - pub fn fail(&mut self, message: &[u8], err: RedisError) -> JsTerminated<()> { + pub fn fail(&mut self, message: &[u8], err: RedisError) -> JsResult<()> { debug!("failed: {}: {:?}", bstr::BStr::new(message), err); if self.flags.failed { return Ok(()); @@ -596,9 +551,9 @@ impl ValkeyClient { global_this: GlobalRef::from(vm.global()), in_flight: core::mem::replace( &mut self.in_flight, - command::promise_pair::Queue::init(), + command::PromiseQueue::init(), ), - queue: core::mem::replace(&mut self.queue, command::entry::Queue::init()), + queue: core::mem::replace(&mut self.queue, command::EntryQueue::init()), }); deferred_failure.enqueue(); } @@ -615,7 +570,7 @@ impl ValkeyClient { &mut self, global_this: &JSGlobalObject, jsvalue: JSValue, - ) -> JsTerminated<()> { + ) -> JsResult<()> { if self.flags.failed { return Ok(()); } @@ -657,101 +612,132 @@ impl ValkeyClient { socket.close(uws::CloseCode::Normal); if is_semi_socket { self.status = Status::Disconnected; - let _ = self.on_close(); + if let Err(e) = self.on_close() { + self.global_object().report_active_exception_as_unhandled(e); + } } } /// Handle connection closed event - pub fn on_close(&mut self) -> JsTerminated<()> { - self.unregister_auto_flusher(); - self.write_buffer.clear_and_free(); - - // If manually closing, don't attempt to reconnect - if self.flags.is_manually_closed { - debug!("skip reconnecting since the connection is manually closed"); - self.fail(b"Connection closed", RedisError::ConnectionClosed)?; - self.on_valkey_close()?; - return Ok(()); - } - - // If auto reconnect is disabled, just fail - if !self.flags.enable_auto_reconnect { - debug!("skip reconnecting since auto reconnect is disabled"); - self.fail(b"Connection closed", RedisError::ConnectionClosed)?; - self.on_valkey_close()?; - return Ok(()); - } - - // Calculate reconnection delay with exponential backoff - self.retry_attempts += 1; - let delay_ms = self.get_reconnect_delay(); + pub fn on_close(&mut self) -> JsResult<()> { + self.on_socket_closed(b"Connection closed") + } - if delay_ms == 0 || self.retry_attempts > self.max_retries { - debug!("Max retries reached or retry strategy returned 0, giving up reconnection"); - self.fail( - b"Max reconnection attempts reached", - RedisError::ConnectionClosed, - )?; - self.on_valkey_close()?; - return Ok(()); + /// Connect-error path: like [`on_close`](Self::on_close) but surfaces the OS + /// errno (e.g. `ECONNREFUSED`) to the rejected promises instead of a + /// generic "Connection closed". + pub fn on_connect_error(&mut self, errno: i32) -> JsResult<()> { + use std::io::Write; + let mut buf = [0u8; 128]; + let mut cursor = &mut buf[..]; + let start = cursor.len(); + if errno > 0 { + let name: &'static str = bun_errno::from_errno(errno).into(); + let _ = write!(&mut cursor, "connect {}", name); + } else { + let _ = write!(&mut cursor, "connect failed (errno {})", errno); } + let written = start - cursor.len(); + self.on_socket_closed(&buf[..written]) + } - debug!( - "reconnect in {}ms (attempt {}/{})", - delay_ms, self.retry_attempts, self.max_retries - ); + /// Shared body of [`on_close`](Self::on_close) / [`on_connect_error`](Self::on_connect_error): + /// runs the reconnect decision and rejects pending commands with `msg`. + fn on_socket_closed(&mut self, msg: &[u8]) -> JsResult<()> { + self.unregister_auto_flusher(); + self.write_buffer.clear_and_free(); - self.flags.is_reconnecting = true; - self.flags.is_authenticated = false; - self.flags.is_selecting_db_internal = false; + // Decide the outcome first. `fail()` / `reject_in_flight_commands()` may + // return Err, but the parent callback that releases connect()'s + // socket-ref must run regardless or the `JSValkeyClient` leaks. + let auto_reconnect = !self.flags.is_manually_closed && self.flags.enable_auto_reconnect; + let will_reconnect = if auto_reconnect { + self.retry_attempts += 1; + self.retry_attempts <= self.max_retries + } else { + false + }; - self.reject_in_flight_commands(b"Connection closed", RedisError::ConnectionClosed)?; + let fail_result: JsResult<()> = if will_reconnect { + debug!( + "reconnect (attempt {}/{})", + self.retry_attempts, self.max_retries + ); + self.flags.is_reconnecting = true; + self.handshake = Handshake::AwaitingHello; + self.reject_in_flight_commands(msg, RedisError::ConnectionClosed) + } else { + let msg: &[u8] = if !auto_reconnect { + debug!("skip reconnecting (manually closed or auto-reconnect disabled)"); + msg + } else { + debug!("Max retries reached, giving up reconnection"); + b"Max reconnection attempts reached" + }; + self.fail(msg, RedisError::ConnectionClosed) + }; - // Signal reconnect timer should be started - self.on_valkey_reconnect(); - Ok(()) + if will_reconnect { + self.parent().on_valkey_reconnect(); + fail_result + } else { + let close_result = self.parent().on_valkey_close(); + fail_result?; + close_result + } } pub fn send_next_command(&mut self) { if self.write_buffer.remaining().is_empty() && self.connection_ready() { if self.queue.readable_length() > 0 { // Check the command at the head of the queue - let flags = self.queue.readable_slice(0)[0].meta; + let flags = self.queue.readable_slice(0)[0].promise.meta; if !flags.contains(command::Meta::SUPPORTS_AUTO_PIPELINING) { // Head is non-pipelineable. Try to drain it serially if nothing is in-flight. if self.in_flight.readable_length() == 0 { - let _ = self.drain(); // Send the single non-pipelineable command + self.drain(); // Send the single non-pipelineable command // After draining, check if the *new* head is pipelineable and schedule flush if needed. // This covers sequences like NON_PIPE -> PIPE -> PIPE ... if self.queue.readable_length() > 0 && self.queue.readable_slice(0)[0] + .promise .meta .contains(command::Meta::SUPPORTS_AUTO_PIPELINING) { - self.register_auto_flusher(self.vm); + self.register_auto_flusher(); } } else { // Non-pipelineable command is blocked by in-flight commands. Do nothing, wait for in-flight to finish. } } else { // Head is pipelineable. Register the flusher to batch it with others. - self.register_auto_flusher(self.vm); + self.register_auto_flusher(); } - } else if self.in_flight.readable_length() == 0 { - // Without auto pipelining, wait for in-flight to empty before draining - let _ = self.drain(); } } - let _ = self.flush_data(); + self.flush_data(); + } + + /// Shared tail of both `on_data` parse loops: hand a fully-parsed reply to + /// `handle_response`, then drive the next command if the connection is + /// still live. Returns `false` when the caller should stop looping. + fn dispatch_reply(&mut self, value: RESPValue) -> JsResult { + let mut value = value; + self.handle_response(&mut value)?; + if self.status == Status::Disconnected || self.flags.failed { + return Ok(false); + } + self.send_next_command(); + Ok(true) } /// Process data received from socket /// /// Caller refs / derefs. - pub fn on_data(&mut self, data: &[u8]) -> JsTerminated<()> { + pub fn on_data(&mut self, data: &[u8]) -> JsResult<()> { debug!( "Low-level onData called with {} bytes: {}", data.len(), @@ -759,9 +745,7 @@ impl ValkeyClient { ); // Path 1: Buffer already has data, append and process from buffer if !self.read_buffer.remaining().is_empty() { - self.read_buffer - .write(data) - .expect("failed to write to read buffer"); + self.read_buffer.write(data).unwrap_or_oom(); // Process as many complete messages from the buffer as possible loop { @@ -788,30 +772,32 @@ impl ValkeyClient { return Ok(()); } Err(err) => { - self.fail(b"Failed to read data (buffer path)", err)?; + self.fail(b"Failed to parse server response", err)?; return Ok(()); } } let mut reader = protocol::ValkeyReader::init(remaining_buffer); - let before_read_pos = reader_pos(&reader); + let before_read_pos = reader.pos(); let value = match reader.read_value() { - Ok(v) => v, + Ok(Some(v)) => v, + Ok(None) => { + // Scanner said Complete but the tree parser ran out of + // bytes — the two disagree, which is a protocol error. + self.fail(b"Parser/scanner mismatch", RedisError::InvalidResponse)?; + return Ok(()); + } Err(err) => { - // The scanner verified a complete reply is buffered, so - // a parse failure here (including `InvalidResponse`) is - // a protocol error, not a short read. - self.fail(b"Failed to read data (buffer path)", err)?; + self.fail(b"Failed to parse server response", err)?; return Ok(()); } }; - // Note: `defer value.deinit(allocator)` — RESPValue should impl Drop. - let bytes_consumed = reader_pos(&reader) - before_read_pos; + let bytes_consumed = reader.pos() - before_read_pos; if bytes_consumed == 0 && !remaining_buffer.is_empty() { self.fail( - b"Parser consumed 0 bytes unexpectedly (buffer path)", + b"Parser consumed 0 bytes unexpectedly", RedisError::InvalidResponse, )?; return Ok(()); @@ -821,13 +807,9 @@ impl ValkeyClient { .consume(u32::try_from(bytes_consumed).expect("int cast")); self.reply_scanner.reset(); - let mut value_to_handle = value; // Use temp var for defer - self.handle_response(&mut value_to_handle)?; - - if self.status == Status::Disconnected || self.flags.failed { + if !self.dispatch_reply(value)? { return Ok(()); } - self.send_next_command(); } return Ok(()); // Finished processing buffered data for now } @@ -836,41 +818,37 @@ impl ValkeyClient { let mut current_data_slice = data; // Create a mutable view of the incoming data while !current_data_slice.is_empty() { let mut reader = protocol::ValkeyReader::init(current_data_slice); - let before_read_pos = reader_pos(&reader); + let before_read_pos = reader.pos(); let value = match reader.read_value() { - Ok(v) => v, - Err(err) => { - if err == RedisError::InvalidResponse { - // Partial message encountered on the stack-allocated path. - // Copy the *remaining* part of the stack data to the heap buffer - // and wait for more data. - if cfg!(debug_assertions) { - debug!( - "read_buffer: partial message on stack ({} bytes), switching to buffer", - current_data_slice.len() - before_read_pos - ); - } - self.reply_scanner.reset(); - self.read_buffer - .write(¤t_data_slice[before_read_pos..]) - .expect("failed to write remaining stack data to buffer"); - return Ok(()); // Exit onData, next call will use the buffer path - } else { - // Any other error is fatal - self.fail(b"Failed to read data (stack path)", err)?; - return Ok(()); + Ok(Some(v)) => v, + Ok(None) => { + // Partial message on the stack-allocated path — copy the + // remainder to the heap buffer and wait for more data. + if cfg!(debug_assertions) { + debug!( + "read_buffer: partial message on stack ({} bytes), switching to buffer", + current_data_slice.len() - before_read_pos + ); } + self.reply_scanner.reset(); + self.read_buffer + .write(¤t_data_slice[before_read_pos..]) + .unwrap_or_oom(); + return Ok(()); + } + Err(err) => { + self.fail(b"Failed to parse server response", err)?; + return Ok(()); } }; // Successfully read a full message from the stack data - // Note: `defer value.deinit(allocator)` — RESPValue should impl Drop. - let bytes_consumed = reader_pos(&reader) - before_read_pos; + let bytes_consumed = reader.pos() - before_read_pos; if bytes_consumed == 0 { // This case should ideally not happen if readValue succeeded and slice wasn't empty self.fail( - b"Parser consumed 0 bytes unexpectedly (stack path)", + b"Parser consumed 0 bytes unexpectedly", RedisError::InvalidResponse, )?; return Ok(()); @@ -879,18 +857,9 @@ impl ValkeyClient { // Advance the view into the stack data slice for the next iteration current_data_slice = ¤t_data_slice[bytes_consumed..]; - // Handle the successfully parsed response - let mut value_to_handle = value; // Use temp var for defer - self.handle_response(&mut value_to_handle)?; - - // Check connection status after handling - if self.status == Status::Disconnected || self.flags.failed { + if !self.dispatch_reply(value)? { return Ok(()); } - - // After handling a response, try to send the next command - self.send_next_command(); - // Loop continues with the remainder of current_data_slice } @@ -898,106 +867,81 @@ impl ValkeyClient { Ok(()) } - /// Try handling this response as a subscriber-state response. - /// Returns `handled` if we handled it, `fallthrough` if we did not. + /// Handle a subscriber-state push response whose kind is a recognised + /// `SubscriptionPushMessage`. fn handle_subscribe_response( &mut self, - value: &mut RESPValue, - pair: Option<&mut command::PromisePair>, - ) -> JsResult { - // Resolve the promise with the potentially transformed value + push: &mut protocol::Push, + kind: protocol::SubscriptionPushMessage, + pair: Option<&mut command::Promise>, + ) -> JsResult<()> { let global_this = self.global_object(); - debug!("Handling a subscribe response: {}", value); + debug!("Handling a subscribe response: {:?}", kind); // SAFETY: `event_loop()` returns the live VM-owned `*mut EventLoop`; the guard holds the // raw pointer (no long-lived `&mut`) and calls `exit()` on drop. let _exit = self.vm.enter_event_loop_scope(); - match value { - RESPValue::Error(_) => { - if let Some(p) = pair { - p.promise - .reject(&global_this, resp_value_to_js(value, &global_this))?; - } - Ok(SubscribeHandled::Handled) - } - RESPValue::Push(push) => { - let p = self.parent(); - let sub_count = p - ._subscription_ctx - .get() - .channels_subscribed_to_count(&global_this)?; - - if let Some(msg_type) = protocol::SubscriptionPushMessage::from_bytes(&push.kind) { - match msg_type { - protocol::SubscriptionPushMessage::Message => { - self.on_valkey_message(&mut push.data); - Ok(SubscribeHandled::Handled) - } - protocol::SubscriptionPushMessage::Subscribe => { - p.add_subscription(); - self.on_valkey_subscribe(value); - - // For SUBSCRIBE responses, only resolve the promise for the first channel confirmation - // Additional channel confirmations from multi-channel SUBSCRIBE commands don't need promise pairs - if let Some(req_pair) = pair { - req_pair.promise.promise.resolve( - &global_this, - JSValue::js_number(f64::from(sub_count)), - )?; - } - Ok(SubscribeHandled::Handled) - } - protocol::SubscriptionPushMessage::Unsubscribe => { - self.on_valkey_unsubscribe()?; - self.parent().remove_subscription(); - - // For UNSUBSCRIBE responses, only resolve the promise if we have one - // Additional channel confirmations from multi-channel UNSUBSCRIBE commands don't need promise pairs - if let Some(req_pair) = pair { - req_pair - .promise - .promise - .resolve(&global_this, JSValue::UNDEFINED)?; - } - Ok(SubscribeHandled::Handled) - } - } + if kind.is_message() { + // RESP3 `pmessage` data is [pattern, channel, payload]; skip the + // leading pattern so `on_valkey_message` sees [channel, payload]. + let data: &mut [RESPValue] = + if matches!(kind, protocol::SubscriptionPushMessage::PMessage) + && !push.data.is_empty() + { + &mut push.data[1..] } else { - // We should rarely reach this point. If we're guaranteed to be handling a subscribe/unsubscribe, - // then this is an unexpected path. - bun_core::hint::cold(); - self.fail( - b"Push message is not a subscription message.", - RedisError::InvalidResponseType, - )?; - Ok(SubscribeHandled::Handled) - } + &mut push.data + }; + self.parent().on_valkey_message(data); + return Ok(()); + } + if kind.is_subscribe_ack() { + let p = self.parent(); + let sub_count = p + .subscription_ctx + .get() + .channels_subscribed_to_count(&global_this); + // Only `.subscribe(ch, handler)` wires the handler map; enter + // subscriber mode iff a handler is actually registered, so raw + // `send('SUBSCRIBE'/'PSUBSCRIBE'/'SSUBSCRIBE', ...)` never does. + if sub_count > 0 { + p.add_subscription(); + self.parent().on_valkey_subscribe(); } - _ => { - // This may be a regular command response. Let's pass it down - // to the next handler. - Ok(SubscribeHandled::Fallthrough) + + // For SUBSCRIBE responses, only resolve the promise for the first channel confirmation + // Additional channel confirmations from multi-channel SUBSCRIBE commands don't need promise pairs + if let Some(req_pair) = pair { + req_pair + .promise + .resolve(&global_this, JSValue::js_number(f64::from(sub_count)))?; } + return Ok(()); } + debug_assert!(kind.is_unsubscribe_ack()); + self.parent().on_valkey_unsubscribe(); + self.parent().remove_subscription(); + + // For UNSUBSCRIBE responses, only resolve the promise if we have one + // Additional channel confirmations from multi-channel UNSUBSCRIBE commands don't need promise pairs + if let Some(req_pair) = pair { + req_pair.promise.resolve(&global_this, JSValue::UNDEFINED)?; + } + Ok(()) } - fn handle_hello_response(&mut self, value: &mut RESPValue) -> JsTerminated<()> { + fn handle_hello_response(&mut self, value: &mut RESPValue) -> JsResult<()> { debug!("Processing HELLO response"); + if let Some(err) = value.as_server_error() { + self.fail(err, RedisError::AuthenticationFailed)?; + return Ok(()); + } match value { - RESPValue::Error(err) => { - self.fail(err, RedisError::AuthenticationFailed)?; - Ok(()) - } RESPValue::SimpleString(str_) => { if str_.as_ref() == b"OK" { - self.status = Status::Connected; - self.flags.is_authenticated = true; - self.flags.is_reconnecting = false; - self.retry_attempts = 0; - self.on_valkey_connect(value)?; - return Ok(()); + return self.mark_connected(value); } self.fail( b"Authentication failed (unexpected response)", @@ -1032,12 +976,7 @@ impl ValkeyClient { } // Authentication successful via HELLO - self.status = Status::Connected; - self.flags.is_authenticated = true; - self.flags.is_reconnecting = false; - self.retry_attempts = 0; - self.on_valkey_connect(value)?; - Ok(()) + self.mark_connected(value) } _ => { self.fail( @@ -1050,9 +989,9 @@ impl ValkeyClient { } /// Handle Valkey protocol response - fn handle_response(&mut self, value: &mut RESPValue) -> JsTerminated<()> { + fn handle_response(&mut self, value: &mut RESPValue) -> JsResult<()> { // Special handling for the initial HELLO response - if !self.flags.is_authenticated { + if self.handshake == Handshake::AwaitingHello { self.handle_hello_response(value)?; // We've handled the HELLO response without consuming anything from the command queue @@ -1060,14 +999,14 @@ impl ValkeyClient { } // Handle initial SELECT response - if self.flags.is_selecting_db_internal { - self.flags.is_selecting_db_internal = false; + if self.handshake == Handshake::SelectingDb { + self.handshake = Handshake::Ready; + if let Some(err_str) = value.as_server_error() { + self.fail(err_str, RedisError::InvalidCommand)?; + return Ok(()); + } return match value { - RESPValue::Error(err_str) => { - self.fail(err_str, RedisError::InvalidCommand)?; - Ok(()) - } RESPValue::SimpleString(ok_str) => { if ok_str.as_ref() != b"OK" { // SELECT returned something other than "OK" @@ -1097,28 +1036,27 @@ impl ValkeyClient { } // Check if this is a subscription push message that might not need a promise pair let mut should_consume_promise_pair = true; - let mut pair_maybe: Option = None; + let mut pair_maybe: Option = None; // For subscription clients, check if this is a push message that doesn't need a promise pair if let RESPValue::Push(push) = value { match protocol::SubscriptionPushMessage::from_bytes(&push.kind) { - Some(protocol::SubscriptionPushMessage::Message) => { + Some(k) if k.is_message() => { // Message pushes never need promise pairs should_consume_promise_pair = false; } - Some( - protocol::SubscriptionPushMessage::Subscribe - | protocol::SubscriptionPushMessage::Unsubscribe, - ) => { - // Subscribe/unsubscribe pushes only need promise pairs if we have pending commands - if self.in_flight.readable_length() == 0 { - should_consume_promise_pair = false; - } + Some(_) => { + // A subscribe/unsubscribe ack only pairs with a promise that was + // itself a SUBSCRIBE/UNSUBSCRIBE request; multi-channel SUBSCRIBE + // yields N acks for one promise, and other commands may be in flight. + should_consume_promise_pair = self + .in_flight + .readable_slice(0) + .first() + .is_some_and(|p| p.meta.contains(command::Meta::SUBSCRIPTION_REQUEST)); } None => { - if !protocol::SubscriptionPushMessage::is_reply_kind(&push.kind) { - should_consume_promise_pair = false; - } + should_consume_promise_pair = false; } } } @@ -1140,26 +1078,31 @@ impl ValkeyClient { if self.parent().is_subscriber() || request_is_subscribe { debug!("This client is a subscriber. Handling as subscriber..."); - match value { - RESPValue::Error(err) => { - self.fail(err, RedisError::InvalidResponse)?; - return Ok(()); + if let Some(err) = value.as_server_error() { + if let Some(mut p) = pair_maybe.take() { + let global_this = self.global_object(); + let js = valkey_error_to_js(&global_this, err, RedisError::ServerError); + p.reject(&global_this, js)?; + } else { + debug!( + "subscriber: server error without pending promise: {}", + bstr::BStr::new(err) + ); } + return Ok(()); + } + match value { RESPValue::Push(push) => { - if protocol::SubscriptionPushMessage::from_bytes(&push.kind).is_some() { - if self.handle_subscribe_response(value, pair_maybe.as_mut())? - == SubscribeHandled::Handled - { - return Ok(()); - } - } else { - bun_core::hint::cold(); - self.fail( - b"Unexpected push message kind without promise", - RedisError::InvalidResponseType, - )?; + if let Some(kind) = protocol::SubscriptionPushMessage::from_bytes(&push.kind) { + self.handle_subscribe_response(push, kind, pair_maybe.as_mut())?; return Ok(()); } + bun_core::hint::cold(); + debug!( + "subscriber: ignoring push kind {:?}", + bstr::BStr::new(&push.kind) + ); + return Ok(()); } _ => { // In the else case, we fall through to the regular @@ -1176,36 +1119,22 @@ impl ValkeyClient { return Ok(()); }; - let meta = pair.meta; - - // Handle the response based on command type - if meta.contains(command::Meta::RETURN_AS_BOOL) { - // EXISTS returns 1 if key exists, 0 if not - we convert to boolean - if let RESPValue::Integer(int_value) = *value { - *value = RESPValue::Boolean(int_value > 0); - } - } - - // Resolve the promise with the potentially transformed value - let promise_ptr = &mut pair.promise; let global_this = self.global_object(); let _exit = self.vm.enter_event_loop_scope(); - if matches!(value, RESPValue::Error(_)) { - let js_err = match resp_value_to_js(value, &global_this) { - Ok(v) => v, - Err(err) => global_this.take_error(err), - }; - promise_ptr.reject(&global_this, Ok(js_err))?; + let value = core::mem::replace(value, RESPValue::Null); + if let Some(msg) = value.as_server_error() { + let js_err = valkey_error_to_js(&global_this, msg, RedisError::ServerError); + pair.reject(&global_this, js_err)?; } else { - promise_ptr.resolve(&global_this, value)?; + pair.resolve(&global_this, value)?; } Ok(()) } /// Send authentication command to Valkey server - fn authenticate(&mut self) -> JsTerminated<()> { + fn authenticate(&mut self) -> JsResult<()> { // First send HELLO command for RESP3 protocol debug!("Sending HELLO 3 command"); @@ -1258,17 +1187,16 @@ impl ValkeyClient { args: Args::Raw(&[db_str]), meta: command::Meta::default(), }; - if let Err(_err) = select_cmd.write(self.writer()) { + if let Err(_err) = select_cmd.write(&mut self.writer()) { self.fail(b"Failed to write SELECT command", RedisError::OutOfMemory)?; return Ok(()); } - self.flags.is_selecting_db_internal = true; } Ok(()) } /// Handle socket open event - pub fn on_open(&mut self, socket: AnySocket) -> JsTerminated<()> { + pub fn on_open(&mut self, socket: AnySocket) -> JsResult<()> { self.socket = socket; self.write_buffer.clear_and_free(); self.read_buffer.clear_and_free(); @@ -1276,12 +1204,11 @@ impl ValkeyClient { // A fresh socket has opened, so reset per-connection state. Without // this, `send()` would permanently reject with "Connection has failed" // after a previous connection exhausted retries (#29925), and the - // new HELLO response would be dropped because `is_authenticated` was - // still set from a prior successful handshake — blocking the client + // new HELLO response would be dropped because the handshake was still + // `Ready` from a prior successful connection — blocking the client // from ever transitioning back to `.connected`. self.flags.failed = false; - self.flags.is_authenticated = false; - self.flags.is_selecting_db_internal = false; + self.handshake = Handshake::AwaitingHello; if matches!(self.socket, AnySocket::SocketTcp(_)) { // if is tcp, we need to start the connection process // if is tls, we need to wait for the handshake to complete @@ -1291,50 +1218,58 @@ impl ValkeyClient { } /// Start the connection process - pub fn start(&mut self) -> JsTerminated<()> { + pub fn start(&mut self) -> JsResult<()> { self.authenticate()?; - let _ = self.flush_data(); + self.flush_data(); Ok(()) } /// Test whether we are ready to run "normal" RESP commands, such as /// get/set, pub/sub, etc. fn connection_ready(&self) -> bool { - self.flags.is_authenticated && !self.flags.is_selecting_db_internal + self.status == Status::Connected && self.handshake == Handshake::Ready + } + + fn mark_connected(&mut self, hello: &mut RESPValue) -> JsResult<()> { + self.status = Status::Connected; + self.handshake = if self.database > 0 { + Handshake::SelectingDb + } else { + Handshake::Ready + }; + self.flags.is_reconnecting = false; + self.retry_attempts = 0; + self.parent().on_valkey_connect(hello) } /// Process queued commands in the offline queue - pub fn drain(&mut self) -> bool { + pub fn drain(&mut self) { // If there's something in the in-flight queue and the next command // doesn't support pipelining, we should wait for in-flight commands to complete if self.in_flight.readable_length() > 0 { let queue_slice = self.queue.readable_slice(0); if !queue_slice.is_empty() && !queue_slice[0] + .promise .meta .contains(command::Meta::SUPPORTS_AUTO_PIPELINING) { - return false; + return; } } let Some(offline_cmd) = self.queue.read_item() else { - return false; + return; }; // Add the promise to the command queue first self.in_flight - .write_item(command::PromisePair { - meta: offline_cmd.meta, - promise: offline_cmd.promise, - }) + .write_item(offline_cmd.promise) .unwrap_or_oom(); let data = offline_cmd.serialized_data; if self.connection_ready() && self.write_buffer.remaining().is_empty() { // Optimization: avoid cloning the data an extra time. - // Note: `defer allocator.free(data)` — `data: Box<[u8]>` drops at scope end. - let wrote = self.socket.write(&data); let unwritten = &data[usize::try_from(wrote.max(0)).expect("int cast")..]; @@ -1343,83 +1278,61 @@ impl ValkeyClient { self.write_buffer.write(unwritten).unwrap_or_oom(); } - return true; + return; } // Write the pre-serialized data directly to the output buffer - let _ = self.write(&data).unwrap_or_oom(); - // Note: `bun.default_allocator.free(data)` — Box<[u8]> drops here. - - true + self.write_buffer.write(&data).unwrap_or_oom(); } pub fn on_writable(&mut self) { - self.ref_(); + let _guard = self.parent().ref_scope(); self.send_next_command(); - self.deref(); } fn enqueue( &mut self, command: &Command, mut promise: command::Promise, - ) -> Result<(), crate::Error> { + ) -> Result<(), RedisError> { let can_pipeline = command .meta .contains(command::Meta::SUPPORTS_AUTO_PIPELINING) && self.flags.enable_auto_pipelining; - // For commands that don't support pipelining, we need to wait for the queue to drain completely - // before sending the command. This ensures proper order of execution for state-changing commands. - let must_wait_for_queue = !command - .meta - .contains(command::Meta::SUPPORTS_AUTO_PIPELINING) - && self.queue.readable_length() > 0; - - if - // If there are any pending commands, queue this one - self.queue.readable_length() > 0 - // With auto pipelining, we can accept commands regardless of in_flight commands - || (!can_pipeline && self.in_flight.readable_length() > 0) - // We need authentication before processing commands - || !self.connection_ready() - // Commands that don't support pipelining must wait for the entire queue to drain - || must_wait_for_queue - // If can pipeline, we can accept commands regardless of in_flight commands - || can_pipeline - { + let write_now = !can_pipeline + && self.connection_ready() + && self.queue.readable_length() == 0 + && self.in_flight.readable_length() == 0; + + if !write_now { // We serialize the bytes in here, so we don't need to worry about the lifetime of the Command itself. let entry = command::Entry::create(command, promise)?; - self.queue.write_item(entry)?; + self.queue + .write_item(entry) + .map_err(|_| RedisError::OutOfMemory)?; // If we're connected and using auto pipelining, schedule a flush if self.status == Status::Connected && can_pipeline { - self.register_auto_flusher(self.vm); + self.register_auto_flusher(); } return Ok(()); } - match self.status { - Status::Connecting | Status::Connected => { - if command.write(self.writer()).is_err() { - let global = self.global_object(); - let _ = promise.reject(&global, Ok(global.create_out_of_memory_error())); - return Ok(()); - } - } - _ => unreachable!(), + debug_assert!(self.status == Status::Connected); + if command.write(&mut self.writer()).is_err() { + let global = self.global_object(); + let _ = promise.reject(&global, global.create_out_of_memory_error()); + return Ok(()); } - let cmd_pair = command::PromisePair { - meta: command.meta, - promise, - }; - // Add to queue with command type - self.in_flight.write_item(cmd_pair)?; + self.in_flight + .write_item(promise) + .map_err(|_| RedisError::OutOfMemory)?; - let _ = self.flush_data(); + self.flush_data(); Ok(()) } @@ -1427,10 +1340,10 @@ impl ValkeyClient { &mut self, global_this: &JSGlobalObject, command: &Command, - ) -> Result<*mut JSPromise, crate::Error> { + ) -> Result<*mut JSPromise, RedisError> { // FIX: Check meta before using it for routing decisions let mut checked_command = *command; - checked_command.meta = command.meta.check(command); + checked_command.meta = command.meta.check(command.command); let mut promise = command::Promise::create(global_this, checked_command.meta); @@ -1438,29 +1351,18 @@ impl ValkeyClient { if self.flags.failed { let _ = promise.reject( global_this, - Ok(global_this + global_this .err( bun_jsc::ErrorCode::REDIS_CONNECTION_CLOSED, format_args!("Connection has failed"), ) - .to_js()), + .to_js(), ); } else { // Handle disconnected state with offline queue match self.status { Status::Connected => { self.enqueue(&checked_command, promise)?; - - // Schedule auto-flushing to process this command if pipelining is enabled - if self.flags.enable_auto_pipelining - && checked_command - .meta - .contains(command::Meta::SUPPORTS_AUTO_PIPELINING) - && self.status == Status::Connected - && self.queue.readable_length() > 0 - { - self.register_auto_flusher(self.vm); - } } Status::Connecting | Status::Disconnected => { // Only queue if offline queue is enabled @@ -1469,14 +1371,14 @@ impl ValkeyClient { } else { let _ = promise.reject( global_this, - Ok(global_this + global_this .err( bun_jsc::ErrorCode::REDIS_CONNECTION_CLOSED, format_args!( "Connection is closed and offline queue is disabled" ), ) - .to_js()), + .to_js(), ); } } @@ -1495,70 +1397,17 @@ impl ValkeyClient { } } - /// Get a writer for the connected socket - // ValkeyClient itself serves as the writer (see `write` below). - pub fn writer(&mut self) -> &mut Self { - self - } - - /// Write data to the socket buffer - fn write(&mut self, data: &[u8]) -> Result { - self.write_buffer - .write(data) - .map_err(|_| RedisError::OutOfMemory)?; - Ok(data.len()) - } - - /// Increment reference count - pub fn ref_(&mut self) { - self.parent().ref_(); - } - - pub fn deref(&mut self) { - let parent = std::ptr::from_ref(self.parent()).cast_mut(); - // SAFETY: only called in balanced `ref_()`/`deref()` pairs - // (`on_auto_flush`, `on_writable`), so the count stays > 0 and the - // outer `&mut self` protector is never invalidated by deallocation. - unsafe { JSValkeyClient::deref(parent) }; + /// Get a writer targeting the outgoing write buffer. + pub(crate) fn writer(&mut self) -> WriteBufWriter<'_> { + WriteBufWriter(&mut self.write_buffer) } #[inline] - fn global_object(&mut self) -> GlobalRef { + fn global_object(&self) -> GlobalRef { self.parent().global_object } - - pub fn on_valkey_connect(&mut self, value: &mut RESPValue) -> JsTerminated<()> { - self.parent().on_valkey_connect(value) - } - - pub fn on_valkey_subscribe(&mut self, value: &mut RESPValue) { - self.parent().on_valkey_subscribe(value); - } - - pub fn on_valkey_unsubscribe(&mut self) -> JsResult<()> { - self.parent().on_valkey_unsubscribe() - } - - pub fn on_valkey_message(&mut self, value: &mut [RESPValue]) { - self.parent().on_valkey_message(value); - } - - pub fn on_valkey_reconnect(&mut self) { - self.parent().on_valkey_reconnect(); - } - - pub fn on_valkey_close(&mut self) -> JsTerminated<()> { - self.parent().on_valkey_close() - } - - pub fn on_valkey_timeout(&mut self) { - self.parent().on_valkey_timeout(); - } } -// Auto-pipelining -use crate::webcore::{AutoFlusher, HasAutoFlusher}; - impl HasAutoFlusher for ValkeyClient { #[inline] fn auto_flusher(&self) -> &AutoFlusher { @@ -1572,21 +1421,11 @@ impl HasAutoFlusher for ValkeyClient { } } -// `bun_io::Write` impl so `Command::write(self.writer())` type-checks. -impl bun_io::Write for ValkeyClient { - #[inline] - fn write_all(&mut self, buf: &[u8]) -> bun_io::Result<()> { - self.write_buffer - .write(buf) - .map_err(|_| bun_core::Error::Alloc(bun_alloc::AllocError)) - } -} - /// Newtype around `&mut OffsetByteList` so `Command::write` can target the /// write buffer directly when other `&self` field borrows (username/password) /// are still live — Rust's split-borrow rules permit `&self.username` + /// `&mut self.write_buffer`, but not `&self.username` + `&mut self`. -struct WriteBufWriter<'a>(&'a mut OffsetByteList); +pub(crate) struct WriteBufWriter<'a>(&'a mut OffsetByteList); impl bun_io::Write for WriteBufWriter<'_> { #[inline] @@ -1596,19 +1435,3 @@ impl bun_io::Write for WriteBufWriter<'_> { .map_err(|_| bun_core::Error::Alloc(bun_alloc::AllocError)) } } - -// Local extension trait providing `.unwrap_or_oom()` on `Result`. -// No shared `UnwrapOrOom` trait exists yet (bun_alloc has none); delegate to -// `bun_core::handle_oom` so every call site keeps its method-chain shape. -trait UnwrapOrOom { - type Output; - fn unwrap_or_oom(self) -> Self::Output; -} -impl UnwrapOrOom for core::result::Result { - type Output = T; - #[inline] - #[track_caller] - fn unwrap_or_oom(self) -> T { - bun_core::handle_oom(self) - } -} diff --git a/src/valkey/Cargo.toml b/src/valkey/Cargo.toml index 132b21038edc..8b577cdf9dcf 100644 --- a/src/valkey/Cargo.toml +++ b/src/valkey/Cargo.toml @@ -12,10 +12,4 @@ workspace = true [dependencies] strum.workspace = true bstr.workspace = true -scopeguard.workspace = true -const_format.workspace = true -enum-map.workspace = true -enumset.workspace = true -libc.workspace = true -bitflags.workspace = true bun_core.workspace = true diff --git a/src/valkey/lib.rs b/src/valkey/lib.rs index ef4795961a65..c9509753b303 100644 --- a/src/valkey/lib.rs +++ b/src/valkey/lib.rs @@ -1,3 +1,6 @@ -#![allow(non_snake_case, non_camel_case_types, non_upper_case_globals)] -#![warn(unused_must_use)] pub mod valkey_protocol; + +pub use valkey_protocol::{ + Attribute, MapEntry, Push, RESPValue, RedisError, ReplyScanner, ScanResult, + SubscriptionPushMessage, ValkeyReader, VerbatimString, +}; diff --git a/src/valkey/valkey_protocol.rs b/src/valkey/valkey_protocol.rs index b8887ec606e1..7d4c68255eed 100644 --- a/src/valkey/valkey_protocol.rs +++ b/src/valkey/valkey_protocol.rs @@ -5,29 +5,23 @@ use bstr::BStr; #[derive(strum::IntoStaticStr, strum::EnumString, Debug, Clone, Copy, PartialEq, Eq)] pub enum RedisError { AuthenticationFailed, + ServerError, ConnectionClosed, InvalidArgument, - InvalidArray, InvalidAttribute, - InvalidBigNumber, InvalidBlobError, InvalidBoolean, InvalidBulkString, InvalidCommand, InvalidDouble, - InvalidErrorString, InvalidInteger, InvalidMap, - InvalidNull, InvalidPush, InvalidResponse, InvalidResponseType, InvalidSet, - InvalidSimpleString, InvalidVerbatimString, - JSError, OutOfMemory, - JSTerminated, UnsupportedProtocol, ConnectionTimeout, IdleTimeout, @@ -37,20 +31,6 @@ pub enum RedisError { bun_core::impl_tag_error!(RedisError); -impl From for RedisError { - /// Reverse of the `RedisError → bun_core::Error` interning above so the - /// `JSValkeyClient::send` → `valkey_error_to_js` path round-trips through - /// `bun_core::Error` without losing the variant. - /// Unknown names collapse to `ConnectionClosed` — the only non-`RedisError` - /// producer on the `send` path is the offline-queue OOM, which `OutOfMemory` - /// already covers. - fn from(e: bun_core::Error) -> Self { - e.name().parse().unwrap_or(RedisError::ConnectionClosed) - } -} - -// `valkeyErrorToJS` alias deleted — lives in bun_runtime::valkey_jsc::protocol_jsc (extension trait). - /// RESP protocol types #[repr(u8)] #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -119,7 +99,18 @@ pub enum RESPValue { BigNumber(Box<[u8]>), } -// `deinit` deleted — all payloads are Box/Vec; Drop is automatic. +impl RESPValue { + /// `Some(msg)` when the value is a server-side error reply (`-ERR…` or + /// RESP3 `!…`), peeling through any `Attribute` wrapper. Used by the + /// client to decide resolve-vs-reject for a command promise. + pub fn as_server_error(&self) -> Option<&[u8]> { + match self { + RESPValue::Error(msg) | RESPValue::BlobError(msg) => Some(msg), + RESPValue::Attribute(attr) => attr.value.as_server_error(), + _ => None, + } + } +} impl fmt::Display for RESPValue { fn fmt(&self, writer: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -208,9 +199,6 @@ impl fmt::Display for RESPValue { } } -// `toJS` / `ToJSOptions` / `toJSWithOptions` aliases deleted — live in -// bun_runtime::valkey_jsc::protocol_jsc as extension-trait methods. - pub struct ValkeyReader<'a> { buffer: &'a [u8], pos: usize, @@ -238,7 +226,7 @@ impl<'a> ValkeyReader<'a> { self.pos } - pub fn read_byte(&mut self) -> Result { + fn read_byte(&mut self) -> Result { if self.pos >= self.buffer.len() { return Err(RedisError::InvalidResponse); } @@ -247,7 +235,7 @@ impl<'a> ValkeyReader<'a> { Ok(byte) } - pub fn read_until_crlf(&mut self) -> Result<&'a [u8], RedisError> { + fn read_until_crlf(&mut self) -> Result<&'a [u8], RedisError> { let buffer = &self.buffer[self.pos..]; let limit = buffer.len().min(Self::MAX_LINE_LEN + 1); let start = self.crlf_skip.min(limit); @@ -265,12 +253,12 @@ impl<'a> ValkeyReader<'a> { Err(RedisError::InvalidResponse) } - pub fn read_integer(&mut self) -> Result { + fn read_integer(&mut self) -> Result { let str = self.read_until_crlf()?; bun_core::fmt::parse_int::(str, 10).map_err(|_| RedisError::InvalidInteger) } - pub fn read_double(&mut self) -> Result { + fn read_double(&mut self) -> Result { let str = self.read_until_crlf()?; // Handle special values @@ -288,7 +276,7 @@ impl<'a> ValkeyReader<'a> { bun_core::fmt::parse_f64(str).ok_or(RedisError::InvalidDouble) } - pub fn read_boolean(&mut self) -> Result { + fn read_boolean(&mut self) -> Result { let str = self.read_until_crlf()?; if str.len() != 1 { return Err(RedisError::InvalidBoolean); @@ -301,31 +289,56 @@ impl<'a> ValkeyReader<'a> { } } - pub fn read_verbatim_string(&mut self) -> Result { + /// Consume a CRLF at the current position. Unlike `read_until_crlf` this + /// inspects exactly two bytes, so a malformed trailer fails in O(1). + fn expect_crlf(&mut self, invalid: RedisError) -> Result<(), RedisError> { + match self.buffer.get(self.pos..self.pos + 2) { + Some([b'\r', b'\n']) => { + self.pos += 2; + Ok(()) + } + Some(_) => Err(invalid), + None => Err(RedisError::InvalidResponse), + } + } + + /// Read a length-prefixed blob (`$`, `!`, `=`) and its trailing CRLF. + /// Returns `Ok(None)` only when `allow_null` and the declared length is + /// negative (RESP2 `$-1`). The bounds check subtracts instead of adding so + /// `pos + len` can never wrap `usize`. + fn read_blob( + &mut self, + allow_null: bool, + invalid: RedisError, + ) -> Result, RedisError> { let len = self.read_integer()?; - if !(0..=Self::MAX_BULK_LEN).contains(&len) { - return Err(RedisError::InvalidVerbatimString); + if len < 0 { + return if allow_null { Ok(None) } else { Err(invalid) }; + } + if len > Self::MAX_BULK_LEN { + return Err(invalid); } let len = usize::try_from(len).expect("int cast"); - if self.pos + len > self.buffer.len() { + if self.buffer.len() - self.pos < len { return Err(RedisError::InvalidResponse); } - - let content_with_format = &self.buffer[self.pos..self.pos + len]; + let start = self.pos; self.pos += len; + self.expect_crlf(invalid)?; + Ok(Some(&self.buffer[start..start + len])) + } - // Expect CRLF after content - let crlf = self.read_until_crlf()?; - if !crlf.is_empty() { - return Err(RedisError::InvalidVerbatimString); - } + fn read_verbatim_string(&mut self) -> Result { + let content_with_format = self + .read_blob(false, RedisError::InvalidVerbatimString)? + .expect("!allow_null"); // Format should be "xxx:" followed by content if content_with_format.len() < 4 || content_with_format[3] != b':' { return Err(RedisError::InvalidVerbatimString); } - let format = Box::<[u8]>::from(&content_with_format[0..3]); + let format: [u8; 3] = content_with_format[0..3].try_into().expect("3-byte slice"); let content = Box::<[u8]>::from(&content_with_format[4..]); Ok(VerbatimString { format, content }) @@ -360,9 +373,57 @@ impl<'a> ValkeyReader<'a> { cap } - pub fn read_value(&mut self) -> Result { + /// Shared prelude for `* % ~ | >`: depth guard + signed length read. + /// `Ok(None)` only when `allow_null` and the declared length is negative + /// (RESP2 `*-1`). + fn read_aggregate_header( + &mut self, + depth: usize, + allow_null: bool, + invalid: RedisError, + ) -> Result, RedisError> { + if depth >= Self::MAX_NESTING_DEPTH { + return Err(RedisError::NestingDepthExceeded); + } + let len = self.read_integer()?; + if len < 0 { + return if allow_null { Ok(None) } else { Err(invalid) }; + } + Ok(Some(usize::try_from(len).expect("int cast"))) + } + + fn read_n_values(&mut self, depth: usize, len: usize) -> Result, RedisError> { + let mut out = Vec::with_capacity(self.take_prealloc_budget(len, size_of::())); + for _ in 0..len { + out.push(self.read_value_with_depth(depth + 1)?); + } + Ok(out) + } + + fn read_n_entries(&mut self, depth: usize, len: usize) -> Result, RedisError> { + let mut out = Vec::with_capacity(self.take_prealloc_budget(len, size_of::())); + for _ in 0..len { + let key = self.read_value_with_depth(depth + 1)?; + let value = self.read_value_with_depth(depth + 1)?; + out.push(MapEntry { key, value }); + } + Ok(out) + } + + /// Parse one complete RESP value. Returns `Ok(None)` when the buffer ends + /// mid-value (caller should append more bytes and retry); `Err` is always a + /// real protocol error. + pub fn read_value(&mut self) -> Result, RedisError> { self.prealloc_budget = self.buffer.len() - self.pos; - self.read_value_with_depth(0) + let start = self.pos; + match self.read_value_with_depth(0) { + Ok(v) => Ok(Some(v)), + Err(RedisError::InvalidResponse) => { + self.pos = start; + Ok(None) + } + Err(e) => Err(e), + } } fn read_value_with_depth(&mut self, depth: usize) -> Result { @@ -384,50 +445,20 @@ impl<'a> ValkeyReader<'a> { let int = self.read_integer()?; Ok(RESPValue::Integer(int)) } - RESPType::BulkString => { - let len = self.read_integer()?; - if len < 0 { - return Ok(RESPValue::BulkString(None)); - } - if len > Self::MAX_BULK_LEN { - return Err(RedisError::InvalidBulkString); - } - let len = usize::try_from(len).expect("int cast"); - if self.pos + len > self.buffer.len() { - return Err(RedisError::InvalidResponse); - } - let str = &self.buffer[self.pos..self.pos + len]; - self.pos += len; - let crlf = self.read_until_crlf()?; - if !crlf.is_empty() { - return Err(RedisError::InvalidBulkString); - } - let owned = Box::<[u8]>::from(str); - Ok(RESPValue::BulkString(Some(owned))) - } + RESPType::BulkString => Ok(RESPValue::BulkString( + self.read_blob(true, RedisError::InvalidBulkString)? + .map(Box::<[u8]>::from), + )), RESPType::Array => { - if depth >= Self::MAX_NESTING_DEPTH { - return Err(RedisError::NestingDepthExceeded); - } - let len = self.read_integer()?; - if len < 0 { - return Ok(RESPValue::Array(Vec::new())); + match self.read_aggregate_header(depth, true, RedisError::InvalidResponse)? { + None => Ok(RESPValue::Null), + Some(n) => Ok(RESPValue::Array(self.read_n_values(depth, n)?)), } - let len = usize::try_from(len).expect("int cast"); - let mut array = - Vec::with_capacity(self.take_prealloc_budget(len, size_of::())); - // errdefer cleanup handled by Vec Drop on `?` - let mut i: usize = 0; - while i < len { - array.push(self.read_value_with_depth(depth + 1)?); - i += 1; - } - Ok(RESPValue::Array(array)) } // RESP3 types RESPType::Null => { - let _ = self.read_until_crlf()?; // Read and discard CRLF + self.expect_crlf(RedisError::InvalidResponseType)?; Ok(RESPValue::Null) } RESPType::Double => { @@ -439,140 +470,46 @@ impl<'a> ValkeyReader<'a> { Ok(RESPValue::Boolean(b)) } RESPType::BlobError => { - let len = self.read_integer()?; - if !(0..=Self::MAX_BULK_LEN).contains(&len) { - return Err(RedisError::InvalidBlobError); - } - let len = usize::try_from(len).expect("int cast"); - if self.pos + len > self.buffer.len() { - return Err(RedisError::InvalidResponse); - } - let str = &self.buffer[self.pos..self.pos + len]; - self.pos += len; - let crlf = self.read_until_crlf()?; - if !crlf.is_empty() { - return Err(RedisError::InvalidBlobError); - } - let owned = Box::<[u8]>::from(str); - Ok(RESPValue::BlobError(owned)) + let bytes = self + .read_blob(false, RedisError::InvalidBlobError)? + .expect("!allow_null"); + Ok(RESPValue::BlobError(Box::<[u8]>::from(bytes))) } RESPType::VerbatimString => Ok(RESPValue::VerbatimString(self.read_verbatim_string()?)), RESPType::Map => { - if depth >= Self::MAX_NESTING_DEPTH { - return Err(RedisError::NestingDepthExceeded); - } - let len = self.read_integer()?; - if len < 0 { - return Err(RedisError::InvalidMap); - } - let len = usize::try_from(len).expect("int cast"); - - let mut entries = - Vec::with_capacity(self.take_prealloc_budget(len, size_of::())); - // errdefer cleanup handled by Vec Drop on `?` - let mut i: usize = 0; - while i < len { - let key = self.read_value_with_depth(depth + 1)?; - // errdefer key.deinit() — `key` drops automatically on `?` below - let value = self.read_value_with_depth(depth + 1)?; - entries.push(MapEntry { key, value }); - i += 1; - } - Ok(RESPValue::Map(entries)) + let n = self + .read_aggregate_header(depth, false, RedisError::InvalidMap)? + .expect("!allow_null"); + Ok(RESPValue::Map(self.read_n_entries(depth, n)?)) } RESPType::Set => { - if depth >= Self::MAX_NESTING_DEPTH { - return Err(RedisError::NestingDepthExceeded); - } - let len = self.read_integer()?; - if len < 0 { - return Err(RedisError::InvalidSet); - } - let len = usize::try_from(len).expect("int cast"); - - let mut set = - Vec::with_capacity(self.take_prealloc_budget(len, size_of::())); - // errdefer cleanup handled by Vec Drop on `?` - let mut i: usize = 0; - while i < len { - set.push(self.read_value_with_depth(depth + 1)?); - i += 1; - } - Ok(RESPValue::Set(set)) + let n = self + .read_aggregate_header(depth, false, RedisError::InvalidSet)? + .expect("!allow_null"); + Ok(RESPValue::Set(self.read_n_values(depth, n)?)) } RESPType::Attribute => { - if depth >= Self::MAX_NESTING_DEPTH { - return Err(RedisError::NestingDepthExceeded); - } - let len = self.read_integer()?; - if len < 0 { - return Err(RedisError::InvalidAttribute); - } - let len = usize::try_from(len).expect("int cast"); - - let mut attrs = - Vec::with_capacity(self.take_prealloc_budget(len, size_of::())); - // errdefer cleanup handled by Vec Drop on `?` - let mut i: usize = 0; - while i < len { - let key = self.read_value_with_depth(depth + 1)?; - // errdefer key.deinit() — `key` drops automatically on `?` below - let value = self.read_value_with_depth(depth + 1)?; - attrs.push(MapEntry { key, value }); - i += 1; - } - - // Read the actual value that follows the attributes + let n = self + .read_aggregate_header(depth, false, RedisError::InvalidAttribute)? + .expect("!allow_null"); + let attributes = self.read_n_entries(depth, n)?; let value = Box::new(self.read_value_with_depth(depth + 1)?); - - Ok(RESPValue::Attribute(Attribute { - attributes: attrs, - value, - })) + Ok(RESPValue::Attribute(Attribute { attributes, value })) } RESPType::Push => { - if depth >= Self::MAX_NESTING_DEPTH { - return Err(RedisError::NestingDepthExceeded); - } - let len = self.read_integer()?; - if len <= 0 { + let n = self + .read_aggregate_header(depth, false, RedisError::InvalidPush)? + .expect("!allow_null"); + if n == 0 { return Err(RedisError::InvalidPush); } - // First element is the push type - let push_type = self.read_value_with_depth(depth + 1)?; - // defer push_type.deinit() — drops at scope end - let push_type_str: &[u8] = match &push_type { - RESPValue::SimpleString(str) => str, - RESPValue::BulkString(maybe_str) => { - if let Some(str) = maybe_str { - str - } else { - return Err(RedisError::InvalidPush); - } - } + let kind: Box<[u8]> = match self.read_value_with_depth(depth + 1)? { + RESPValue::SimpleString(s) | RESPValue::BulkString(Some(s)) => s, _ => return Err(RedisError::InvalidPush), }; - - // Copy the push type string since the original will be freed - let push_type_dup = Box::<[u8]>::from(push_type_str); - // errdefer free(push_type_dup) — drops automatically on `?` - - // Read the rest of the data - let data_len = usize::try_from(len - 1).expect("int cast"); - let mut data = - Vec::with_capacity(self.take_prealloc_budget(data_len, size_of::())); - // errdefer cleanup handled by Vec Drop on `?` - let mut i: usize = 0; - while i < data_len { - data.push(self.read_value_with_depth(depth + 1)?); - i += 1; - } - - Ok(RESPValue::Push(Push { - kind: push_type_dup, - data, - })) + let data = self.read_n_values(depth, n - 1)?; + Ok(RESPValue::Push(Push { kind, data })) } RESPType::BigNumber => { let str = self.read_until_crlf()?; @@ -673,7 +610,7 @@ impl ReplyScanner { /// Skip a single element starting at `reader.pos`. Returns `Some(n)` for an /// aggregate expecting `n` further child values, or `None` for a /// fully-skipped scalar. `InvalidResponse` means the element is not yet - /// fully buffered. + /// fully buffered and is never surfaced past [`ReplyScanner::scan`]. fn scan_one(reader: &mut ValkeyReader<'_>, depth: usize) -> Result, RedisError> { let type_byte = reader.read_byte()?; let ty = RESPType::from_byte(type_byte).ok_or(RedisError::InvalidResponseType)?; @@ -770,42 +707,49 @@ pub struct MapEntry { pub value: RESPValue, } -// `MapEntry::deinit` deleted — fields drop automatically. - pub struct VerbatimString { - pub format: Box<[u8]>, // e.g. "txt" or "mkd" + pub format: [u8; 3], // e.g. "txt" or "mkd" pub content: Box<[u8]>, } -// `VerbatimString::deinit` deleted — Box<[u8]> fields drop automatically. - pub struct Push { pub kind: Box<[u8]>, pub data: Vec, } -// `Push::deinit` deleted — Box/Vec fields drop automatically. - pub struct Attribute { pub attributes: Vec, pub value: Box, } -// `Attribute::deinit` deleted — Vec/Box fields drop automatically. - +/// The nine RESP3 pub/sub push kinds: plain, pattern (`p`-prefixed) and +/// sharded (`s`-prefixed) message delivery plus the matching subscribe / +/// unsubscribe acks. #[repr(u8)] #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum SubscriptionPushMessage { Message, + PMessage, + SMessage, Subscribe, + PSubscribe, + SSubscribe, Unsubscribe, + PUnsubscribe, + SUnsubscribe, } bun_core::comptime_string_map! { static SUBSCRIPTION_PUSH_MESSAGES: SubscriptionPushMessage = { b"message" => SubscriptionPushMessage::Message, + b"pmessage" => SubscriptionPushMessage::PMessage, + b"smessage" => SubscriptionPushMessage::SMessage, b"subscribe" => SubscriptionPushMessage::Subscribe, + b"psubscribe" => SubscriptionPushMessage::PSubscribe, + b"ssubscribe" => SubscriptionPushMessage::SSubscribe, b"unsubscribe" => SubscriptionPushMessage::Unsubscribe, + b"punsubscribe" => SubscriptionPushMessage::PUnsubscribe, + b"sunsubscribe" => SubscriptionPushMessage::SUnsubscribe, }; } @@ -815,17 +759,21 @@ impl SubscriptionPushMessage { SUBSCRIPTION_PUSH_MESSAGES.get(bytes).copied() } - /// Pattern (`p`-prefixed) and sharded (`s`-prefixed) variants of the - /// `Subscribe`/`Unsubscribe` push kinds; the unprefixed kinds are matched by - /// `from_bytes` before this is consulted. #[inline] - pub fn is_reply_kind(kind: &[u8]) -> bool { - match kind.split_first() { - Some((b'p' | b's', base)) => matches!( - Self::from_bytes(base), - Some(Self::Subscribe | Self::Unsubscribe) - ), - _ => false, - } + pub fn is_message(self) -> bool { + matches!(self, Self::Message | Self::PMessage | Self::SMessage) + } + + #[inline] + pub fn is_subscribe_ack(self) -> bool { + matches!(self, Self::Subscribe | Self::PSubscribe | Self::SSubscribe) + } + + #[inline] + pub fn is_unsubscribe_ack(self) -> bool { + matches!( + self, + Self::Unsubscribe | Self::PUnsubscribe | Self::SUnsubscribe + ) } } diff --git a/test/js/valkey/reliability/connection-failures.test.ts b/test/js/valkey/reliability/connection-failures.test.ts index bcf8964ac348..6914b6977cd3 100644 --- a/test/js/valkey/reliability/connection-failures.test.ts +++ b/test/js/valkey/reliability/connection-failures.test.ts @@ -27,8 +27,8 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { await client.set("key", "value"); expect(false).toBe(true); // Should not reach here } catch (error) { - // Expect an error with connection closed message - expect(error.message).toMatch(/connection closed|socket closed|failed to connect/i); + // Expect a connect-errno (on_connect_error surfaces e.g. ECONNREFUSED) or a connection-closed message + expect(error.message).toMatch(/connection closed|socket closed|failed to connect|connect E\w+/i); } finally { // Cleanup await client.close(); @@ -52,7 +52,9 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { expect(false).toBe(true); // Should not reach here } catch (error) { // Should fail with connection error - expect(error.message).toMatch(/connection closed|socket closed|failed to connect|offline queue is disabled/i); + expect(error.message).toMatch( + /connection closed|socket closed|failed to connect|connect E\w+|offline queue is disabled/i, + ); } try { @@ -60,7 +62,9 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { expect(false).toBe(true); // Should not reach here } catch (error) { // Should fail with connection error - expect(error.message).toMatch(/connection closed|socket closed|failed to connect|offline queue is disabled/i); + expect(error.message).toMatch( + /connection closed|socket closed|failed to connect|connect E\w+|offline queue is disabled/i, + ); } try { @@ -68,7 +72,9 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { expect(false).toBe(true); // Should not reach here } catch (error) { // Should fail with connection error - expect(error.message).toMatch(/connection closed|socket closed|failed to connect|offline queue is disabled/i); + expect(error.message).toMatch( + /connection closed|socket closed|failed to connect|connect E\w+|offline queue is disabled/i, + ); } try { @@ -76,7 +82,9 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { expect(false).toBe(true); // Should not reach here } catch (error) { // Should fail with connection error - expect(error.message).toMatch(/connection closed|socket closed|failed to connect|offline queue is disabled/i); + expect(error.message).toMatch( + /connection closed|socket closed|failed to connect|connect E\w+|offline queue is disabled/i, + ); } }); @@ -134,7 +142,7 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { expect(false).toBe(true); // Should not reach here } catch (error) { // Should fail with a connection error - expect(error.message).toMatch(/connection closed|socket closed|failed to connect/i); + expect(error.message).toMatch(/connection closed|socket closed|failed to connect|connect E\w+/i); } await client.close(); @@ -153,7 +161,7 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { await client.set("key", "value"); expect(false).toBe(true); // Should not reach here } catch (error) { - expect(error.message).toMatch(/connection closed|offline queue is disabled/i); + expect(error.message).toMatch(/connection closed|connect E\w+|offline queue is disabled/i); } await client.close(); @@ -277,9 +285,7 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { await client.close(); expect(client.connected).toBe(false); - expect(async () => { - await client.get("any-key"); - }).toThrowErrorMatchingInlineSnapshot(`"Connection closed"`); + await expect(client.get("any-key")).rejects.toThrow(/connection closed|connect E\w+/i); // Multiple disconnects should not cause issues await client.close(); await client.close(); @@ -322,7 +328,7 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { const promises = clients.map(client => client.get("key").catch(err => { // We expect errors, but want to make sure they're the right kind - expect(err.message).toMatch(/connection closed|socket closed|failed to connect/i); + expect(err.message).toMatch(/connection closed|socket closed|failed to connect|connect E\w+/i); }), ); diff --git a/test/js/valkey/reliability/error-handling.test.ts b/test/js/valkey/reliability/error-handling.test.ts index da12977c462b..441c0992c554 100644 --- a/test/js/valkey/reliability/error-handling.test.ts +++ b/test/js/valkey/reliability/error-handling.test.ts @@ -109,7 +109,7 @@ describe.skipIf(!isEnabled)("Valkey: Error Handling", () => { // Invalid args type // @ts-expect-error: Testing runtime behavior with invalid types expect(async () => await client.send("GET", "not-an-array")).toThrowErrorMatchingInlineSnapshot( - `"Arguments must be an array"`, + `"Expected args to be a array for 'send'."`, ); // Non-string command diff --git a/test/js/valkey/reliability/resp-nesting-depth.test.ts b/test/js/valkey/reliability/resp-nesting-depth.test.ts index 600f5005ffae..aa9ff41e2db3 100644 --- a/test/js/valkey/reliability/resp-nesting-depth.test.ts +++ b/test/js/valkey/reliability/resp-nesting-depth.test.ts @@ -114,6 +114,7 @@ describe("Valkey: RESP Nesting Depth Handling", () => { } catch (error: any) { // The client should surface an error rather than crashing. expect(error.code).toBe("ERR_REDIS_INVALID_RESPONSE"); + expect(error.message).toContain("NestingDepthExceeded"); } finally { client.close(); } @@ -348,7 +349,9 @@ describe("Valkey: RESP push frame routing", () => { const psubscribed = client.psubscribe("news.*"); const pinged = client.send("PING", []); - expect(await psubscribed).toEqual({ type: "psubscribe", data: ["news.*", 1] }); + // psubscribe ack now routes through the subscribe-ack path and resolves + // with the handler-map count, matching subscribe()'s shape. + expect(await psubscribed).toEqual(0); expect(await pinged).toBe("PONG"); } finally { client.close(); @@ -357,4 +360,43 @@ describe("Valkey: RESP push frame routing", () => { server.close(); } }); + + for (const [label, cmd, ack] of [ + [ + "psubscribe()", + (c: Bun.RedisClient) => c.psubscribe("news.*"), + ">3\r\n$10\r\npsubscribe\r\n$6\r\nnews.*\r\n:1\r\n", + ], + [ + "send('SUBSCRIBE', ...)", + (c: Bun.RedisClient) => c.send("SUBSCRIBE", ["ch"]), + ">3\r\n$9\r\nsubscribe\r\n$2\r\nch\r\n:1\r\n", + ], + [ + "send('SSUBSCRIBE', ...)", + (c: Bun.RedisClient) => c.send("SSUBSCRIBE", ["shard-ch"]), + ">3\r\n$10\r\nssubscribe\r\n$8\r\nshard-ch\r\n:1\r\n", + ], + ] as const) { + test(`${label} does not enter subscriber mode (get/set still allowed)`, async () => { + const getReply = Buffer.from("$5\r\nvalue\r\n"); + const { server, port } = await createMockRedisServer([Buffer.from(ack), getReply]); + try { + const client = new Bun.RedisClient(`redis://127.0.0.1:${port}`, { + autoReconnect: false, + connectionTimeout: 2000, + }); + try { + expect(await cmd(client)).toEqual(0); + // Only `.subscribe(channel, handler)` populates the handler map and + // flips subscriber mode; pattern/shard acks must not block regular commands. + expect(await client.get("k")).toBe("value"); + } finally { + client.close(); + } + } finally { + server.close(); + } + }); + } }); diff --git a/test/js/valkey/valkey-incremental-scan.test.ts b/test/js/valkey/valkey-incremental-scan.test.ts index 4cd4f306be9e..838ae910b5e6 100644 --- a/test/js/valkey/valkey-incremental-scan.test.ts +++ b/test/js/valkey/valkey-incremental-scan.test.ts @@ -115,12 +115,9 @@ describe.concurrent("Valkey reply torn across socket reads", () => { test.each(LONG_SPLITS)("BlobError (!) torn at byte %i decodes instead of failing the connection", async splitAt => { const server = createTornReplyServer(`!21${CRLF}SYNTAX invalid syntax${CRLF}`, splitAt); await withClient(server, async client => { - // A parsed BlobError resolves (not rejects) with an Error carrying the - // server's message. Before the fix this rejected with - // "Failed to read data (stack path)" and killed the connection. - const result = await client.get("k"); - expect(result).toBeInstanceOf(Error); - expect((result as unknown as Error).message).toBe("SYNTAX invalid syntax"); + // A parsed BlobError rejects the command promise with the server's + // message but leaves the connection open for subsequent commands. + await expect(client.get("k")).rejects.toThrow("SYNTAX invalid syntax"); expect(await client.send("PING", [])).toBe("OK"); }); }); diff --git a/test/js/valkey/valkey.test.ts b/test/js/valkey/valkey.test.ts index 0e521c0fc6d3..a63d795cd529 100644 --- a/test/js/valkey/valkey.test.ts +++ b/test/js/valkey/valkey.test.ts @@ -989,7 +989,7 @@ for (const connectionType of [ConnectionType.TLS, ConnectionType.TCP]) { const redis = ctx.redis; expect(async () => { await redis.unlink({} as any); - }).toThrowErrorMatchingInlineSnapshot(`"Expected additional arguments to be a string or buffer for 'unlink'."`); + }).toThrowErrorMatchingInlineSnapshot(`"Expected key to be a string or buffer for 'unlink'."`); }); test("should reject invalid additional key in UNLINK", async () => { @@ -1003,7 +1003,7 @@ for (const connectionType of [ConnectionType.TLS, ConnectionType.TCP]) { const redis = ctx.redis; expect(async () => { await redis.touch(null as any); - }).toThrowErrorMatchingInlineSnapshot(`"The "key" argument must be specified"`); + }).toThrowErrorMatchingInlineSnapshot(`"Expected key to be a string or buffer for 'touch'."`); }); test("should reject invalid additional key in TOUCH", async () => { @@ -3394,7 +3394,7 @@ for (const connectionType of [ConnectionType.TLS, ConnectionType.TCP]) { const redis = ctx.redis; expect(async () => { await redis.zrem({} as any, "member"); - }).toThrowErrorMatchingInlineSnapshot(`"Expected additional arguments to be a string or buffer for 'zrem'."`); + }).toThrowErrorMatchingInlineSnapshot(`"Expected key to be a string or buffer for 'zrem'."`); }); test("should reject invalid key in ZMSCORE", async () => { @@ -4278,7 +4278,7 @@ for (const connectionType of [ConnectionType.TLS, ConnectionType.TCP]) { const redis = ctx.redis; expect(async () => { await redis.zrangebylex(null as any, "-", "+"); - }).toThrowErrorMatchingInlineSnapshot(`"The "key" argument must be specified"`); + }).toThrowErrorMatchingInlineSnapshot(`"Expected key to be a string or buffer for 'zrangebylex'."`); }); test("should return members in reverse lexicographical order with ZREVRANGEBYLEX", async () => { @@ -4380,9 +4380,7 @@ for (const connectionType of [ConnectionType.TLS, ConnectionType.TCP]) { const redis = ctx.redis; expect(async () => { await redis.zrevrangebylex({} as any, "+", "-"); - }).toThrowErrorMatchingInlineSnapshot( - `"Expected additional arguments to be a string or buffer for 'zrevrangebylex'."`, - ); + }).toThrowErrorMatchingInlineSnapshot(`"Expected key to be a string or buffer for 'zrevrangebylex'."`); }); test("should reject invalid destination in ZRANGESTORE", async () => {