fix(builtins/intl, builtins/promise): convert panics to EngineError::Panic using js_expect (#5014)

Part of #3241.

This removes several internal panic paths by replacing unwrap/downcast
usage with proper error propagation using `js_expect()` and `JsResult`.

It changes the following:

* `core/engine/src/builtins/intl/segmenter/iterator.rs`: converted 1
panic to `js_expect().ok()?`
* `core/engine/src/builtins/intl/segmenter/segments.rs`: converted 2
panics to `js_expect()?`
* `core/engine/src/builtins/intl/collator/mod.rs`: converted 8 panics to
`js_expect()?`
* `core/engine/src/builtins/intl/list_format/mod.rs`: converted 7 panics
to `js_expect()?`
* `core/engine/src/builtins/intl/number_format/options.rs`: converted 2
panics to `js_expect()?`
* `core/engine/src/builtins/intl/date_time_format/mod.rs`: converted 2
panics by propagating `JsResult`
* `core/engine/src/builtins/promise/mod.rs`: converted 4 panics to
`js_expect()?`

Co-authored-by: José Julián Espina <jedel0124@gmail.com>

Author: KaustubhOG

core/engine/src/builtins/intl/collator/mod.rs

@@ -9,7 +9,7 @@ use icu_collator::{
 use icu_locale::{Locale, extensions::unicode};
 
 use crate::{
-    Context, JsArgs, JsData, JsNativeError, JsResult, JsString, JsValue,
+    Context, JsArgs, JsData, JsExpect, JsNativeError, JsResult, JsString, JsValue,
     builtins::{
         BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject, OrdinaryObject,
         options::get_option,
@@ -362,7 +362,7 @@ impl Collator {
                         // 2. Assert: Type(collator) is Object and collator has an [[InitializedCollator]] internal slot.
                         let collator = collator
                             .downcast_ref::<Self>()
-                            .expect("checked above that the object was a collator object");
+                            .js_expect("checked above that the object was a collator object")?;
 
                         // 3. If x is not provided, let x be undefined.
                         // 5. Let X be ? ToString(x).
@@ -445,7 +445,7 @@ impl Collator {
                 js_string!(collator.locale.to_string()),
                 context,
             )
-            .expect("operation must not fail per the spec");
+            .js_expect("operation must not fail per the spec")?;
         options
             .create_data_property_or_throw(
                 js_string!("usage"),
@@ -455,7 +455,7 @@ impl Collator {
                 },
                 context,
             )
-            .expect("operation must not fail per the spec");
+            .js_expect("operation must not fail per the spec")?;
         options
             .create_data_property_or_throw(
                 js_string!("sensitivity"),
@@ -467,14 +467,14 @@ impl Collator {
                 },
                 context,
             )
-            .expect("operation must not fail per the spec");
+            .js_expect("operation must not fail per the spec")?;
         options
             .create_data_property_or_throw(
                 js_string!("ignorePunctuation"),
                 collator.ignore_punctuation,
                 context,
             )
-            .expect("operation must not fail per the spec");
+            .js_expect("operation must not fail per the spec")?;
         options
             .create_data_property_or_throw(
                 js_string!("collation"),
@@ -484,18 +484,18 @@ impl Collator {
                     .unwrap_or(js_string!("default")),
                 context,
             )
-            .expect("operation must not fail per the spec");
+            .js_expect("operation must not fail per the spec")?;
         options
             .create_data_property_or_throw(js_string!("numeric"), collator.numeric, context)
-            .expect("operation must not fail per the spec");
+            .js_expect("operation must not fail per the spec")?;
         if let Some(kf) = collator.case_first {
             options
                 .create_data_property_or_throw(
                     js_string!("caseFirst"),
                     js_string!(kf.as_str()),
                     context,
                 )
-                .expect("operation must not fail per the spec");
+                .js_expect("operation must not fail per the spec")?;
         }
 
         // 5. Return options.

core/engine/src/builtins/intl/date_time_format/mod.rs

@@ -8,7 +8,7 @@
 //! [spec]: https://tc39.es/ecma402/#datetimeformat-objects
 
 use crate::{
-    Context, JsArgs, JsData, JsResult, JsString, JsValue, NativeFunction,
+    Context, JsArgs, JsData, JsExpect, JsResult, JsString, JsValue, NativeFunction,
     builtins::{
         BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject,
         date::utils::{
@@ -272,7 +272,7 @@ impl DateTimeFormat {
 
                         let formatter = dtf.borrow().data().formatter.clone();
 
-                        let dt = fields.to_formattable_datetime();
+                        let dt = fields.to_formattable_datetime()?;
                         let tz_info = dtf.borrow().data().time_zone.to_time_zone_info();
                         let tz_info_at_time = tz_info.at_date_time_iso(dt);
 
@@ -482,13 +482,15 @@ impl ToLocalTime {
         })
     }
 
-    pub(crate) fn to_formattable_datetime(&self) -> DateTime<Iso> {
-        DateTime {
+    pub(crate) fn to_formattable_datetime(&self) -> JsResult<DateTime<Iso>> {
+        Ok(DateTime {
             date: Date::try_new_iso(self.year, self.month, self.day)
-                .expect("TimeClip insures valid range."),
+                .ok()
+                .js_expect("TimeClip insures valid range.")?,
             time: Time::try_new(self.hour, self.minute, self.second, self.subsecond)
-                .expect("valid values"),
-        }
+                .ok()
+                .js_expect("valid values")?,
+        })
     }
 }
 

core/engine/src/builtins/intl/list_format/mod.rs

@@ -9,7 +9,7 @@ use icu_list::{
 use icu_locale::Locale;
 
 use crate::{
-    Context, JsArgs, JsData, JsNativeError, JsResult, JsString, JsValue,
+    Context, JsArgs, JsData, JsExpect, JsNativeError, JsResult, JsString, JsValue,
     builtins::{
         Array, BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject, OrdinaryObject,
         intl::options::EmptyPreferences,
@@ -374,7 +374,7 @@ impl ListFormat {
 
         // 2. Let result be ! ArrayCreate(0).
         let result = Array::array_create(0, None, context)
-            .expect("creating an empty array with default proto must not fail");
+            .js_expect("creating an empty array with default proto must not fail")?;
 
         // 3. Let n be 0.
         // 4. For each Record { [[Type]], [[Value]] } part in parts, do
@@ -388,16 +388,16 @@ impl ListFormat {
 
             // b. Perform ! CreateDataPropertyOrThrow(O, "type", part.[[Type]]).
             o.create_data_property_or_throw(js_string!("type"), js_string!(part.typ()), context)
-                .expect("operation must not fail per the spec");
+                .js_expect("operation must not fail per the spec")?;
 
             // c. Perform ! CreateDataPropertyOrThrow(O, "value", part.[[Value]]).
             o.create_data_property_or_throw(js_string!("value"), js_string!(part.value()), context)
-                .expect("operation must not fail per the spec");
+                .js_expect("operation must not fail per the spec")?;
 
             // d. Perform ! CreateDataPropertyOrThrow(result, ! ToString(n), O).
             result
                 .create_data_property_or_throw(n, o, context)
-                .expect("operation must not fail per the spec");
+                .js_expect("operation must not fail per the spec")?;
 
             // e. Increment n by 1.
         }
@@ -446,7 +446,7 @@ impl ListFormat {
                 js_string!(lf.locale.to_string()),
                 context,
             )
-            .expect("operation must not fail per the spec");
+            .js_expect("operation must not fail per the spec")?;
         options
             .create_data_property_or_throw(
                 js_string!("type"),
@@ -457,7 +457,7 @@ impl ListFormat {
                 },
                 context,
             )
-            .expect("operation must not fail per the spec");
+            .js_expect("operation must not fail per the spec")?;
         options
             .create_data_property_or_throw(
                 js_string!("style"),
@@ -469,7 +469,7 @@ impl ListFormat {
                 },
                 context,
             )
-            .expect("operation must not fail per the spec");
+            .js_expect("operation must not fail per the spec")?;
 
         // 5. Return options.
         Ok(options.into())

core/engine/src/builtins/intl/number_format/options.rs

@@ -19,7 +19,7 @@ use icu_provider::{
 use tinystr::TinyAsciiStr;
 
 use crate::{
-    Context, JsNativeError, JsObject, JsResult, JsStr, JsString, JsValue,
+    Context, JsExpect, JsNativeError, JsObject, JsResult, JsStr, JsString, JsValue,
     builtins::{
         intl::{
             ServicePreferences,
@@ -500,7 +500,7 @@ impl UnitFormatOptions {
             Style::Currency => {
                 UnitFormatOptions::Currency {
                     // a. Set intlObj.[[Currency]] to the ASCII-uppercase of currency.
-                    currency: currency.expect("asserted above that `currency` is not None"),
+                    currency: currency.js_expect("asserted above that `currency` is not None")?,
                     // b. Set intlObj.[[CurrencyDisplay]] to currencyDisplay.
                     display: currency_display,
                     // c. Set intlObj.[[CurrencySign]] to currencySign.
@@ -511,7 +511,7 @@ impl UnitFormatOptions {
             Style::Unit => {
                 UnitFormatOptions::Unit {
                     //     a. Set intlObj.[[Unit]] to unit.
-                    unit: unit.expect("asserted above that `unit` is not None"),
+                    unit: unit.js_expect("asserted above that `unit` is not None")?,
                     // b. Set intlObj.[[UnitDisplay]] to unitDisplay.
                     display: unit_display,
                 }

core/engine/src/builtins/intl/segmenter/iterator.rs

@@ -5,7 +5,7 @@ use icu_segmenter::{
 };
 
 use crate::{
-    Context, JsData, JsNativeError, JsObject, JsResult, JsString, JsSymbol, JsValue,
+    Context, JsData, JsExpect, JsNativeError, JsObject, JsResult, JsString, JsSymbol, JsValue,
     builtins::{BuiltInBuilder, IntrinsicObject, iterable::create_iter_result_object},
     context::intrinsics::Intrinsics,
     js_string,
@@ -126,7 +126,8 @@ impl SegmentIterator {
             let segmenter = iter
                 .segmenter
                 .downcast_ref::<Segmenter>()
-                .expect("segment iterator object should contain a segmenter");
+                .js_expect("segment iterator object should contain a segmenter")
+                .ok()?;
             let mut segments = segmenter.native.segment(string.variant());
             // the first elem is always 0.
             segments.next();

core/engine/src/builtins/intl/segmenter/segments.rs

@@ -2,7 +2,8 @@ use boa_gc::{Finalize, Trace};
 use itertools::Itertools;
 
 use crate::{
-    Context, JsArgs, JsData, JsNativeError, JsObject, JsResult, JsString, JsSymbol, JsValue,
+    Context, JsArgs, JsData, JsExpect, JsNativeError, JsObject, JsResult, JsString, JsSymbol,
+    JsValue,
     builtins::{BuiltInBuilder, IntrinsicObject},
     context::intrinsics::Intrinsics,
     js_string,
@@ -67,7 +68,7 @@ impl Segments {
         let segmenter = segments
             .segmenter
             .downcast_ref::<Segmenter>()
-            .expect("segments object should contain a segmenter");
+            .js_expect("segments object should contain a segmenter")?;
 
         // 4. Let string be segments.[[SegmentsString]].
         // 5. Let len be the length of string.
@@ -93,7 +94,7 @@ impl Segments {
                 .tuple_windows()
                 .find(|((i, _), (j, _))| (*i..*j).contains(&n))
                 .map(|((i, _), (j, word))| ((i..j), word))
-                .expect("string should have at least a length of 1, and `n` must be in range")
+                .js_expect("string should have at least a length of 1, and `n` must be in range")?
         };
 
         // 10. Return ! CreateSegmentDataObject(segmenter, string, startIndex, endIndex).

core/engine/src/builtins/promise/mod.rs

@@ -8,7 +8,7 @@ use super::{
     iterable::{IteratorHint, IteratorRecord},
 };
 use crate::{
-    Context, JsArgs, JsError, JsResult, JsString,
+    Context, JsArgs, JsError, JsExpect, JsResult, JsString,
     builtins::{Array, BuiltInObject},
     context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
     error::JsNativeError,
@@ -915,15 +915,15 @@ impl Promise {
                             js_string!("fulfilled"),
                             context,
                         )
-                        .expect("cannot fail per spec");
+                        .js_expect("cannot fail per spec")?;
 
                         // 11. Perform ! CreateDataPropertyOrThrow(obj, "value", x).
                         obj.create_data_property_or_throw(
                             js_string!("value"),
                             args.get_or_undefined(0).clone(),
                             context,
                         )
-                        .expect("cannot fail per spec");
+                        .js_expect("cannot fail per spec")?;
 
                         // 12. Set values[index] to obj.
                         captures.values.borrow_mut()[captures.index] = obj.into();
@@ -1005,15 +1005,15 @@ impl Promise {
                             js_string!("rejected"),
                             context,
                         )
-                        .expect("cannot fail per spec");
+                        .js_expect("cannot fail per spec")?;
 
                         // 11. Perform ! CreateDataPropertyOrThrow(obj, "reason", x).
                         obj.create_data_property_or_throw(
                             js_string!("reason"),
                             args.get_or_undefined(0).clone(),
                             context,
                         )
-                        .expect("cannot fail per spec");
+                        .js_expect("cannot fail per spec")?;
 
                         // 12. Set values[index] to obj.
                         captures.values.borrow_mut()[captures.index] = obj.into();