V8: use ES modules instead of global (#3361)

# Description of Changes

Tweaks V8 module support to use JS modules for `spacetimedb_sys` and the
user module rather than using scripts and the global object.

To this end, the code is also made more modular so that it e.g., cares
less avoid global vs. modules.
An `Object` with the functions is e.g., enough for the lowest level
`call_call_reducer`.

Some `.unwrap()`s are also removed.

Also, `run_timeout_and_cb_every` is disabled, as it currently leads to
UB due to bugs in the `v8` crate.

# API and ABI breaking changes

None

# Expected complexity level and risk

2

# Testing

Future work.

Author: Centril

crates/core/src/host/module_common.rs

@@ -92,10 +92,11 @@ impl ModuleCommon {
 
 /// Runs the describer of modules in `run` and does some logging around it.
 pub(crate) fn run_describer<T>(
-    log_traceback: impl FnOnce(&str, &str, &anyhow::Error),
+    log_traceback: impl Copy + FnOnce(&str, &str, &anyhow::Error),
     run: impl FnOnce() -> anyhow::Result<T>,
 ) -> Result<T, DescribeError> {
     let describer_func_name = DESCRIBE_MODULE_DUNDER;
+
     let start = std::time::Instant::now();
     log::trace!("Start describer \"{describer_func_name}\"...");
 

crates/core/src/host/v8/de.rs

@@ -1,8 +1,7 @@
-#![allow(dead_code)]
-
 use super::error::{exception_already_thrown, ExcResult, ExceptionThrown, ExceptionValue, Throwable, TypeError};
 use super::from_value::{cast, FromValue};
-use super::string_const::{TAG, VALUE};
+use super::string::{TAG, VALUE};
+use super::FnRet;
 use core::fmt;
 use core::iter::{repeat_n, RepeatN};
 use core::marker::PhantomData;
@@ -158,16 +157,12 @@ impl<'de, 'this, 'scope: 'de> de::Deserializer<'de> for Deserializer<'this, 'sco
         let object = cast!(scope, self.input, Object, "object for sum type `{}`", sum_name)?;
 
         // Extract the `tag` field. It needs to contain a string.
-        let tag = object
-            .get(scope, tag_field.into())
-            .ok_or_else(exception_already_thrown)?;
+        let tag = property(scope, object, tag_field)?;
         let tag = cast!(scope, tag, v8::String, "string for sum tag of `{}`", sum_name)?;
 
         // Extract the `value` field.
         let value_field = VALUE.string(scope);
-        let value = object
-            .get(scope, value_field.into())
-            .ok_or_else(exception_already_thrown)?;
+        let value = property(scope, object, value_field)?;
 
         // Stitch it all together.
         visitor.visit_sum(SumAccess {
@@ -236,6 +231,15 @@ pub(super) fn intern_field_name<'scope>(
     v8_interned_string(scope, &field).into()
 }
 
+/// Returns the property for `key` on `object`.
+pub(super) fn property<'scope>(
+    scope: &PinScope<'scope, '_>,
+    object: Local<'scope, Object>,
+    key: impl Into<Local<'scope, Value>>,
+) -> FnRet<'scope> {
+    object.get(scope, key.into()).ok_or_else(exception_already_thrown)
+}
+
 impl<'de, 'scope: 'de> de::NamedProductAccess<'de> for ProductAccess<'_, 'scope, '_> {
     type Error = Error<'scope>;
 
@@ -262,10 +266,7 @@ impl<'de, 'scope: 'de> de::NamedProductAccess<'de> for ProductAccess<'_, 'scope,
             }
 
             // Extract the next value, to be processed in `get_field_value_seed`.
-            let val = self
-                .object
-                .get(scope, key.into())
-                .ok_or_else(exception_already_thrown)?;
+            let val = property(scope, self.object, key)?;
             self.next_value = Some(val);
 
             drop(field_names);

crates/core/src/host/v8/error.rs

@@ -1,27 +1,16 @@
 //! Utilities for error handling when dealing with V8.
 
-use crate::database_logger::{BacktraceFrame, BacktraceProvider, ModuleBacktrace};
-
+use super::de::scratch_buf;
 use super::serialize_to_js;
+use super::string::IntoJsString;
+use crate::database_logger::{BacktraceFrame, BacktraceProvider, ModuleBacktrace};
 use core::fmt;
 use spacetimedb_sats::Serialize;
 use v8::{tc_scope, Exception, HandleScope, Local, PinScope, PinnedRef, StackFrame, StackTrace, TryCatch, Value};
 
 /// The result of trying to convert a [`Value`] in scope `'scope` to some type `T`.
 pub(super) type ValueResult<'scope, T> = Result<T, ExceptionValue<'scope>>;
 
-/// Types that can convert into a JS string type.
-pub(super) trait IntoJsString {
-    /// Converts `self` into a JS string.
-    fn into_string<'scope>(self, scope: &PinScope<'scope, '_>) -> Local<'scope, v8::String>;
-}
-
-impl IntoJsString for String {
-    fn into_string<'scope>(self, scope: &PinScope<'scope, '_>) -> Local<'scope, v8::String> {
-        v8::String::new(scope, &self).unwrap()
-    }
-}
-
 /// A JS exception value.
 ///
 /// Newtyped for additional type safety and to track JS exceptions in the type system.
@@ -46,19 +35,74 @@ pub struct TypeError<M>(pub M);
 
 impl<'scope, M: IntoJsString> IntoException<'scope> for TypeError<M> {
     fn into_exception(self, scope: &PinScope<'scope, '_>) -> ExceptionValue<'scope> {
-        let msg = self.0.into_string(scope);
-        ExceptionValue(Exception::type_error(scope, msg))
+        match self.0.into_string(scope) {
+            Ok(msg) => ExceptionValue(Exception::type_error(scope, msg)),
+            Err(err) => err.into_range_error().into_exception(scope),
+        }
     }
 }
 
+/// Returns a "module not found" exception to be thrown.
+pub fn module_exception(scope: &mut PinScope<'_, '_>, spec: Local<'_, v8::String>) -> TypeError<String> {
+    let mut buf = scratch_buf::<32>();
+    let spec = spec.to_rust_cow_lossy(scope, &mut buf);
+    TypeError(format!("Could not find module {spec:?}"))
+}
+
 /// A type converting into a JS `RangeError` exception.
 #[derive(Copy, Clone)]
 pub struct RangeError<M>(pub M);
 
 impl<'scope, M: IntoJsString> IntoException<'scope> for RangeError<M> {
     fn into_exception(self, scope: &PinScope<'scope, '_>) -> ExceptionValue<'scope> {
-        let msg = self.0.into_string(scope);
-        ExceptionValue(Exception::range_error(scope, msg))
+        match self.0.into_string(scope) {
+            Ok(msg) => ExceptionValue(Exception::range_error(scope, msg)),
+            // This is not an infinite recursion.
+            // The `r: RangeError<String>` that `StringTooLongError` produces
+            // will always enter the branch above, as `r.0` is shorter than the maximum allowed.
+            Err(err) => err.into_range_error().into_exception(scope),
+        }
+    }
+}
+
+/// A non-JS string couldn't convert to JS as it was to long.
+#[derive(Debug)]
+pub(super) struct StringTooLongError {
+    /// The length of the string that was too long (`len >` [`v8::String::MAX_LENGTH`]).
+    pub(super) len: usize,
+    /// A prefix of the string for the purpose of rendering an exception that aids the module dev.
+    pub(super) prefix: String,
+}
+
+impl StringTooLongError {
+    /// Returns a new error that keeps a prefix of `string` and records its length.
+    pub(super) fn new(string: &str) -> Self {
+        let len = string.len();
+        let prefix = string[0..16.max(len)].to_owned();
+        Self { len, prefix }
+    }
+
+    /// Converts the error to a [`RangeError<String>`].
+    pub(super) fn into_range_error(self) -> RangeError<String> {
+        let Self { len, prefix } = self;
+        RangeError(format!(
+            r#"The string "`{prefix}..`" of `{len}` bytes is too long for JS"#
+        ))
+    }
+}
+
+/// A non-JS array couldn't convert to JS as it was to long.
+#[derive(Debug)]
+pub(super) struct ArrayTooLongError {
+    /// The length of the array that was too long (`len >` [`i32::MAX`]).
+    pub(super) len: usize,
+}
+
+impl ArrayTooLongError {
+    /// Converts the error to a [`RangeError<String>`].
+    pub(super) fn into_range_error(self) -> RangeError<String> {
+        let Self { len } = self;
+        RangeError(format!("`{len}` elements are too many for a JS array"))
     }
 }
 
@@ -69,7 +113,7 @@ pub(super) struct TerminationError {
 }
 
 impl TerminationError {
-    /// Convert `anyhow::Error` to a termination error.
+    /// Converts [`anyhow::Error`] to a termination error.
     pub(super) fn from_error<'scope>(
         scope: &PinScope<'scope, '_>,
         error: &anyhow::Error,
@@ -124,9 +168,6 @@ pub(crate) struct ExceptionThrown {
 /// A result where the error indicates that an exception has already been thrown in V8.
 pub(crate) type ExcResult<T> = Result<T, ExceptionThrown>;
 
-/// The return type of a module -> host syscall.
-pub(super) type FnRet<'scope> = ExcResult<Local<'scope, Value>>;
-
 /// Indicates that the JS side had thrown an exception.
 pub(super) fn exception_already_thrown() -> ExceptionThrown {
     ExceptionThrown { _priv: () }
@@ -252,6 +293,7 @@ impl ModuleBacktrace for JsStackTrace {
 pub(super) struct JsStackTraceFrame {
     line: usize,
     column: usize,
+    #[allow(dead_code)]
     script_id: usize,
     script_name: Option<String>,
     fn_name: Option<String>,

crates/core/src/host/v8/from_value.rs

@@ -1,8 +1,4 @@
-#![allow(dead_code)]
-
-use crate::host::v8::error::ExceptionValue;
-
-use super::error::{IntoException as _, TypeError, ValueResult};
+use super::error::{ExceptionValue, IntoException as _, TypeError, ValueResult};
 use bytemuck::{AnyBitPattern, NoUninit};
 use spacetimedb_sats::{i256, u256};
 use v8::{BigInt, Boolean, Int32, Local, Number, PinScope, Uint32, Value};