From 656665897a5d98afd4c61055f27a44fc4f157364 Mon Sep 17 00:00:00 2001 From: Shivampal157 Date: Mon, 2 Mar 2026 23:32:49 +0530 Subject: [PATCH 1/2] fix(cli): handle recoverable errors gracefully and remove panics #4798 --- cli/src/debug/mod.rs | 6 ++++-- cli/src/helper.rs | 14 +++++++++----- cli/src/logger.rs | 4 ++-- cli/src/main.rs | 35 ++++++++++++++++++++++------------- 4 files changed, 37 insertions(+), 22 deletions(-) diff --git a/cli/src/debug/mod.rs b/cli/src/debug/mod.rs index 0cb3d585d9e..eb4f7f49042 100644 --- a/cli/src/debug/mod.rs +++ b/cli/src/debug/mod.rs @@ -2,6 +2,7 @@ #![allow(clippy::unnecessary_wraps)] use boa_engine::{Context, JsObject, js_string, object::ObjectInitializer, property::Attribute}; +use color_eyre::Result; mod function; mod gc; @@ -67,7 +68,7 @@ fn create_boa_object(context: &mut Context) -> JsObject { } #[allow(clippy::redundant_pub_crate)] -pub(crate) fn init_boa_debug_object(context: &mut Context) { +pub(crate) fn init_boa_debug_object(context: &mut Context) -> Result<()> { let boa_object = create_boa_object(context); context .register_global_property( @@ -75,5 +76,6 @@ pub(crate) fn init_boa_debug_object(context: &mut Context) { boa_object, Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, ) - .expect("cannot fail with the default object"); + .map_err(|e| color_eyre::eyre::eyre!("failed to register debug object: {e}"))?; + Ok(()) } diff --git a/cli/src/helper.rs b/cli/src/helper.rs index cbab1bf5191..928a779f223 100644 --- a/cli/src/helper.rs +++ b/cli/src/helper.rs @@ -140,12 +140,13 @@ static KEYWORDS: Set<&'static str> = phf_set! { }; struct LineHighlighter { - regex: Regex, + regex: Option, } impl LineHighlighter { fn new() -> Self { - // Precompiles the regex to avoid creating it again after every highlight + // Precompiles the regex to avoid creating it again after every highlight. + // If compilation fails (e.g. unsupported Unicode on some platforms), we fall back to no highlighting. let regex = Regex::new( r#"(?x) (?P\b[$_\p{ID_Start}][$_\p{ID_Continue}\u{200C}\u{200D}]*\b) | @@ -154,7 +155,7 @@ impl LineHighlighter { (?P`([^`\\]|\\.)*`) | (?P[+\-/*%~^!&|=<>;:]) | (?P0[bB][01](_?[01])*n?|0[oO][0-7](_?[0-7])*n?|0[xX][0-9a-fA-F](_?[0-9a-fA-F])*n?|(([0-9](_?[0-9])*\.([0-9](_?[0-9])*)?)|(([0-9](_?[0-9])*)?\.[0-9](_?[0-9])*)|([0-9](_?[0-9])*))([eE][+-]?[0-9](_?[0-9])*)?n?)"#, - ).expect("could not compile regular expression"); + ).ok(); Self { regex } } @@ -200,12 +201,15 @@ impl Highlighter for LineHighlighter { }; if let Some(colored) = colored { - write!(dst, "{colored}").expect("could not append data to dst"); + let _ = write!(dst, "{colored}"); } else { dst.push_str(&caps[0]); } } } - self.regex.replace_all(line, Colorizer) + match &self.regex { + Some(regex) => regex.replace_all(line, Colorizer), + None => Borrowed(line), + } } } diff --git a/cli/src/logger.rs b/cli/src/logger.rs index 658fab4db0d..1a667b4911a 100644 --- a/cli/src/logger.rs +++ b/cli/src/logger.rs @@ -21,12 +21,12 @@ impl SharedExternalPrinterLogger { pub(crate) fn set(&self, inner: T) { self.inner .lock() - .expect("printer lock failed") + .unwrap_or_else(|e| e.into_inner()) .replace(Box::new(inner)); } pub(crate) fn print(&self, message: String) { - if let Some(l) = &mut *self.inner.lock().expect("printer lock failed") { + if let Some(l) = &mut *self.inner.lock().unwrap_or_else(|e| e.into_inner()) { // Ignore errors, there's nothing we can do at this point. drop(l.print(message)); } else { diff --git a/cli/src/main.rs b/cli/src/main.rs index adb9d79b542..b0b6d437fa4 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -303,9 +303,9 @@ fn dump(src: Source<'_, R>, args: &Opt, context: &mut Context) -> R let _timer = counters.new_timer("AST generation"); match arg { DumpFormat::Json => serde_json::to_string(&module) - .expect("could not convert AST to a JSON string"), + .wrap_err("could not convert AST to a JSON string")?, DumpFormat::JsonPretty => serde_json::to_string_pretty(&module) - .expect("could not convert AST to a pretty JSON string"), + .wrap_err("could not convert AST to a pretty JSON string")?, DumpFormat::Debug => format!("{module:#?}"), } } else { @@ -324,9 +324,9 @@ fn dump(src: Source<'_, R>, args: &Opt, context: &mut Context) -> R let _timer = counters.new_timer("AST generation"); match arg { DumpFormat::Json => serde_json::to_string(&script) - .expect("could not convert AST to a JSON string"), + .wrap_err("could not convert AST to a JSON string")?, DumpFormat::JsonPretty => serde_json::to_string_pretty(&script) - .expect("could not convert AST to a pretty JSON string"), + .wrap_err("could not convert AST to a pretty JSON string")?, DumpFormat::Debug => format!("{script:#?}"), } }; @@ -431,13 +431,17 @@ fn evaluate_file( printer: &SharedExternalPrinterLogger, ) -> Result<()> { if args.has_dump_flag() { - return dump(Source::from_filepath(file)?, args, context); + return dump( + Source::from_filepath(file).wrap_err_with(|| format!("failed to read file '{}'", file.display()))?, + args, + context, + ); } if let Some(flowgraph) = args.flowgraph { let flowgraph = generate_flowgraph( context, - Source::from_filepath(file)?, + Source::from_filepath(file).wrap_err_with(|| format!("failed to read file '{}'", file.display()))?, flowgraph.unwrap_or(FlowgraphFormat::Graphviz), args.flowgraph_direction, )?; @@ -448,7 +452,8 @@ fn evaluate_file( } if args.module { - let source = Source::from_filepath(file)?; + let source = Source::from_filepath(file) + .wrap_err_with(|| format!("failed to read file '{}'", file.display()))?; let mut counters = Counters::new(args.time); let module = { let _timer = counters.new_timer("Parsing"); @@ -479,7 +484,8 @@ fn evaluate_file( }; } - let source = Source::from_filepath(file)?; + let source = Source::from_filepath(file) + .wrap_err_with(|| format!("failed to read file '{}'", file.display()))?; let mut counters = Counters::new(args.time); let script = { let _timer = counters.new_timer("Parsing"); @@ -545,13 +551,13 @@ fn main() -> Result<()> { context.strict(args.strict); // Add `console`. - add_runtime(printer.clone(), &mut context); + add_runtime(printer.clone(), &mut context)?; // Trace Output context.set_trace(args.trace); if args.debug_object { - init_boa_debug_object(&mut context); + init_boa_debug_object(&mut context)?; } // Configure optimizer options @@ -600,7 +606,9 @@ fn main() -> Result<()> { thread::sleep(Duration::from_millis(10)); } - handle.join().expect("failed to join thread"); + handle + .join() + .map_err(|_| eyre!("readline thread panicked"))?; Ok(()) } @@ -682,7 +690,7 @@ fn start_readline_thread( } /// Adds the CLI runtime to the context with default options. -fn add_runtime(printer: SharedExternalPrinterLogger, context: &mut Context) { +fn add_runtime(printer: SharedExternalPrinterLogger, context: &mut Context) -> Result<()> { boa_runtime::register( ( boa_runtime::extensions::ConsoleExtension(printer), @@ -694,7 +702,8 @@ fn add_runtime(printer: SharedExternalPrinterLogger, context: &mut Context) { None, context, ) - .expect("should not fail while registering the runtime"); + .map_err(|e| eyre!("failed to register runtime: {e}"))?; + Ok(()) } #[allow(clippy::struct_field_names)] From b06849c9cc1f9605bd4220f2f410b891b8931941 Mon Sep 17 00:00:00 2001 From: Shivampal157 Date: Tue, 3 Mar 2026 00:30:09 +0530 Subject: [PATCH 2/2] refactor: remove panics in JsRegExp wrapper --- core/engine/src/object/builtins/jsregexp.rs | 138 +++++++++++++------- 1 file changed, 93 insertions(+), 45 deletions(-) diff --git a/core/engine/src/object/builtins/jsregexp.rs b/core/engine/src/object/builtins/jsregexp.rs index c2f11070a91..b9a04c590ad 100644 --- a/core/engine/src/object/builtins/jsregexp.rs +++ b/core/engine/src/object/builtins/jsregexp.rs @@ -60,10 +60,11 @@ impl JsRegExp { where S: Into, { - let regexp = RegExp::initialize(None, &pattern.into(), &flags.into(), context)? - .as_object() - .expect("RegExp::initialize must return a RegExp object") - .clone(); + let value = RegExp::initialize(None, &pattern.into(), &flags.into(), context)?; + let regexp = value.as_object().ok_or_else(|| { + JsNativeError::error() + .with_message("RegExp.initialize did not return an object") + })?.clone(); Ok(Self { inner: regexp }) } @@ -83,50 +84,71 @@ impl JsRegExp { /// Returns a boolean value for whether the `d` flag is present in `JsRegExp` flags #[inline] pub fn has_indices(&self, context: &mut Context) -> JsResult { - RegExp::get_has_indices(&self.inner.clone().into(), &[], context) - .map(|v| v.as_boolean().expect("value must be a bool")) + let value = RegExp::get_has_indices(&self.inner.clone().into(), &[], context)?; + value.as_boolean().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.hasIndices getter must return a boolean") + }).map_err(Into::into) } /// Returns a boolean value for whether the `g` flag is present in `JsRegExp` flags #[inline] pub fn global(&self, context: &mut Context) -> JsResult { - RegExp::get_global(&self.inner.clone().into(), &[], context) - .map(|v| v.as_boolean().expect("value must be a bool")) + let value = RegExp::get_global(&self.inner.clone().into(), &[], context)?; + value.as_boolean().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.global getter must return a boolean") + }).map_err(Into::into) } /// Returns a boolean value for whether the `i` flag is present in `JsRegExp` flags #[inline] pub fn ignore_case(&self, context: &mut Context) -> JsResult { - RegExp::get_ignore_case(&self.inner.clone().into(), &[], context) - .map(|v| v.as_boolean().expect("value must be a bool")) + let value = RegExp::get_ignore_case(&self.inner.clone().into(), &[], context)?; + value.as_boolean().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.ignoreCase getter must return a boolean") + }).map_err(Into::into) } /// Returns a boolean value for whether the `m` flag is present in `JsRegExp` flags #[inline] pub fn multiline(&self, context: &mut Context) -> JsResult { - RegExp::get_multiline(&self.inner.clone().into(), &[], context) - .map(|v| v.as_boolean().expect("value must be a bool")) + let value = RegExp::get_multiline(&self.inner.clone().into(), &[], context)?; + value.as_boolean().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.multiline getter must return a boolean") + }).map_err(Into::into) } /// Returns a boolean value for whether the `s` flag is present in `JsRegExp` flags #[inline] pub fn dot_all(&self, context: &mut Context) -> JsResult { - RegExp::get_dot_all(&self.inner.clone().into(), &[], context) - .map(|v| v.as_boolean().expect("value must be a bool")) + let value = RegExp::get_dot_all(&self.inner.clone().into(), &[], context)?; + value.as_boolean().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.dotAll getter must return a boolean") + }).map_err(Into::into) } /// Returns a boolean value for whether the `u` flag is present in `JsRegExp` flags #[inline] pub fn unicode(&self, context: &mut Context) -> JsResult { - RegExp::get_unicode(&self.inner.clone().into(), &[], context) - .map(|v| v.as_boolean().expect("value must be a bool")) + let value = RegExp::get_unicode(&self.inner.clone().into(), &[], context)?; + value.as_boolean().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.unicode getter must return a boolean") + }).map_err(Into::into) } /// Returns a boolean value for whether the `y` flag is present in `JsRegExp` flags #[inline] pub fn sticky(&self, context: &mut Context) -> JsResult { - RegExp::get_sticky(&self.inner.clone().into(), &[], context) - .map(|v| v.as_boolean().expect("value must be a bool")) + let value = RegExp::get_sticky(&self.inner.clone().into(), &[], context)?; + value.as_boolean().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.sticky getter must return a boolean") + }).map_err(Into::into) } /// Returns the flags of `JsRegExp` as a string @@ -146,11 +168,16 @@ impl JsRegExp { /// ``` #[inline] pub fn flags(&self, context: &mut Context) -> JsResult { - RegExp::get_flags(&self.inner.clone().into(), &[], context).map(|v| { - v.as_string() - .expect("value must be string") - .to_std_string() - .expect("flags must be a valid string") + let value = RegExp::get_flags(&self.inner.clone().into(), &[], context)?; + let s = value.as_string().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.flags getter must return a string") + })?; + + s.to_std_string().map_err(|e| { + JsNativeError::error() + .with_message(format!("invalid string value for RegExp.prototype.flags: {e}")) + .into() }) } @@ -171,11 +198,16 @@ impl JsRegExp { /// ``` #[inline] pub fn source(&self, context: &mut Context) -> JsResult { - RegExp::get_source(&self.inner.clone().into(), &[], context).map(|v| { - v.as_string() - .expect("value must be string") - .to_std_string() - .expect("source must be a valid string") + let value = RegExp::get_source(&self.inner.clone().into(), &[], context)?; + let s = value.as_string().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.source getter must return a string") + })?; + + s.to_std_string().map_err(|e| { + JsNativeError::error() + .with_message(format!("invalid string value for RegExp.prototype.source: {e}")) + .into() }) } @@ -198,8 +230,13 @@ impl JsRegExp { where S: Into, { - RegExp::test(&self.inner.clone().into(), &[search_string.into()], context) - .map(|v| v.as_boolean().expect("value must be a bool")) + let value = + RegExp::test(&self.inner.clone().into(), &[search_string.into()], context)?; + + value.as_boolean().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.test must return a boolean") + }).map_err(Into::into) } /// Executes a search for a match in a specified string @@ -209,16 +246,20 @@ impl JsRegExp { where S: Into, { - RegExp::exec(&self.inner.clone().into(), &[search_string.into()], context).map(|v| { - if v.is_null() { - None - } else { - Some( - JsArray::from_object(v.to_object(context).expect("v must be an array")) - .expect("from_object must not fail if v is an array object"), - ) - } - }) + let value = + RegExp::exec(&self.inner.clone().into(), &[search_string.into()], context)?; + + if value.is_null() { + return Ok(None); + } + + let obj = value.as_object().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.exec must return an array object or null") + })?.clone(); + + let array = JsArray::from_object(obj)?; + Ok(Some(array)) } /// Return a string representing the regular expression. @@ -238,11 +279,18 @@ impl JsRegExp { /// ``` #[inline] pub fn to_string(&self, context: &mut Context) -> JsResult { - RegExp::to_string(&self.inner.clone().into(), &[], context).map(|v| { - v.as_string() - .expect("value must be a string") - .to_std_string() - .expect("to_string value must be a valid string") + let value = RegExp::to_string(&self.inner.clone().into(), &[], context)?; + let s = value.as_string().ok_or_else(|| { + JsNativeError::typ() + .with_message("RegExp.prototype.toString must return a string") + })?; + + s.to_std_string().map_err(|e| { + JsNativeError::error() + .with_message(format!( + "invalid string value produced by RegExp.prototype.toString: {e}" + )) + .into() }) } }