From 8369e532d4f4585576e00196eb570f1a637c2775 Mon Sep 17 00:00:00 2001 From: Steve Korshakov Date: Mon, 1 Jun 2026 12:21:27 -0700 Subject: [PATCH 1/3] Add JS OS call and REPL pause support --- crates/monty-js/__test__/async.spec.ts | 16 + crates/monty-js/__test__/external.spec.ts | 16 + crates/monty-js/__test__/repl.spec.ts | 30 +- crates/monty-js/__test__/start.spec.ts | 34 +- crates/monty-js/src/convert.rs | 114 ++++ crates/monty-js/src/monty_cls.rs | 673 +++++++++++++++++++--- crates/monty-js/wrapper.ts | 37 +- 7 files changed, 839 insertions(+), 81 deletions(-) diff --git a/crates/monty-js/__test__/async.spec.ts b/crates/monty-js/__test__/async.spec.ts index ba0410508..6858600ff 100644 --- a/crates/monty-js/__test__/async.spec.ts +++ b/crates/monty-js/__test__/async.spec.ts @@ -87,6 +87,22 @@ test('runMontyAsync with args and kwargs', async (t) => { t.is(result, 'test: 3') }) +test('runMontyAsync handles OS datetime.now via externalFunctions', async (t) => { + const m = new Monty(` +from datetime import datetime, timezone +now = datetime.now(timezone.utc) +[now.year, now.month, now.day, now.hour, now.minute, now.second, now.microsecond] +`) + + const result = await runMontyAsync(m, { + externalFunctions: { + 'datetime.now': () => new Date(Date.UTC(2027, 1, 3, 4, 5, 6, 7)), + }, + }) + + t.deepEqual(result, [2027, 2, 3, 4, 5, 6, 7000]) +}) + // ============================================================================= // Error handling tests // ============================================================================= diff --git a/crates/monty-js/__test__/external.spec.ts b/crates/monty-js/__test__/external.spec.ts index eca8a9e8c..2b9208578 100644 --- a/crates/monty-js/__test__/external.spec.ts +++ b/crates/monty-js/__test__/external.spec.ts @@ -143,6 +143,22 @@ test('external function with input', (t) => { t.is(m.run({ inputs: { x: 5 }, externalFunctions: { process } }), 50) }) +test('OS datetime.now can be handled by externalFunctions', (t) => { + const m = new Monty(` +from datetime import datetime +now = datetime.now() +[now.year, now.month, now.day, now.hour, now.minute, now.second, now.microsecond] +`) + + const result = m.run({ + externalFunctions: { + 'datetime.now': () => new Date(2026, 0, 2, 3, 4, 5, 6), + }, + }) + + t.deepEqual(result, [2026, 1, 2, 3, 4, 5, 6000]) +}) + // ============================================================================= // Error handling tests // ============================================================================= diff --git a/crates/monty-js/__test__/repl.spec.ts b/crates/monty-js/__test__/repl.spec.ts index 455e5452a..fb1999dab 100644 --- a/crates/monty-js/__test__/repl.spec.ts +++ b/crates/monty-js/__test__/repl.spec.ts @@ -1,6 +1,6 @@ import test from 'ava' -import { MontyRepl } from '../wrapper' +import { MontyComplete, MontyRepl, MontySnapshot } from '../wrapper' test('feed preserves state without replay', (t) => { const repl = new MontyRepl() @@ -32,3 +32,31 @@ test('repl dump/load roundtrip', (t) => { t.is(loaded.feed('x + 1'), 42) }) + +test('feedStart pauses and resumes external function calls', (t) => { + const repl = new MontyRepl() + + let progress = repl.feedStart('value = get_value()\nvalue') + t.true(progress instanceof MontySnapshot) + t.is((progress as MontySnapshot).functionName, 'get_value') + t.false((progress as MontySnapshot).isOsFunction) + + progress = (progress as MontySnapshot).resume({ returnValue: 41 }) + t.true(progress instanceof MontyComplete) + t.is((progress as MontyComplete).output, 41) + t.is(repl.feed('value + 1'), 42) +}) + +test('feedStart pauses and resumes OS calls', (t) => { + const repl = new MontyRepl() + + let progress = repl.feedStart('from datetime import datetime\nstamp = datetime.now()\nstamp.year') + t.true(progress instanceof MontySnapshot) + t.is((progress as MontySnapshot).functionName, 'datetime.now') + t.true((progress as MontySnapshot).isOsFunction) + + progress = (progress as MontySnapshot).resume({ returnValue: new Date(2032, 4, 6, 7, 8, 9, 10) }) + t.true(progress instanceof MontyComplete) + t.is((progress as MontyComplete).output, 2032) + t.is(repl.feed('stamp.month'), 5) +}) diff --git a/crates/monty-js/__test__/start.spec.ts b/crates/monty-js/__test__/start.spec.ts index 70afa46c2..d04cd3763 100644 --- a/crates/monty-js/__test__/start.spec.ts +++ b/crates/monty-js/__test__/start.spec.ts @@ -398,18 +398,36 @@ test('start can reuse monty instance', (t) => { // OS call handling in start() tests // ============================================================================= -test('os.environ via start() throws RuntimeError', (t) => { +test('os.environ via start() returns OS snapshot', (t) => { const m = new Monty('import os\nx = os.environ') - const error = t.throws(() => m.start(), { instanceOf: MontyRuntimeError }) - t.is(error.exception.typeName, 'RuntimeError') - t.is(error.exception.message, "'os.environ' is not supported in this environment") + const progress = m.start() + t.true(progress instanceof MontySnapshot) + t.is((progress as MontySnapshot).functionName, 'os.environ') + t.true((progress as MontySnapshot).isOsFunction) + t.deepEqual((progress as MontySnapshot).args, []) + t.deepEqual((progress as MontySnapshot).kwargs, {}) }) -test('os.getenv via start() throws RuntimeError', (t) => { +test('os.getenv via start() returns OS snapshot', (t) => { const m = new Monty("import os\nx = os.getenv('HOME')") - const error = t.throws(() => m.start(), { instanceOf: MontyRuntimeError }) - t.is(error.exception.typeName, 'RuntimeError') - t.is(error.exception.message, "'os.getenv' is not supported in this environment") + const progress = m.start() + t.true(progress instanceof MontySnapshot) + t.is((progress as MontySnapshot).functionName, 'os.getenv') + t.true((progress as MontySnapshot).isOsFunction) + t.deepEqual((progress as MontySnapshot).args, ['HOME', null]) + t.deepEqual((progress as MontySnapshot).kwargs, {}) +}) + +test('datetime.now via start() returns OS snapshot and resumes', (t) => { + const m = new Monty('from datetime import datetime\nnow = datetime.now()\nnow.year') + const progress = m.start() + t.true(progress instanceof MontySnapshot) + t.is((progress as MontySnapshot).functionName, 'datetime.now') + t.true((progress as MontySnapshot).isOsFunction) + + const complete = (progress as MontySnapshot).resume({ returnValue: new Date(2031, 6, 8, 9, 10, 11, 12) }) + t.true(complete instanceof MontyComplete) + t.is((complete as MontyComplete).output, 2031) }) // ============================================================================= diff --git a/crates/monty-js/src/convert.rs b/crates/monty-js/src/convert.rs index 72899961e..035d200df 100644 --- a/crates/monty-js/src/convert.rs +++ b/crates/monty-js/src/convert.rs @@ -460,6 +460,11 @@ pub fn js_to_monty(value: Unknown<'_>, env: Env) -> Result { ValueType::Object => { let obj: Object = value.coerce_to_object()?; + // Check if it's a Date + if is_js_date(&obj, env)? { + return Ok(MontyObject::DateTime(js_date_to_monty_datetime(obj, &env, None)?)); + } + // Check if it's a Buffer (Uint8Array) if obj.is_buffer()? { let buffer: BufferSlice = BufferSlice::from_unknown(value)?; @@ -509,6 +514,38 @@ pub fn js_to_monty(value: Unknown<'_>, env: Env) -> Result { } } +/// Converts an external function result to `MontyObject`, with OS-call-specific +/// handling for JavaScript `Date` objects. +pub fn js_external_result_to_monty( + function_name: &str, + call_args: &[MontyObject], + value: Unknown<'_>, + env: Env, +) -> Result { + if value.get_type()? == ValueType::Object { + let obj: Object = value.coerce_to_object()?; + if is_js_date(&obj, env)? { + return match function_name { + "date.today" => Ok(MontyObject::Date(js_date_to_monty_date(obj)?)), + "datetime.now" => Ok(MontyObject::DateTime(js_date_to_monty_datetime( + obj, + &env, + datetime_now_timezone_arg(call_args), + )?)), + _ => Ok(MontyObject::DateTime(js_date_to_monty_datetime(obj, &env, None)?)), + }; + } + } + js_to_monty(value, env) +} + +fn datetime_now_timezone_arg(args: &[MontyObject]) -> Option<&MontyTimeZone> { + match args.first() { + Some(MontyObject::TimeZone(tz)) => Some(tz), + _ => None, + } +} + /// Checks if a JS object is an instance of Set. fn is_js_set(obj: &Object, env: Env) -> Result { let global = env.get_global()?; @@ -523,6 +560,83 @@ fn is_js_map(obj: &Object, env: Env) -> Result { obj.instanceof(map_constructor) } +/// Checks if a JS object is an instance of Date. +fn is_js_date(obj: &Object, env: Env) -> Result { + let global = env.get_global()?; + let date_constructor: Function<()> = global.get_named_property("Date")?; + obj.instanceof(date_constructor) +} + +fn js_date_to_monty_date(date: Object<'_>) -> Result { + Ok(MontyDate { + year: call_js_date_i32(date, "getFullYear")?, + month: u8::try_from(call_js_date_i32(date, "getMonth")? + 1) + .map_err(|_| Error::from_reason("Date month is out of range"))?, + day: u8::try_from(call_js_date_i32(date, "getDate")?) + .map_err(|_| Error::from_reason("Date day is out of range"))?, + }) +} + +fn js_date_to_monty_datetime(date: Object<'_>, env: &Env, timezone: Option<&MontyTimeZone>) -> Result { + if let Some(tz) = timezone { + let adjusted = js_date_with_offset(date, env, tz.offset_seconds)?; + return monty_datetime_from_js_date(adjusted, true, Some(tz.offset_seconds), tz.name.clone()); + } + + monty_datetime_from_js_date(date, false, None, None) +} + +fn monty_datetime_from_js_date( + date: Object<'_>, + utc_getters: bool, + offset_seconds: Option, + timezone_name: Option, +) -> Result { + let method = |local: &'static str, utc: &'static str| if utc_getters { utc } else { local }; + + Ok(MontyDateTime { + year: call_js_date_i32(date, method("getFullYear", "getUTCFullYear"))?, + month: u8::try_from(call_js_date_i32(date, method("getMonth", "getUTCMonth"))? + 1) + .map_err(|_| Error::from_reason("DateTime month is out of range"))?, + day: u8::try_from(call_js_date_i32(date, method("getDate", "getUTCDate"))?) + .map_err(|_| Error::from_reason("DateTime day is out of range"))?, + hour: u8::try_from(call_js_date_i32(date, method("getHours", "getUTCHours"))?) + .map_err(|_| Error::from_reason("DateTime hour is out of range"))?, + minute: u8::try_from(call_js_date_i32(date, method("getMinutes", "getUTCMinutes"))?) + .map_err(|_| Error::from_reason("DateTime minute is out of range"))?, + second: u8::try_from(call_js_date_i32(date, method("getSeconds", "getUTCSeconds"))?) + .map_err(|_| Error::from_reason("DateTime second is out of range"))?, + microsecond: u32::try_from(call_js_date_i32(date, method("getMilliseconds", "getUTCMilliseconds"))?) + .map_err(|_| Error::from_reason("DateTime millisecond is out of range"))? + * 1000, + offset_seconds, + timezone_name, + }) +} + +fn js_date_with_offset<'e>(date: Object<'_>, env: &'e Env, offset_seconds: i32) -> Result> { + let time_ms = call_js_date_f64(date, "getTime")?; + let adjusted_ms = time_ms + f64::from(offset_seconds) * 1000.0; + let global = env.get_global()?; + let date_constructor: Function = global.get_named_property("Date")?; + date_constructor.new_instance(adjusted_ms)?.coerce_to_object() +} + +fn call_js_date_i32(date: Object<'_>, method_name: &str) -> Result { + let value = call_js_date_f64(date, method_name)?; + #[expect( + clippy::cast_possible_truncation, + reason = "JavaScript Date component methods return small integers" + )] + Ok(value as i32) +} + +fn call_js_date_f64(date: Object<'_>, method_name: &str) -> Result { + let method: Function<()> = date.get_named_property(method_name)?; + let value: Unknown = method.apply(date, ())?; + value.coerce_to_number()?.get_double() +} + /// Converts a JS Map to `MontyObject::Dict`. fn js_map_to_monty(map: Object, env: Env) -> Result { // Get the entries iterator diff --git a/crates/monty-js/src/monty_cls.rs b/crates/monty-js/src/monty_cls.rs index 1629478f2..0010e42a8 100644 --- a/crates/monty-js/src/monty_cls.rs +++ b/crates/monty-js/src/monty_cls.rs @@ -43,12 +43,18 @@ //! console.log('Final result:', progress.output); //! ``` -use std::{borrow::Cow, fmt::Write, mem, ptr, result}; +use std::{ + borrow::Cow, + fmt::Write, + mem, ptr, result, + sync::{Arc, Mutex}, +}; use monty::{ fs::MountTable, ExcType, ExtFunctionResult, FunctionCall, LimitedTracker, MontyException, MontyObject, - MontyRepl as CoreMontyRepl, MontyRun, NameLookup, NameLookupResult, NoLimitTracker, OsCall, PrintWriter, - PrintWriterCallback, ReplProgress, ResourceTracker, RunProgress, + MontyRepl as CoreMontyRepl, MontyRun, NameLookup, NameLookupResult, NoLimitTracker, OsCall, OsFunctionCall, + PrintWriter, PrintWriterCallback, ReplFunctionCall, ReplNameLookup, ReplOsCall, ReplProgress, ReplStartError, + ResourceTracker, RunProgress, }; use monty_type_checking::{type_check, SourceFile}; use napi::{bindgen_prelude::*, sys::Status}; @@ -56,7 +62,7 @@ use napi_derive::napi; use serde::de::DeserializeOwned; use crate::{ - convert::{js_to_monty, monty_to_js, JsMontyObject}, + convert::{js_external_result_to_monty, js_to_monty, monty_to_js, JsMontyObject}, exceptions::{exc_js_to_monty, JsMontyException, MontyTypingError}, limits::JsResourceLimits, mount::{extract_mounts, OsHandler}, @@ -320,7 +326,12 @@ impl Monty { )); } RunProgress::OsCall(call) => { - let result = handle_os_call(&call, &mut mount_table); + let result = handle_os_call_with_external_functions( + env, + &call, + &mut mount_table, + external_functions.as_ref(), + )?; progress = match call.resume(result, print_output.reborrow()) { Ok(p) => p, Err(exc) => { @@ -518,6 +529,8 @@ enum EitherRepl { Limited(CoreMontyRepl), } +type SharedRepl = Arc>>; + /// Options for creating a new `MontyRepl` instance. /// /// Controls the script name shown in tracebacks and optional resource limits @@ -535,6 +548,8 @@ pub struct MontyReplOptions { #[napi(object)] #[derive(Default)] pub struct FeedOptions<'env> { + /// Optional print callback function. + pub print_callback: Option>, /// Filesystem mount(s) for the sandbox. /// A single `MountDir` or an array of `MountDir`. pub mount: Option>, @@ -546,7 +561,7 @@ pub struct FeedOptions<'env> { /// incrementally against persistent heap and namespace state. #[napi] pub struct MontyRepl { - repl: Option, + repl: SharedRepl, script_name: String, } @@ -571,7 +586,7 @@ impl MontyRepl { }; Ok(Self { - repl: Some(repl), + repl: Arc::new(Mutex::new(Some(repl))), script_name, }) } @@ -599,21 +614,28 @@ impl MontyRepl { Some(obj) => OsHandler::from_extracted(extract_mounts(obj)?), None => None, }; + let mut print_cb; + let print_writer = match &options.print_callback { + Some(func) => { + print_cb = CallbackStringPrint::new_js(env, func)?; + PrintWriter::Callback(&mut print_cb) + } + None => PrintWriter::Stdout, + }; // If mounts are provided, use feed_start/resume loop to handle OsCall if os_handler.is_some() { - return self.feed_with_mounts(env, code, os_handler); + let print_callback_ref = options.print_callback.as_ref().map(Function::create_ref).transpose()?; + return self.feed_with_mounts(env, code, os_handler, print_callback_ref); } - let repl = self - .repl - .as_mut() - .ok_or_else(|| Error::from_reason("REPL session is currently in use"))?; + let mut repl = take_shared_repl(&self.repl)?; - let output = match repl { - EitherRepl::NoLimit(repl) => repl.feed_run(&code, vec![], PrintWriter::Stdout), - EitherRepl::Limited(repl) => repl.feed_run(&code, vec![], PrintWriter::Stdout), + let output = match &mut repl { + EitherRepl::NoLimit(repl) => repl.feed_run(&code, vec![], print_writer), + EitherRepl::Limited(repl) => repl.feed_run(&code, vec![], print_writer), }; + put_shared_repl(&self.repl, repl)?; match output { Ok(value) => Ok(Either::A(monty_to_js(&value, env)?)), @@ -621,11 +643,75 @@ impl MontyRepl { } } + /// Starts executing one incremental snippet and returns a resumable progress object. + /// + /// This is the REPL equivalent of `Monty.start()`: it may return a + /// `MontySnapshot` for an external function or OS call, a `MontyNameLookup`, + /// or a `MontyComplete`. When the returned progress chain completes or + /// raises, the REPL state is restored so later `feed()` / `feedStart()` + /// calls can continue from the same session. + #[napi] + pub fn feed_start<'env>( + &self, + env: &'env Env, + code: String, + options: Option>, + ) -> Result> { + let options = options.unwrap_or_default(); + let os_handler = match options.mount.as_ref() { + Some(obj) => OsHandler::from_extracted(extract_mounts(obj)?), + None => None, + }; + let mount_table: Option = os_handler.as_ref().map(OsHandler::take).transpose()?; + let mount_state = os_handler.zip(mount_table); + let print_callback_ref = options.print_callback.as_ref().map(Function::create_ref).transpose()?; + + let repl = take_shared_repl(&self.repl)?; + let repl_state = Arc::clone(&self.repl); + + macro_rules! feed_start_impl { + ($repl:expr) => {{ + let mut print_cb = match &print_callback_ref { + Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), + None => None, + }; + let print_writer = match &mut print_cb { + Some(cb) => PrintWriter::Callback(cb), + None => PrintWriter::Stdout, + }; + let progress = match $repl.feed_start(&code, vec![], print_writer) { + Ok(p) => p, + Err(e) => { + put_shared_repl(&repl_state, EitherRepl::from_core(e.repl))?; + put_back_mount_state(mount_state); + return Ok(Either4::D(JsMontyException::new(e.error))); + } + }; + repl_progress_to_result_with_mounts( + env, + progress, + print_callback_ref, + self.script_name.clone(), + mount_state, + repl_state, + ) + }}; + } + + match repl { + EitherRepl::NoLimit(repl) => feed_start_impl!(repl), + EitherRepl::Limited(repl) => feed_start_impl!(repl), + } + } + /// Serializes this REPL session to bytes. #[napi] pub fn dump(&self) -> Result { - let repl = self + let guard = self .repl + .lock() + .map_err(|_| Error::from_reason("REPL session lock is poisoned"))?; + let repl = guard .as_ref() .ok_or_else(|| Error::from_reason("REPL session is currently in use"))?; let serialized = SerializedRepl { @@ -643,7 +729,7 @@ impl MontyRepl { let serialized: SerializedReplOwned = postcard::from_bytes(&data).map_err(|e| Error::from_reason(format!("Deserialization failed: {e}")))?; Ok(Self { - repl: Some(serialized.repl), + repl: Arc::new(Mutex::new(Some(serialized.repl))), script_name: serialized.script_name, }) } @@ -666,6 +752,7 @@ impl MontyRepl { env: &'env Env, code: String, os_handler: Option, + print_callback: Option, ) -> Result, JsMontyException>> { // Take mounts out of shared slots let mut mount_table: Option = os_handler.as_ref().map(OsHandler::take).transpose()?; @@ -677,18 +764,23 @@ impl MontyRepl { }; // Take the REPL out (feed_start consumes it) - let repl = self - .repl - .take() - .ok_or_else(|| Error::from_reason("REPL session is currently in use"))?; + let repl = take_shared_repl(&self.repl)?; macro_rules! feed_loop { ($repl:expr) => {{ - let progress = match $repl.feed_start(&code, vec![], PrintWriter::Stdout) { + let mut print_cb = match &print_callback { + Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), + None => None, + }; + let print_writer = match &mut print_cb { + Some(cb) => PrintWriter::Callback(cb), + None => PrintWriter::Stdout, + }; + let progress = match $repl.feed_start(&code, vec![], print_writer) { Ok(p) => p, Err(e) => { // Restore REPL from error - self.repl = Some(EitherRepl::from_core(e.repl)); + put_shared_repl(&self.repl, EitherRepl::from_core(e.repl))?; put_back(mount_table); return Ok(Either::B(JsMontyException::new(e.error))); } @@ -698,7 +790,7 @@ impl MontyRepl { loop { match progress { ReplProgress::Complete { repl, value } => { - self.repl = Some(EitherRepl::from_core(repl)); + put_shared_repl(&self.repl, EitherRepl::from_core(repl))?; put_back(mount_table); return Ok(Either::A(monty_to_js(&value, env)?)); } @@ -708,10 +800,14 @@ impl MontyRepl { } else { call.function_call.on_no_handler().into() }; - progress = match call.resume(os_result, PrintWriter::Stdout) { + let print_writer = match &mut print_cb { + Some(cb) => PrintWriter::Callback(cb), + None => PrintWriter::Stdout, + }; + progress = match call.resume(os_result, print_writer) { Ok(p) => p, Err(e) => { - self.repl = Some(EitherRepl::from_core(e.repl)); + put_shared_repl(&self.repl, EitherRepl::from_core(e.repl))?; put_back(mount_table); return Ok(Either::B(JsMontyException::new(e.error))); } @@ -720,7 +816,7 @@ impl MontyRepl { ReplProgress::FunctionCall(call) => { // No external function support in feed — return error let func_name = call.function_name.clone(); - self.repl = Some(EitherRepl::from_core(call.into_repl())); + put_shared_repl(&self.repl, EitherRepl::from_core(call.into_repl()))?; put_back(mount_table); return Ok(Either::B(JsMontyException::new(MontyException::new( ExcType::RuntimeError, @@ -731,17 +827,21 @@ impl MontyRepl { } ReplProgress::NameLookup(lookup) => { // No name lookup support — let it raise NameError - progress = match lookup.resume(NameLookupResult::Undefined, PrintWriter::Stdout) { + let print_writer = match &mut print_cb { + Some(cb) => PrintWriter::Callback(cb), + None => PrintWriter::Stdout, + }; + progress = match lookup.resume(NameLookupResult::Undefined, print_writer) { Ok(p) => p, Err(e) => { - self.repl = Some(EitherRepl::from_core(e.repl)); + put_shared_repl(&self.repl, EitherRepl::from_core(e.repl))?; put_back(mount_table); return Ok(Either::B(JsMontyException::new(e.error))); } }; } ReplProgress::ResolveFutures(state) => { - self.repl = Some(EitherRepl::from_core(state.into_repl())); + put_shared_repl(&self.repl, EitherRepl::from_core(state.into_repl()))?; put_back(mount_table); return Err(Error::from_reason( "Async futures are not supported in REPL feed()", @@ -759,16 +859,41 @@ impl MontyRepl { } } +fn take_shared_repl(shared: &SharedRepl) -> Result { + let mut guard = shared + .lock() + .map_err(|_| Error::from_reason("REPL session lock is poisoned"))?; + guard + .take() + .ok_or_else(|| Error::from_reason("REPL session is currently in use")) +} + +fn put_shared_repl(shared: &SharedRepl, repl: EitherRepl) -> Result<()> { + let mut guard = shared + .lock() + .map_err(|_| Error::from_reason("REPL session lock is poisoned"))?; + if guard.is_some() { + return Err(Error::from_reason("REPL session has already been restored")); + } + *guard = Some(repl); + Ok(()) +} + +fn restore_repl_start_error(shared: &SharedRepl, err: ReplStartError) -> Result +where + EitherRepl: FromCoreRepl, +{ + put_shared_repl(shared, EitherRepl::from_core(err.repl))?; + Ok(JsMontyException::new(err.error)) +} + /// Handles an OS call from the REPL using a mount table. fn handle_os_call_with_table_repl( - call: &monty::ReplOsCall, + call: &ReplOsCall, table: &mut MountTable, ) -> ExtFunctionResult { - match table.handle_os_call(&call.function_call) { - Some(Ok(obj)) => obj.into(), - Some(Err(mount_err)) => mount_err.into_exception().into(), - None => call.function_call.on_no_handler().into(), - } + handle_os_function_with_table(&call.function_call, table) + .unwrap_or_else(|| call.function_call.on_no_handler().into()) } /// Trait to convert a typed `CoreMontyRepl` back into `EitherRepl`. @@ -864,6 +989,12 @@ fn extract_input_values_in_order( enum EitherSnapshot { NoLimit(FunctionCall), Limited(FunctionCall), + NoLimitOs(OsCall), + LimitedOs(OsCall), + ReplNoLimitFn(ReplFunctionCall), + ReplNoLimitOs(ReplOsCall), + ReplLimitedFn(ReplFunctionCall), + ReplLimitedOs(ReplOsCall), /// Sentinel indicating the snapshot has been consumed via `resume()`. Done, } @@ -890,6 +1021,8 @@ pub struct MontySnapshot { script_name: String, /// The name of the external function being called. function_name: String, + /// Whether this snapshot represents an OS-level operation. + is_os_function: bool, /// The positional arguments passed to the function (stored as MontyObject for serialization). args: Vec, /// The keyword arguments passed to the function (stored as MontyObject pairs for serialization). @@ -898,6 +1031,8 @@ pub struct MontySnapshot { print_callback: Option, /// Mount state carried from `start()` for use during `resume()`. mount_state: Option, + /// REPL state to restore when a REPL feed-start chain completes or errors. + repl_state: Option, } /// Options for resuming execution. @@ -941,6 +1076,12 @@ impl MontySnapshot { self.function_name.clone() } + /// Returns true when this snapshot represents an OS-level operation. + #[napi(getter)] + pub fn is_os_function(&self) -> bool { + self.is_os_function + } + /// Returns the positional arguments passed to the external function. #[napi(getter)] pub fn args<'env>(&self, env: &'env Env) -> Result>> { @@ -979,7 +1120,7 @@ impl MontySnapshot { // Validate that exactly one of returnValue or exception is provided let external_result = match (options.return_value, options.exception) { (Some(value), None) => { - let monty_value = js_to_monty(value, *env)?; + let monty_value = js_external_result_to_monty(&self.function_name, &self.args, value, *env)?; ExtFunctionResult::Return(monty_value) } (None, Some(exc)) => { @@ -1016,6 +1157,7 @@ impl MontySnapshot { // Take mount state from the snapshot let mount_state = mem::take(&mut self.mount_state); + let repl_state = self.repl_state.clone(); // Resume execution based on the snapshot type match snapshot { @@ -1039,6 +1181,114 @@ impl MontySnapshot { }; progress_to_result_with_mounts(env, progress, print_callback, self.script_name.clone(), mount_state) } + EitherSnapshot::NoLimitOs(call) => { + let progress = match call.resume(external_result, print_writer) { + Ok(p) => p, + Err(exc) => { + put_back_mount_state(mount_state); + return Ok(Either4::D(JsMontyException::new(exc))); + } + }; + progress_to_result_with_mounts(env, progress, print_callback, self.script_name.clone(), mount_state) + } + EitherSnapshot::LimitedOs(call) => { + let progress = match call.resume(external_result, print_writer) { + Ok(p) => p, + Err(exc) => { + put_back_mount_state(mount_state); + return Ok(Either4::D(JsMontyException::new(exc))); + } + }; + progress_to_result_with_mounts(env, progress, print_callback, self.script_name.clone(), mount_state) + } + EitherSnapshot::ReplNoLimitFn(call) => { + let Some(repl_state) = repl_state else { + put_back_mount_state(mount_state); + return Err(Error::from_reason("REPL snapshot is missing its REPL owner")); + }; + let progress = match call.resume(external_result, print_writer) { + Ok(p) => p, + Err(exc) => { + let exc = restore_repl_start_error(&repl_state, *exc)?; + put_back_mount_state(mount_state); + return Ok(Either4::D(exc)); + } + }; + repl_progress_to_result_with_mounts( + env, + progress, + print_callback, + self.script_name.clone(), + mount_state, + repl_state, + ) + } + EitherSnapshot::ReplNoLimitOs(call) => { + let Some(repl_state) = repl_state else { + put_back_mount_state(mount_state); + return Err(Error::from_reason("REPL snapshot is missing its REPL owner")); + }; + let progress = match call.resume(external_result, print_writer) { + Ok(p) => p, + Err(exc) => { + let exc = restore_repl_start_error(&repl_state, *exc)?; + put_back_mount_state(mount_state); + return Ok(Either4::D(exc)); + } + }; + repl_progress_to_result_with_mounts( + env, + progress, + print_callback, + self.script_name.clone(), + mount_state, + repl_state, + ) + } + EitherSnapshot::ReplLimitedFn(call) => { + let Some(repl_state) = repl_state else { + put_back_mount_state(mount_state); + return Err(Error::from_reason("REPL snapshot is missing its REPL owner")); + }; + let progress = match call.resume(external_result, print_writer) { + Ok(p) => p, + Err(exc) => { + let exc = restore_repl_start_error(&repl_state, *exc)?; + put_back_mount_state(mount_state); + return Ok(Either4::D(exc)); + } + }; + repl_progress_to_result_with_mounts( + env, + progress, + print_callback, + self.script_name.clone(), + mount_state, + repl_state, + ) + } + EitherSnapshot::ReplLimitedOs(call) => { + let Some(repl_state) = repl_state else { + put_back_mount_state(mount_state); + return Err(Error::from_reason("REPL snapshot is missing its REPL owner")); + }; + let progress = match call.resume(external_result, print_writer) { + Ok(p) => p, + Err(exc) => { + let exc = restore_repl_start_error(&repl_state, *exc)?; + put_back_mount_state(mount_state); + return Ok(Either4::D(exc)); + } + }; + repl_progress_to_result_with_mounts( + env, + progress, + print_callback, + self.script_name.clone(), + mount_state, + repl_state, + ) + } EitherSnapshot::Done => Err(Error::from_reason("Snapshot has already been resumed")), } } @@ -1054,6 +1304,17 @@ impl MontySnapshot { if matches!(self.snapshot, EitherSnapshot::Done) { return Err(Error::from_reason("Cannot dump snapshot that has already been resumed")); } + if matches!( + self.snapshot, + EitherSnapshot::ReplNoLimitFn(_) + | EitherSnapshot::ReplNoLimitOs(_) + | EitherSnapshot::ReplLimitedFn(_) + | EitherSnapshot::ReplLimitedOs(_) + ) { + return Err(Error::from_reason( + "Cannot dump REPL snapshots from the JS bindings; dump the MontyRepl after completion instead", + )); + } let serialized = SerializedSnapshot { snapshot: &self.snapshot, @@ -1077,11 +1338,16 @@ impl MontySnapshot { pub fn load(data: Buffer, options: Option) -> Result { let serialized: SerializedSnapshotOwned = postcard::from_bytes(&data).map_err(|e| Error::from_reason(format!("Deserialization failed: {e}")))?; + let is_os_function = matches!( + &serialized.snapshot, + EitherSnapshot::NoLimitOs(_) | EitherSnapshot::LimitedOs(_) + ); Ok(Self { snapshot: serialized.snapshot, script_name: serialized.script_name, function_name: serialized.function_name, + is_os_function, args: serialized.args, kwargs: serialized.kwargs, print_callback: options @@ -1090,6 +1356,7 @@ impl MontySnapshot { .map(Function::create_ref) .transpose()?, mount_state: None, + repl_state: None, }) } @@ -1144,6 +1411,8 @@ impl MontyComplete { enum EitherLookupSnapshot { NoLimit(NameLookup), Limited(NameLookup), + ReplNoLimit(ReplNameLookup), + ReplLimited(ReplNameLookup), /// Sentinel indicating the snapshot has been consumed via `resume()`. Done, } @@ -1166,6 +1435,24 @@ impl FromLookupSnapshot for EitherLookupSnapshot { } } +/// Trait to convert a typed `ReplNameLookup` into `EitherLookupSnapshot`. +trait FromReplLookupSnapshot { + /// Wraps a REPL name-lookup snapshot. + fn from_repl_lookup(lookup: ReplNameLookup) -> Self; +} + +impl FromReplLookupSnapshot for EitherLookupSnapshot { + fn from_repl_lookup(lookup: ReplNameLookup) -> Self { + Self::ReplNoLimit(lookup) + } +} + +impl FromReplLookupSnapshot for EitherLookupSnapshot { + fn from_repl_lookup(lookup: ReplNameLookup) -> Self { + Self::ReplLimited(lookup) + } +} + // ============================================================================= // MontyNameLookup - Paused execution at a name lookup // ============================================================================= @@ -1187,6 +1474,8 @@ pub struct MontyNameLookup { print_callback: Option, /// Mount state carried from `start()` for use during `resume()`. mount_state: Option, + /// REPL state to restore when a REPL feed-start chain completes or errors. + repl_state: Option, } /// Options for resuming execution from a name lookup. @@ -1260,6 +1549,7 @@ impl MontyNameLookup { // Take mount state from the snapshot let mount_state = mem::take(&mut self.mount_state); + let repl_state = self.repl_state.clone(); match snapshot { EitherLookupSnapshot::NoLimit(lookup) => { @@ -1282,6 +1572,50 @@ impl MontyNameLookup { }; progress_to_result_with_mounts(env, progress, print_callback, self.script_name.clone(), mount_state) } + EitherLookupSnapshot::ReplNoLimit(lookup) => { + let Some(repl_state) = repl_state else { + put_back_mount_state(mount_state); + return Err(Error::from_reason("REPL name lookup is missing its REPL owner")); + }; + let progress = match lookup.resume(lookup_result, print_writer) { + Ok(p) => p, + Err(exc) => { + let exc = restore_repl_start_error(&repl_state, *exc)?; + put_back_mount_state(mount_state); + return Ok(Either4::D(exc)); + } + }; + repl_progress_to_result_with_mounts( + env, + progress, + print_callback, + self.script_name.clone(), + mount_state, + repl_state, + ) + } + EitherLookupSnapshot::ReplLimited(lookup) => { + let Some(repl_state) = repl_state else { + put_back_mount_state(mount_state); + return Err(Error::from_reason("REPL name lookup is missing its REPL owner")); + }; + let progress = match lookup.resume(lookup_result, print_writer) { + Ok(p) => p, + Err(exc) => { + let exc = restore_repl_start_error(&repl_state, *exc)?; + put_back_mount_state(mount_state); + return Ok(Either4::D(exc)); + } + }; + repl_progress_to_result_with_mounts( + env, + progress, + print_callback, + self.script_name.clone(), + mount_state, + repl_state, + ) + } EitherLookupSnapshot::Done => Err(Error::from_reason("Name lookup has already been resumed")), } } @@ -1298,6 +1632,14 @@ impl MontyNameLookup { "Cannot dump name lookup that has already been resumed", )); } + if matches!( + self.snapshot, + EitherLookupSnapshot::ReplNoLimit(_) | EitherLookupSnapshot::ReplLimited(_) + ) { + return Err(Error::from_reason( + "Cannot dump REPL name lookups from the JS bindings; dump the MontyRepl after completion instead", + )); + } let serialized = SerializedNameLookup { snapshot: &self.snapshot, @@ -1330,6 +1672,7 @@ impl MontyNameLookup { .map(Function::create_ref) .transpose()?, mount_state: None, + repl_state: None, }) } @@ -1401,7 +1744,7 @@ fn progress_to_result_with_mounts( ) -> Result> where T: ResourceTracker + serde::Serialize + DeserializeOwned, - EitherSnapshot: FromSnapshot, + EitherSnapshot: FromSnapshot + FromOsSnapshot, EitherLookupSnapshot: FromLookupSnapshot, { // Build a reusable print callback so OsCall resumes use the JS callback @@ -1426,10 +1769,12 @@ where snapshot: EitherSnapshot::from_snapshot(call), script_name, function_name, + is_os_function: false, args, kwargs, print_callback, mount_state, + repl_state: None, })); } RunProgress::NameLookup(lookup) => { @@ -1440,6 +1785,7 @@ where variable_name, print_callback, mount_state, + repl_state: None, })); } RunProgress::ResolveFutures(_) => { @@ -1453,20 +1799,135 @@ where let os_result = if let Some((_, ref mut table)) = mount_state { handle_os_call_with_table(&call, table) } else { - // No mounts configured — use on_no_handler for appropriate error - call.function_call.on_no_handler().into() + None }; - let print_writer = match &mut print_cb { - Some(cb) => PrintWriter::Callback(cb), - None => PrintWriter::Stdout, - }; - progress = match call.resume(os_result, print_writer) { - Ok(p) => p, - Err(exc) => { - put_back_mount_state(mount_state); - return Ok(Either4::D(JsMontyException::new(exc))); - } + if let Some(os_result) = os_result { + let print_writer = match &mut print_cb { + Some(cb) => PrintWriter::Callback(cb), + None => PrintWriter::Stdout, + }; + progress = match call.resume(os_result, print_writer) { + Ok(p) => p, + Err(exc) => { + put_back_mount_state(mount_state); + return Ok(Either4::D(JsMontyException::new(exc))); + } + }; + } else { + let function_name = call.function_call.name().to_owned(); + let (args, kwargs) = call.function_call.clone().to_args(); + return Ok(Either4::A(MontySnapshot { + snapshot: EitherSnapshot::from_os_snapshot(call), + script_name, + function_name, + is_os_function: true, + args, + kwargs, + print_callback, + mount_state, + repl_state: None, + })); + } + } + } + } +} + +fn repl_progress_to_result_with_mounts( + env: &Env, + mut progress: ReplProgress, + print_callback: Option, + script_name: String, + mut mount_state: Option, + repl_state: SharedRepl, +) -> Result> +where + T: ResourceTracker + serde::Serialize + DeserializeOwned, + EitherRepl: FromCoreRepl, + EitherSnapshot: FromReplSnapshot + FromReplOsSnapshot, + EitherLookupSnapshot: FromReplLookupSnapshot, +{ + let mut print_cb = match &print_callback { + Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), + None => None, + }; + + loop { + match progress { + ReplProgress::Complete { repl, value } => { + put_shared_repl(&repl_state, EitherRepl::from_core(repl))?; + put_back_mount_state(mount_state); + return Ok(Either4::C(MontyComplete { output_value: value })); + } + ReplProgress::FunctionCall(call) => { + let function_name = call.function_name.clone(); + let args = call.args.clone(); + let kwargs = call.kwargs.clone(); + return Ok(Either4::A(MontySnapshot { + snapshot: EitherSnapshot::from_repl_snapshot(call), + script_name, + function_name, + is_os_function: false, + args, + kwargs, + print_callback, + mount_state, + repl_state: Some(repl_state), + })); + } + ReplProgress::NameLookup(lookup) => { + let variable_name = lookup.name.clone(); + return Ok(Either4::B(MontyNameLookup { + snapshot: EitherLookupSnapshot::from_repl_lookup(lookup), + script_name, + variable_name, + print_callback, + mount_state, + repl_state: Some(repl_state), + })); + } + ReplProgress::ResolveFutures(state) => { + put_shared_repl(&repl_state, EitherRepl::from_core(state.into_repl()))?; + put_back_mount_state(mount_state); + return Ok(Either4::D(JsMontyException::new(MontyException::new( + ExcType::NotImplementedError, + Some("Async futures (ResolveFutures) are not yet supported in the JS bindings".to_owned()), + )))); + } + ReplProgress::OsCall(call) => { + let os_result = if let Some((_, ref mut table)) = mount_state { + handle_repl_os_call_with_table(&call, table) + } else { + None }; + if let Some(os_result) = os_result { + let print_writer = match &mut print_cb { + Some(cb) => PrintWriter::Callback(cb), + None => PrintWriter::Stdout, + }; + progress = match call.resume(os_result, print_writer) { + Ok(p) => p, + Err(exc) => { + let exc = restore_repl_start_error(&repl_state, *exc)?; + put_back_mount_state(mount_state); + return Ok(Either4::D(exc)); + } + }; + } else { + let function_name = call.function_call.name().to_owned(); + let (args, kwargs) = call.function_call.clone().to_args(); + return Ok(Either4::A(MontySnapshot { + snapshot: EitherSnapshot::from_repl_os_snapshot(call), + script_name, + function_name, + is_os_function: true, + args, + kwargs, + print_callback, + mount_state, + repl_state: Some(repl_state), + })); + } } } } @@ -1479,25 +1940,51 @@ fn put_back_mount_state(mount_state: Option) { } } -/// Handles an OS call by dispatching to the mount table. -/// -/// Returns the result from the mount table if it handles the call, -/// or falls back to [`OsFunctionCall::on_no_handler`] for unhandled calls. -fn handle_os_call(call: &OsCall, mount_table: &mut Option) -> ExtFunctionResult { - if let Some(table) = mount_table { - handle_os_call_with_table(call, table) - } else { - call.function_call.on_no_handler().into() +/// Handles an OS call using a specific mount table. +fn handle_os_call_with_table( + call: &OsCall, + table: &mut MountTable, +) -> Option { + handle_os_function_with_table(&call.function_call, table) +} + +/// Handles a REPL OS call using a specific mount table. +fn handle_repl_os_call_with_table( + call: &ReplOsCall, + table: &mut MountTable, +) -> Option { + handle_os_function_with_table(&call.function_call, table) +} + +fn handle_os_function_with_table(call: &OsFunctionCall, table: &mut MountTable) -> Option { + match table.handle_os_call(call) { + Some(Ok(obj)) => Some(obj.into()), + Some(Err(mount_err)) => Some(mount_err.into_exception().into()), + None => return None, } } -/// Handles an OS call using a specific mount table. -fn handle_os_call_with_table(call: &OsCall, table: &mut MountTable) -> ExtFunctionResult { - match table.handle_os_call(&call.function_call) { - Some(Ok(obj)) => obj.into(), - Some(Err(mount_err)) => mount_err.into_exception().into(), - None => call.function_call.on_no_handler().into(), +fn handle_os_call_with_external_functions( + env: &Env, + call: &OsCall, + mount_table: &mut Option, + external_functions: Option<&Object<'_>>, +) -> Result { + if let Some(table) = mount_table.as_mut() { + if let Some(result) = handle_os_call_with_table(call, table) { + return Ok(result); + } + } + + let function_name = call.function_call.name(); + if let Some(functions) = external_functions { + if functions.has_named_property(function_name)? { + let (args, kwargs) = call.function_call.clone().to_args(); + return call_external_function(env, Some(functions), function_name, &args, &kwargs); + } } + + Ok(call.function_call.on_no_handler().into()) } /// Trait to convert a typed `FunctionCall` into `EitherSnapshot`. @@ -1518,6 +2005,60 @@ impl FromSnapshot for EitherSnapshot { } } +/// Trait to convert a typed `OsCall` into `EitherSnapshot`. +trait FromOsSnapshot { + /// Wraps an OS-call snapshot. + fn from_os_snapshot(call: OsCall) -> Self; +} + +impl FromOsSnapshot for EitherSnapshot { + fn from_os_snapshot(call: OsCall) -> Self { + Self::NoLimitOs(call) + } +} + +impl FromOsSnapshot for EitherSnapshot { + fn from_os_snapshot(call: OsCall) -> Self { + Self::LimitedOs(call) + } +} + +/// Trait to convert a typed `ReplFunctionCall` into `EitherSnapshot`. +trait FromReplSnapshot { + /// Wraps a REPL function-call snapshot. + fn from_repl_snapshot(call: ReplFunctionCall) -> Self; +} + +impl FromReplSnapshot for EitherSnapshot { + fn from_repl_snapshot(call: ReplFunctionCall) -> Self { + Self::ReplNoLimitFn(call) + } +} + +impl FromReplSnapshot for EitherSnapshot { + fn from_repl_snapshot(call: ReplFunctionCall) -> Self { + Self::ReplLimitedFn(call) + } +} + +/// Trait to convert a typed `ReplOsCall` into `EitherSnapshot`. +trait FromReplOsSnapshot { + /// Wraps a REPL OS-call snapshot. + fn from_repl_os_snapshot(call: ReplOsCall) -> Self; +} + +impl FromReplOsSnapshot for EitherSnapshot { + fn from_repl_os_snapshot(call: ReplOsCall) -> Self { + Self::ReplNoLimitOs(call) + } +} + +impl FromReplOsSnapshot for EitherSnapshot { + fn from_repl_os_snapshot(call: ReplOsCall) -> Self { + Self::ReplLimitedOs(call) + } +} + /// Converts a string exception type to `ExcType`. fn string_to_exc_type(type_name: &str) -> Result { type_name @@ -1693,7 +2234,7 @@ fn call_external_function( // Convert the result back to Monty format // SAFETY: [DH] - result_raw is valid on success let result = unsafe { Unknown::from_raw_unchecked(env.raw(), result_raw) }; - let monty_result = js_to_monty(result, *env)?; + let monty_result = js_external_result_to_monty(function_name, args, result, *env)?; Ok(ExtFunctionResult::Return(monty_result)) } diff --git a/crates/monty-js/wrapper.ts b/crates/monty-js/wrapper.ts index 660024a9a..439e5f664 100644 --- a/crates/monty-js/wrapper.ts +++ b/crates/monty-js/wrapper.ts @@ -57,6 +57,8 @@ export interface StartOptions extends Omit { /** Options for REPL feed(). */ export interface FeedOptions extends Omit { + /** Callback invoked on each print() call. */ + printCallback?: (stream: string, text: string) => void /** Filesystem mount(s) for the sandbox. */ mount?: MountDir | MountDir[] } @@ -421,6 +423,20 @@ export class MontyRepl { return result } + /** + * Starts one incremental snippet and returns a resumable progress object. + * + * @param code - Snippet code to execute + * @param options - Optional feed options (mount, printCallback) + * @returns MontySnapshot if paused at function/OS call, MontyNameLookup if + * paused at name lookup, MontyComplete if done + * @throws {MontyRuntimeError} If execution raises an exception + */ + feedStart(code: string, options?: FeedOptions): MontySnapshot | MontyNameLookup | MontyComplete { + const result = this._native.feedStart(code, options) + return wrapStartResult(result) + } + /** Serializes the REPL session to bytes. */ dump(): Buffer { return this._native.dump() @@ -486,6 +502,11 @@ export class MontySnapshot { return this._native.functionName } + /** Returns true when this snapshot represents an OS-level operation. */ + get isOsFunction(): boolean { + return this._native.isOsFunction + } + /** Returns the positional arguments passed to the external function. */ get args(): JsMontyObject[] { return this._native.args @@ -687,13 +708,17 @@ export async function runMontyAsync(montyRunner: Monty, options: RunMontyAsyncOp const extFunction = externalFunctions[funcName] if (!extFunction) { - // Function not found — this shouldn't normally happen since NameLookup - // would have raised NameError, but handle it defensively + const exception = snapshot.isOsFunction + ? { + type: 'RuntimeError', + message: `'${funcName}' is not supported in this environment`, + } + : { + type: 'NameError', + message: `name '${funcName}' is not defined`, + } progress = snapshot.resume({ - exception: { - type: 'NameError', - message: `name '${funcName}' is not defined`, - }, + exception, }) continue } From cd70f8dace12b718e4c367fa94bb7a976a8dd6c2 Mon Sep 17 00:00:00 2001 From: Steve Korshakov Date: Mon, 1 Jun 2026 18:25:36 -0700 Subject: [PATCH 2/3] Fix JS OS date and REPL restore errors --- crates/monty-js/__test__/external.spec.ts | 17 ++++++ crates/monty-js/__test__/repl.spec.ts | 19 ++++++- crates/monty-js/src/convert.rs | 13 ++++- crates/monty-js/src/monty_cls.rs | 65 +++++++++++++++++------ 4 files changed, 95 insertions(+), 19 deletions(-) diff --git a/crates/monty-js/__test__/external.spec.ts b/crates/monty-js/__test__/external.spec.ts index 2b9208578..39aed78eb 100644 --- a/crates/monty-js/__test__/external.spec.ts +++ b/crates/monty-js/__test__/external.spec.ts @@ -159,6 +159,23 @@ now = datetime.now() t.deepEqual(result, [2026, 1, 2, 3, 4, 5, 6000]) }) +test('OS datetime.now rejects invalid JS Date results', (t) => { + const m = new Monty(` +from datetime import datetime +now = datetime.now() +now.year +`) + + const error = t.throws(() => + m.run({ + externalFunctions: { + 'datetime.now': () => new Date(Number.NaN), + }, + }), + ) + t.true(error?.message.includes('Date method getFullYear returned an invalid number')) +}) + // ============================================================================= // Error handling tests // ============================================================================= diff --git a/crates/monty-js/__test__/repl.spec.ts b/crates/monty-js/__test__/repl.spec.ts index fb1999dab..2082d101e 100644 --- a/crates/monty-js/__test__/repl.spec.ts +++ b/crates/monty-js/__test__/repl.spec.ts @@ -1,6 +1,6 @@ import test from 'ava' -import { MontyComplete, MontyRepl, MontySnapshot } from '../wrapper' +import { MontyComplete, MontyRepl, MontyRuntimeError, MontySnapshot } from '../wrapper' test('feed preserves state without replay', (t) => { const repl = new MontyRepl() @@ -60,3 +60,20 @@ test('feedStart pauses and resumes OS calls', (t) => { t.is((progress as MontyComplete).output, 2032) t.is(repl.feed('stamp.month'), 5) }) + +test('feedStart restores REPL after print callback error', (t) => { + const repl = new MontyRepl() + + const error = t.throws( + () => + repl.feedStart('value = 41\nprint(value)', { + printCallback: () => { + throw new Error('print failed') + }, + }), + { instanceOf: MontyRuntimeError }, + ) + t.true(error.message.includes('print failed')) + t.is(repl.feed('value = 41'), null) + t.is(repl.feed('value + 1'), 42) +}) diff --git a/crates/monty-js/src/convert.rs b/crates/monty-js/src/convert.rs index 035d200df..fa06d644f 100644 --- a/crates/monty-js/src/convert.rs +++ b/crates/monty-js/src/convert.rs @@ -624,6 +624,11 @@ fn js_date_with_offset<'e>(date: Object<'_>, env: &'e Env, offset_seconds: i32) fn call_js_date_i32(date: Object<'_>, method_name: &str) -> Result { let value = call_js_date_f64(date, method_name)?; + if value.fract() != 0.0 || value < f64::from(i32::MIN) || value > f64::from(i32::MAX) { + return Err(Error::from_reason(format!( + "Date method {method_name} returned an out-of-range integer" + ))); + } #[expect( clippy::cast_possible_truncation, reason = "JavaScript Date component methods return small integers" @@ -634,7 +639,13 @@ fn call_js_date_i32(date: Object<'_>, method_name: &str) -> Result { fn call_js_date_f64(date: Object<'_>, method_name: &str) -> Result { let method: Function<()> = date.get_named_property(method_name)?; let value: Unknown = method.apply(date, ())?; - value.coerce_to_number()?.get_double() + let value = value.coerce_to_number()?.get_double()?; + if !value.is_finite() { + return Err(Error::from_reason(format!( + "Date method {method_name} returned an invalid number" + ))); + } + Ok(value) } /// Converts a JS Map to `MontyObject::Dict`. diff --git a/crates/monty-js/src/monty_cls.rs b/crates/monty-js/src/monty_cls.rs index 0010e42a8..9c7e580f4 100644 --- a/crates/monty-js/src/monty_cls.rs +++ b/crates/monty-js/src/monty_cls.rs @@ -658,28 +658,38 @@ impl MontyRepl { options: Option>, ) -> Result> { let options = options.unwrap_or_default(); + let print_callback_ref = options.print_callback.as_ref().map(Function::create_ref).transpose()?; + let mut print_cb = match &print_callback_ref { + Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), + None => None, + }; let os_handler = match options.mount.as_ref() { Some(obj) => OsHandler::from_extracted(extract_mounts(obj)?), None => None, }; let mount_table: Option = os_handler.as_ref().map(OsHandler::take).transpose()?; let mount_state = os_handler.zip(mount_table); - let print_callback_ref = options.print_callback.as_ref().map(Function::create_ref).transpose()?; - let repl = take_shared_repl(&self.repl)?; + let repl = match take_shared_repl(&self.repl) { + Ok(repl) => repl, + Err(err) => { + put_back_mount_state(mount_state); + return Err(err); + } + }; let repl_state = Arc::clone(&self.repl); macro_rules! feed_start_impl { ($repl:expr) => {{ - let mut print_cb = match &print_callback_ref { - Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), - None => None, - }; - let print_writer = match &mut print_cb { - Some(cb) => PrintWriter::Callback(cb), - None => PrintWriter::Stdout, + let progress = { + let print_writer = match &mut print_cb { + Some(cb) => PrintWriter::Callback(cb), + None => PrintWriter::Stdout, + }; + $repl.feed_start(&code, vec![], print_writer) }; - let progress = match $repl.feed_start(&code, vec![], print_writer) { + drop(print_cb); + let progress = match progress { Ok(p) => p, Err(e) => { put_shared_repl(&repl_state, EitherRepl::from_core(e.repl))?; @@ -754,6 +764,11 @@ impl MontyRepl { os_handler: Option, print_callback: Option, ) -> Result, JsMontyException>> { + let mut print_cb = match &print_callback { + Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), + None => None, + }; + // Take mounts out of shared slots let mut mount_table: Option = os_handler.as_ref().map(OsHandler::take).transpose()?; @@ -764,14 +779,16 @@ impl MontyRepl { }; // Take the REPL out (feed_start consumes it) - let repl = take_shared_repl(&self.repl)?; + let repl = match take_shared_repl(&self.repl) { + Ok(repl) => repl, + Err(err) => { + put_back(mount_table); + return Err(err); + } + }; macro_rules! feed_loop { ($repl:expr) => {{ - let mut print_cb = match &print_callback { - Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), - None => None, - }; let print_writer = match &mut print_cb { Some(cb) => PrintWriter::Callback(cb), None => PrintWriter::Stdout, @@ -1750,7 +1767,13 @@ where // Build a reusable print callback so OsCall resumes use the JS callback // instead of falling back to stdout. let mut print_cb = match &print_callback { - Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), + Some(func) => match CallbackStringPrint::new_js_ref(env, func) { + Ok(cb) => Some(cb), + Err(err) => { + put_back_mount_state(mount_state); + return Err(err); + } + }, None => None, }; @@ -1848,7 +1871,15 @@ where EitherLookupSnapshot: FromReplLookupSnapshot, { let mut print_cb = match &print_callback { - Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), + Some(func) => match CallbackStringPrint::new_js_ref(env, func) { + Ok(cb) => Some(cb), + Err(err) => { + let repl = EitherRepl::from_core(progress.into_repl()); + put_back_mount_state(mount_state); + put_shared_repl(&repl_state, repl)?; + return Err(err); + } + }, None => None, }; From 407a8b19bc9cb4ed29201c432853d65d1f324d77 Mon Sep 17 00:00:00 2001 From: Steve Korshakov Date: Mon, 1 Jun 2026 18:36:55 -0700 Subject: [PATCH 3/3] Fix Rust lint in JS REPL restore path --- crates/monty-js/src/monty_cls.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/monty-js/src/monty_cls.rs b/crates/monty-js/src/monty_cls.rs index 9c7e580f4..fac417f59 100644 --- a/crates/monty-js/src/monty_cls.rs +++ b/crates/monty-js/src/monty_cls.rs @@ -659,7 +659,7 @@ impl MontyRepl { ) -> Result> { let options = options.unwrap_or_default(); let print_callback_ref = options.print_callback.as_ref().map(Function::create_ref).transpose()?; - let mut print_cb = match &print_callback_ref { + let print_cb = match &print_callback_ref { Some(func) => Some(CallbackStringPrint::new_js_ref(env, func)?), None => None, }; @@ -682,13 +682,13 @@ impl MontyRepl { macro_rules! feed_start_impl { ($repl:expr) => {{ let progress = { + let mut print_cb = print_cb; let print_writer = match &mut print_cb { Some(cb) => PrintWriter::Callback(cb), None => PrintWriter::Stdout, }; $repl.feed_start(&code, vec![], print_writer) }; - drop(print_cb); let progress = match progress { Ok(p) => p, Err(e) => { @@ -1991,7 +1991,7 @@ fn handle_os_function_with_table(call: &OsFunctionCall, table: &mut MountTable) match table.handle_os_call(call) { Some(Ok(obj)) => Some(obj.into()), Some(Err(mount_err)) => Some(mount_err.into_exception().into()), - None => return None, + None => None, } }