Drop helper

codex-rs/code-mode/src/runtime/callbacks.rs

@@ -9,37 +9,80 @@ use super::value::serialize_output_text;
 use super::value::throw_type_error;
 use super::value::v8_value_to_json;
 
+type CallbackResult<'s, T = ()> = Result<T, String>;
+
+struct CallbackThrow<'s>(v8::Local<'s, v8::Value>);
+
+trait CallbackReturn<'s> {
+    fn complete(self, scope: &mut v8::PinScope<'s, '_>, retval: &mut v8::ReturnValue<v8::Value>);
+}
+
+impl<'s> CallbackReturn<'s> for () {
+    fn complete(self, _scope: &mut v8::PinScope<'s, '_>, _retval: &mut v8::ReturnValue<v8::Value>) {
+    }
+}
+
+impl<'s> CallbackReturn<'s> for v8::Local<'s, v8::Value> {
+    fn complete(self, _scope: &mut v8::PinScope<'s, '_>, retval: &mut v8::ReturnValue<v8::Value>) {
+        retval.set(self);
+    }
+}
+
+impl<'s> CallbackReturn<'s> for Option<v8::Local<'s, v8::Value>> {
+    fn complete(self, _scope: &mut v8::PinScope<'s, '_>, retval: &mut v8::ReturnValue<v8::Value>) {
+        if let Some(value) = self {
+            retval.set(value);
+        }
+    }
+}
+
+impl<'s> CallbackReturn<'s> for CallbackThrow<'s> {
+    fn complete(self, scope: &mut v8::PinScope<'s, '_>, _retval: &mut v8::ReturnValue<v8::Value>) {
+        scope.throw_exception(self.0);
+    }
+}
+
+// Keep each exported V8 callback as a thin adapter over an inner Result-returning
+// implementation. This macro handles the final wiring from that Result into V8 by
+// either leaving the return value alone, setting it, or throwing. A macro keeps
+// the callsite readable without introducing lifetime friction from a generic helper.
+macro_rules! run_callback {
+    ($scope:expr, $retval:expr, $result:expr) => {
+        match $result {
+            Ok(value) => value.complete($scope, &mut $retval),
+            Err(error_text) => throw_type_error($scope, &error_text),
+        }
+    };
+}
+
 pub(super) fn tool_callback(
     scope: &mut v8::PinScope<'_, '_>,
     args: v8::FunctionCallbackArguments,
     mut retval: v8::ReturnValue<v8::Value>,
 ) {
+    run_callback!(scope, retval, tool_callback_inner(scope, args));
+}
+
+fn tool_callback_inner<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    args: v8::FunctionCallbackArguments,
+) -> CallbackResult<'s, v8::Local<'s, v8::Value>> {
     let tool_name = args.data().to_rust_string_lossy(scope);
     let input = if args.length() == 0 {
-        Ok(None)
+        None
     } else {
-        v8_value_to_json(scope, args.get(0))
-    };
-    let input = match input {
-        Ok(input) => input,
-        Err(error_text) => {
-            throw_type_error(scope, &error_text);
-            return;
-        }
-    };
-
-    let Some(resolver) = v8::PromiseResolver::new(scope) else {
-        throw_type_error(scope, "failed to create tool promise");
-        return;
+        v8_value_to_json(scope, args.get(0))?
     };
+    let resolver = v8::PromiseResolver::new(scope)
+        .ok_or_else(|| "failed to create tool promise".to_string())?;
     let promise = resolver.get_promise(scope);
 
     let resolver = v8::Global::new(scope, resolver);
-    let Some(state) = scope.get_slot_mut::<RuntimeState>() else {
-        throw_type_error(scope, "runtime state unavailable");
-        return;
-    };
-    let id = format!("tool-{}", state.next_tool_call_id);
+    let state = scope
+        .get_slot_mut::<RuntimeState>()
+        .ok_or_else(|| "runtime state unavailable".to_string())?;
+    let next_tool_call_id = state.next_tool_call_id;
+    let id = format!("tool-{next_tool_call_id}");
     state.next_tool_call_id = state.next_tool_call_id.saturating_add(1);
     let event_tx = state.event_tx.clone();
     state.pending_tool_calls.insert(id.clone(), resolver);
@@ -48,7 +91,7 @@ pub(super) fn tool_callback(
         name: tool_name,
         input,
     });
-    retval.set(promise.into());
+    Ok(promise.into())
 }
 
 pub(super) fn text_callback(
@@ -56,24 +99,25 @@ pub(super) fn text_callback(
     args: v8::FunctionCallbackArguments,
     mut retval: v8::ReturnValue<v8::Value>,
 ) {
+    run_callback!(scope, retval, text_callback_inner(scope, args));
+}
+
+fn text_callback_inner<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    args: v8::FunctionCallbackArguments,
+) -> CallbackResult<'s> {
     let value = if args.length() == 0 {
         v8::undefined(scope).into()
     } else {
         args.get(0)
     };
-    let text = match serialize_output_text(scope, value) {
-        Ok(text) => text,
-        Err(error_text) => {
-            throw_type_error(scope, &error_text);
-            return;
-        }
-    };
+    let text = serialize_output_text(scope, value)?;
     if let Some(state) = scope.get_slot::<RuntimeState>() {
         let _ = state.event_tx.send(RuntimeEvent::ContentItem(
             FunctionCallOutputContentItem::InputText { text },
         ));
     }
-    retval.set(v8::undefined(scope).into());
+    Ok(())
 }
 
 pub(super) fn image_callback(
@@ -81,51 +125,54 @@ pub(super) fn image_callback(
     args: v8::FunctionCallbackArguments,
     mut retval: v8::ReturnValue<v8::Value>,
 ) {
+    run_callback!(scope, retval, image_callback_inner(scope, args));
+}
+
+fn image_callback_inner<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    args: v8::FunctionCallbackArguments,
+) -> CallbackResult<'s> {
     let value = if args.length() == 0 {
         v8::undefined(scope).into()
     } else {
         args.get(0)
     };
-    let image_item = match normalize_output_image(scope, value) {
-        Ok(image_item) => image_item,
-        Err(()) => return,
-    };
+    let image_item = normalize_output_image(scope, value)?;
     if let Some(state) = scope.get_slot::<RuntimeState>() {
         let _ = state.event_tx.send(RuntimeEvent::ContentItem(image_item));
     }
-    retval.set(v8::undefined(scope).into());
+    Ok(())
 }
 
 pub(super) fn store_callback(
     scope: &mut v8::PinScope<'_, '_>,
     args: v8::FunctionCallbackArguments,
-    _retval: v8::ReturnValue<v8::Value>,
+    mut retval: v8::ReturnValue<v8::Value>,
 ) {
-    let key = match args.get(0).to_string(scope) {
-        Some(key) => key.to_rust_string_lossy(scope),
+    run_callback!(scope, retval, store_callback_inner(scope, args));
+}
+
+fn store_callback_inner<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    args: v8::FunctionCallbackArguments,
+) -> CallbackResult<'s> {
+    let key = args
+        .get(0)
+        .to_string(scope)
+        .map(|value| value.to_rust_string_lossy(scope))
+        .ok_or_else(|| "store key must be a string".to_string())?;
+    let serialized = match v8_value_to_json(scope, args.get(1))? {
+        Some(value) => value,
         None => {
-            throw_type_error(scope, "store key must be a string");
-            return;
-        }
-    };
-    let value = args.get(1);
-    let serialized = match v8_value_to_json(scope, value) {
-        Ok(Some(value)) => value,
-        Ok(None) => {
-            throw_type_error(
-                scope,
-                &format!("Unable to store {key:?}. Only plain serializable objects can be stored."),
-            );
-            return;
-        }
-        Err(error_text) => {
-            throw_type_error(scope, &error_text);
-            return;
+            return Err(format!(
+                "Unable to store {key:?}. Only plain serializable objects can be stored."
+            ));
         }
     };
     if let Some(state) = scope.get_slot_mut::<RuntimeState>() {
         state.stored_values.insert(key, serialized);
     }
+    Ok(())
 }
 
 pub(super) fn load_callback(
@@ -133,26 +180,28 @@ pub(super) fn load_callback(
     args: v8::FunctionCallbackArguments,
     mut retval: v8::ReturnValue<v8::Value>,
 ) {
-    let key = match args.get(0).to_string(scope) {
-        Some(key) => key.to_rust_string_lossy(scope),
-        None => {
-            throw_type_error(scope, "load key must be a string");
-            return;
-        }
-    };
+    run_callback!(scope, retval, load_callback_inner(scope, args));
+}
+
+fn load_callback_inner<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    args: v8::FunctionCallbackArguments,
+) -> CallbackResult<'s, Option<v8::Local<'s, v8::Value>>> {
+    let key = args
+        .get(0)
+        .to_string(scope)
+        .map(|value| value.to_rust_string_lossy(scope))
+        .ok_or_else(|| "load key must be a string".to_string())?;
     let value = scope
         .get_slot::<RuntimeState>()
         .and_then(|state| state.stored_values.get(&key))
         .cloned();
     let Some(value) = value else {
-        retval.set(v8::undefined(scope).into());
-        return;
+        return Ok(None);
     };
-    let Some(value) = json_to_v8(scope, &value) else {
-        throw_type_error(scope, "failed to load stored value");
-        return;
-    };
-    retval.set(value);
+    let value =
+        json_to_v8(scope, &value).ok_or_else(|| "failed to load stored value".to_string())?;
+    Ok(Some(value))
 }
 
 pub(super) fn notify_callback(
@@ -160,21 +209,21 @@ pub(super) fn notify_callback(
     args: v8::FunctionCallbackArguments,
     mut retval: v8::ReturnValue<v8::Value>,
 ) {
+    run_callback!(scope, retval, notify_callback_inner(scope, args));
+}
+
+fn notify_callback_inner<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    args: v8::FunctionCallbackArguments,
+) -> CallbackResult<'s> {
     let value = if args.length() == 0 {
         v8::undefined(scope).into()
     } else {
         args.get(0)
     };
-    let text = match serialize_output_text(scope, value) {
-        Ok(text) => text,
-        Err(error_text) => {
-            throw_type_error(scope, &error_text);
-            return;
-        }
-    };
+    let text = serialize_output_text(scope, value)?;
     if text.trim().is_empty() {
-        throw_type_error(scope, "notify expects non-empty text");
-        return;
+        return Err("notify expects non-empty text".to_string());
     }
     if let Some(state) = scope.get_slot::<RuntimeState>() {
         let _ = state.event_tx.send(RuntimeEvent::Notify {
@@ -182,28 +231,43 @@ pub(super) fn notify_callback(
             text,
         });
     }
-    retval.set(v8::undefined(scope).into());
+    Ok(())
 }
 
 pub(super) fn yield_control_callback(
     scope: &mut v8::PinScope<'_, '_>,
-    _args: v8::FunctionCallbackArguments,
-    _retval: v8::ReturnValue<v8::Value>,
+    args: v8::FunctionCallbackArguments,
+    mut retval: v8::ReturnValue<v8::Value>,
 ) {
+    run_callback!(scope, retval, yield_control_callback_inner(scope, args));
+}
+
+fn yield_control_callback_inner<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    _args: v8::FunctionCallbackArguments,
+) -> CallbackResult<'s> {
     if let Some(state) = scope.get_slot::<RuntimeState>() {
         let _ = state.event_tx.send(RuntimeEvent::YieldRequested);
     }
+    Ok(())
 }
 
 pub(super) fn exit_callback(
     scope: &mut v8::PinScope<'_, '_>,
-    _args: v8::FunctionCallbackArguments,
-    _retval: v8::ReturnValue<v8::Value>,
+    args: v8::FunctionCallbackArguments,
+    mut retval: v8::ReturnValue<v8::Value>,
 ) {
+    run_callback!(scope, retval, exit_callback_inner(scope, args));
+}
+
+fn exit_callback_inner<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    _args: v8::FunctionCallbackArguments,
+) -> CallbackResult<'s, CallbackThrow<'s>> {
     if let Some(state) = scope.get_slot_mut::<RuntimeState>() {
         state.exit_requested = true;
     }
-    if let Some(error) = v8::String::new(scope, EXIT_SENTINEL) {
-        scope.throw_exception(error.into());
-    }
+    let error = v8::String::new(scope, EXIT_SENTINEL)
+        .ok_or_else(|| "failed to allocate exit sentinel".to_string())?;
+    Ok(CallbackThrow(error.into()))
 }

codex-rs/code-mode/src/runtime/value.rs

@@ -34,8 +34,8 @@ pub(super) fn serialize_output_text(
 pub(super) fn normalize_output_image(
     scope: &mut v8::PinScope<'_, '_>,
     value: v8::Local<'_, v8::Value>,
-) -> Result<FunctionCallOutputContentItem, ()> {
-    let result = (|| -> Result<FunctionCallOutputContentItem, String> {
+) -> Result<FunctionCallOutputContentItem, String> {
+    (|| -> Result<FunctionCallOutputContentItem, String> {
         let (image_url, detail) = if value.is_string() {
             (value.to_rust_string_lossy(scope), None)
         } else if value.is_object() && !value.is_array() {
@@ -101,15 +101,7 @@ pub(super) fn normalize_output_image(
         };
 
         Ok(FunctionCallOutputContentItem::InputImage { image_url, detail })
-    })();
-
-    match result {
-        Ok(item) => Ok(item),
-        Err(error_text) => {
-            throw_type_error(scope, &error_text);
-            Err(())
-        }
-    }
+    })()
 }
 
 pub(super) fn v8_value_to_json(

codex-rs/code-mode/src/service.rs

@@ -604,6 +604,57 @@ text(JSON.stringify(returnsUndefined));
         );
     }
 
+    #[tokio::test]
+    async fn notify_rejects_empty_text() {
+        let service = CodeModeService::new();
+
+        let response = service
+            .execute(ExecuteRequest {
+                source: r#"notify("   "); text("after");"#.to_string(),
+                yield_time_ms: None,
+                ..execute_request("")
+            })
+            .await
+            .unwrap();
+
+        assert_eq!(
+            response,
+            RuntimeResponse::Result {
+                cell_id: "1".to_string(),
+                content_items: Vec::new(),
+                stored_values: HashMap::new(),
+                error_text: Some("notify expects non-empty text".to_string()),
+            }
+        );
+    }
+
+    #[tokio::test]
+    async fn store_surfaces_serialization_errors() {
+        let service = CodeModeService::new();
+
+        let response = service
+            .execute(ExecuteRequest {
+                source: r#"store("bad", () => {}); text("after");"#.to_string(),
+                yield_time_ms: None,
+                ..execute_request("")
+            })
+            .await
+            .unwrap();
+
+        assert_eq!(
+            response,
+            RuntimeResponse::Result {
+                cell_id: "1".to_string(),
+                content_items: Vec::new(),
+                stored_values: HashMap::new(),
+                error_text: Some(
+                    "failed to serialize JavaScript value: expected value at line 1 column 1"
+                        .to_string(),
+                ),
+            }
+        );
+    }
+
     #[tokio::test]
     async fn terminate_waits_for_runtime_shutdown_before_responding() {
         let inner = test_inner();