diff options
Diffstat (limited to 'core')
| -rw-r--r-- | core/error.rs | 57 | ||||
| -rw-r--r-- | core/modules.rs | 42 | ||||
| -rw-r--r-- | core/runtime.rs | 168 |
3 files changed, 134 insertions, 133 deletions
diff --git a/core/error.rs b/core/error.rs index 8014dceab..587581e82 100644 --- a/core/error.rs +++ b/core/error.rs @@ -389,12 +389,16 @@ impl Display for JsError { } } -pub(crate) fn attach_handle_to_error( - scope: &mut v8::Isolate, +// TODO(piscisaureus): rusty_v8 should implement the Error trait on +// values of type v8::Global<T>. +pub(crate) fn to_v8_type_error( + scope: &mut v8::HandleScope, err: Error, - handle: v8::Local<v8::Value>, -) -> Error { - ErrWithV8Handle::new(scope, err, handle).into() +) -> v8::Global<v8::Value> { + let message = err.to_string(); + let message = v8::String::new(scope, &message).unwrap(); + let exception = v8::Exception::type_error(scope, message); + v8::Global::new(scope, exception) } /// Implements `value instanceof primordials.Error` in JS. Similar to @@ -431,49 +435,6 @@ pub(crate) fn is_instance_of_error<'s>( false } -// TODO(piscisaureus): rusty_v8 should implement the Error trait on -// values of type v8::Global<T>. -pub(crate) struct ErrWithV8Handle { - err: Error, - handle: v8::Global<v8::Value>, -} - -impl ErrWithV8Handle { - pub fn new( - scope: &mut v8::Isolate, - err: Error, - handle: v8::Local<v8::Value>, - ) -> Self { - let handle = v8::Global::new(scope, handle); - Self { err, handle } - } - - pub fn get_handle<'s>( - &self, - scope: &mut v8::HandleScope<'s>, - ) -> v8::Local<'s, v8::Value> { - v8::Local::new(scope, &self.handle) - } -} - -#[allow(clippy::non_send_fields_in_send_ty)] -unsafe impl Send for ErrWithV8Handle {} -unsafe impl Sync for ErrWithV8Handle {} - -impl std::error::Error for ErrWithV8Handle {} - -impl Display for ErrWithV8Handle { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { - <Error as Display>::fmt(&self.err, f) - } -} - -impl Debug for ErrWithV8Handle { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { - <Self as Display>::fmt(self, f) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/core/modules.rs b/core/modules.rs index 481091e0f..79ee9ebf5 100644 --- a/core/modules.rs +++ b/core/modules.rs @@ -1,12 +1,10 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. use crate::bindings; -use crate::error::attach_handle_to_error; use crate::error::generic_error; use crate::module_specifier::ModuleSpecifier; use crate::resolve_import; use crate::resolve_url; -use crate::runtime::exception_to_err_result; use crate::OpState; use anyhow::Error; use futures::future::FutureExt; @@ -494,12 +492,12 @@ impl RecursiveModuleLoad { scope: &mut v8::HandleScope, module_request: &ModuleRequest, module_source: &ModuleSource, - ) -> Result<(), Error> { + ) -> Result<(), ModuleError> { if module_request.expected_module_type != module_source.module_type { - return Err(generic_error(format!( + return Err(ModuleError::Other(generic_error(format!( "Expected a \"{}\" module but loaded a \"{}\" module.", module_request.expected_module_type, module_source.module_type, - ))); + )))); } // Register the module in the module map unless it's already there. If the @@ -711,6 +709,12 @@ enum SymbolicModule { Mod(ModuleId), } +#[derive(Debug)] +pub(crate) enum ModuleError { + Exception(v8::Global<v8::Value>), + Other(Error), +} + /// A collection of JS modules. pub(crate) struct ModuleMap { // Handling of specifiers and v8 objects @@ -778,7 +782,7 @@ impl ModuleMap { scope: &mut v8::HandleScope, name: &str, source: &str, - ) -> Result<ModuleId, Error> { + ) -> Result<ModuleId, ModuleError> { let name_str = v8::String::new(scope, name).unwrap(); let source_str = v8::String::new(scope, strip_bom(source)).unwrap(); @@ -789,9 +793,8 @@ impl ModuleMap { None => { assert!(tc_scope.has_caught()); let exception = tc_scope.exception().unwrap(); - let err = exception_to_err_result(tc_scope, exception, false) - .map_err(|err| attach_handle_to_error(tc_scope, err, exception)); - return err; + let exception = v8::Global::new(tc_scope, exception); + return Err(ModuleError::Exception(exception)); } }; @@ -820,7 +823,7 @@ impl ModuleMap { main: bool, name: &str, source: &str, - ) -> Result<ModuleId, Error> { + ) -> Result<ModuleId, ModuleError> { let name_str = v8::String::new(scope, name).unwrap(); let source_str = v8::String::new(scope, source).unwrap(); @@ -833,8 +836,9 @@ impl ModuleMap { if tc_scope.has_caught() { assert!(maybe_module.is_none()); - let e = tc_scope.exception().unwrap(); - return exception_to_err_result(tc_scope, e, false); + let exception = tc_scope.exception().unwrap(); + let exception = v8::Global::new(tc_scope, exception); + return Err(ModuleError::Exception(exception)); } let module = maybe_module.unwrap(); @@ -862,12 +866,16 @@ impl ModuleMap { // is thrown here validate_import_assertions(tc_scope, &assertions); if tc_scope.has_caught() { - let e = tc_scope.exception().unwrap(); - return exception_to_err_result(tc_scope, e, false); + let exception = tc_scope.exception().unwrap(); + let exception = v8::Global::new(tc_scope, exception); + return Err(ModuleError::Exception(exception)); } let module_specifier = - self.loader.resolve(&import_specifier, name, false)?; + match self.loader.resolve(&import_specifier, name, false) { + Ok(s) => s, + Err(e) => return Err(ModuleError::Other(e)), + }; let expected_module_type = get_module_type_from_assertions(&assertions); let request = ModuleRequest { specifier: module_specifier, @@ -879,11 +887,11 @@ impl ModuleMap { if main { let maybe_main_module = self.info.values().find(|module| module.main); if let Some(main_module) = maybe_main_module { - return Err(generic_error( + return Err(ModuleError::Other(generic_error( format!("Trying to create \"main\" module ({:?}), when one already exists ({:?})", name, main_module.name, - ))); + )))); } } diff --git a/core/runtime.rs b/core/runtime.rs index 5844dc8a9..adc0bf3ee 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -1,14 +1,14 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. use crate::bindings; -use crate::error::attach_handle_to_error; use crate::error::generic_error; -use crate::error::ErrWithV8Handle; +use crate::error::to_v8_type_error; use crate::error::JsError; use crate::extensions::OpDecl; use crate::extensions::OpEventLoopFn; use crate::inspector::JsRuntimeInspector; use crate::module_specifier::ModuleSpecifier; +use crate::modules::ModuleError; use crate::modules::ModuleId; use crate::modules::ModuleLoadId; use crate::modules::ModuleLoader; @@ -652,9 +652,7 @@ impl JsRuntime { /// /// The same `name` value can be used for multiple executions. /// - /// `Error` can be downcast to a type that exposes additional information - /// about the V8 exception. By default this type is `JsError`, however it may - /// be a different type if `RuntimeOptions::js_error_create_fn` has been set. + /// `Error` can usually be downcast to `JsError`. pub fn execute_script( &mut self, name: &str, @@ -666,9 +664,7 @@ impl JsRuntime { /// Takes a snapshot. The isolate should have been created with will_snapshot /// set to true. /// - /// `Error` can be downcast to a type that exposes additional information - /// about the V8 exception. By default this type is `JsError`, however it may - /// be a different type if `RuntimeOptions::js_error_create_fn` has been set. + /// `Error` can usually be downcast to `JsError`. pub fn snapshot(&mut self) -> v8::StartupData { assert!(self.snapshot_creator.is_some()); @@ -730,9 +726,7 @@ impl JsRuntime { if module.get_status() == v8::ModuleStatus::Errored { let exception = module.get_exception(); - let err = exception_to_err_result(scope, exception, false) - .map_err(|err| attach_handle_to_error(scope, err, exception)); - return err; + return exception_to_err_result(scope, exception, false); } assert!(matches!( @@ -1083,7 +1077,7 @@ impl JsRuntime { pub(crate) fn instantiate_module( &mut self, id: ModuleId, - ) -> Result<(), Error> { + ) -> Result<(), v8::Global<v8::Value>> { let module_map_rc = Self::module_map(self.v8_isolate()); let scope = &mut self.handle_scope(); let tc_scope = &mut v8::TryCatch::new(scope); @@ -1095,10 +1089,7 @@ impl JsRuntime { .expect("ModuleInfo not found"); if module.get_status() == v8::ModuleStatus::Errored { - let exception = module.get_exception(); - let err = exception_to_err_result(tc_scope, exception, false) - .map_err(|err| attach_handle_to_error(tc_scope, err, exception)); - return err; + return Err(v8::Global::new(tc_scope, module.get_exception())); } // IMPORTANT: No borrows to `ModuleMap` can be held at this point because @@ -1109,9 +1100,7 @@ impl JsRuntime { if instantiate_result.is_none() { let exception = tc_scope.exception().unwrap(); - let err = exception_to_err_result(tc_scope, exception, false) - .map_err(|err| attach_handle_to_error(tc_scope, err, exception)); - return err; + return Err(v8::Global::new(tc_scope, exception)); } Ok(()) @@ -1203,9 +1192,7 @@ impl JsRuntime { /// Implementors must manually call `run_event_loop()` to drive module /// evaluation future. /// - /// `Error` can be downcast to a type that exposes additional information - /// about the V8 exception. By default this type is `JsError`, however it may - /// be a different type if `RuntimeOptions::js_error_create_fn` has been set. + /// `Error` can usually be downcast to `JsError`. /// /// This function panics if module has not been instantiated. pub fn mod_evaluate( @@ -1287,7 +1274,11 @@ impl JsRuntime { receiver } - fn dynamic_import_reject(&mut self, id: ModuleLoadId, err: Error) { + fn dynamic_import_reject( + &mut self, + id: ModuleLoadId, + exception: v8::Global<v8::Value>, + ) { let module_map_rc = Self::module_map(self.v8_isolate()); let scope = &mut self.handle_scope(); @@ -1298,19 +1289,11 @@ impl JsRuntime { .expect("Invalid dynamic import id"); let resolver = resolver_handle.open(scope); - let exception = err - .downcast_ref::<ErrWithV8Handle>() - .map(|err| err.get_handle(scope)) - .unwrap_or_else(|| { - let message = err.to_string(); - let message = v8::String::new(scope, &message).unwrap(); - v8::Exception::type_error(scope, message) - }); - // IMPORTANT: No borrows to `ModuleMap` can be held at this point because // rejecting the promise might initiate another `import()` which will // in turn call `bindings::host_import_module_dynamically_callback` which // will reach into `ModuleMap` from within the isolate. + let exception = v8::Local::new(scope, exception); resolver.reject(scope, exception).unwrap(); scope.perform_microtask_checkpoint(); } @@ -1375,7 +1358,8 @@ impl JsRuntime { .push(load.into_future()); } Err(err) => { - self.dynamic_import_reject(dyn_import_id, err); + let exception = to_v8_type_error(&mut self.handle_scope(), err); + self.dynamic_import_reject(dyn_import_id, exception); } } // Continue polling for more prepared dynamic imports. @@ -1425,14 +1409,23 @@ impl JsRuntime { .pending_dynamic_imports .push(load.into_future()); } - Err(err) => self.dynamic_import_reject(dyn_import_id, err), + Err(err) => { + let exception = match err { + ModuleError::Exception(e) => e, + ModuleError::Other(e) => { + to_v8_type_error(&mut self.handle_scope(), e) + } + }; + self.dynamic_import_reject(dyn_import_id, exception) + } } } Err(err) => { // A non-javascript error occurred; this could be due to a an invalid // module specifier, or a problem with the source map, or a failure // to fetch the module source code. - self.dynamic_import_reject(dyn_import_id, err) + let exception = to_v8_type_error(&mut self.handle_scope(), err); + self.dynamic_import_reject(dyn_import_id, exception); } } } else { @@ -1441,8 +1434,8 @@ impl JsRuntime { let module_id = load.root_module_id.expect("Root module should be loaded"); let result = self.instantiate_module(module_id); - if let Err(err) = result { - self.dynamic_import_reject(dyn_import_id, err); + if let Err(exception) = result { + self.dynamic_import_reject(dyn_import_id, exception); } self.dynamic_import_module_evaluate(dyn_import_id, module_id)?; } @@ -1499,11 +1492,10 @@ impl JsRuntime { v8::PromiseState::Rejected => { let exception = promise.result(scope); scope.perform_microtask_checkpoint(); - let err1 = exception_to_err_result::<()>(scope, exception, false) - .map_err(|err| attach_handle_to_error(scope, err, exception)) - .unwrap_err(); // Receiver end might have been already dropped, ignore the result - let _ = module_evaluation.sender.send(Err(err1)); + let _ = module_evaluation + .sender + .send(exception_to_err_result(scope, exception, false)); } } } @@ -1532,10 +1524,8 @@ impl JsRuntime { } v8::PromiseState::Rejected => { let exception = promise.result(scope); - let err1 = exception_to_err_result::<()>(scope, exception, false) - .map_err(|err| attach_handle_to_error(scope, err, exception)) - .unwrap_err(); - Some(Err((pending_dyn_evaluate.load_id, err1))) + let exception = v8::Global::new(scope, exception); + Some(Err((pending_dyn_evaluate.load_id, exception))) } } }; @@ -1545,8 +1535,8 @@ impl JsRuntime { Ok((dyn_import_id, module_id)) => { self.dynamic_import_resolve(dyn_import_id, module_id); } - Err((dyn_import_id, err1)) => { - self.dynamic_import_reject(dyn_import_id, err1); + Err((dyn_import_id, exception)) => { + self.dynamic_import_reject(dyn_import_id, exception); } } } @@ -1568,13 +1558,23 @@ impl JsRuntime { ) -> Result<ModuleId, Error> { let module_map_rc = Self::module_map(self.v8_isolate()); if let Some(code) = code { - module_map_rc.borrow_mut().new_es_module( - &mut self.handle_scope(), - // main module - true, - specifier.as_str(), - &code, - )?; + let scope = &mut self.handle_scope(); + module_map_rc + .borrow_mut() + .new_es_module( + scope, + // main module + true, + specifier.as_str(), + &code, + ) + .map_err(|e| match e { + ModuleError::Exception(exception) => { + let exception = v8::Local::new(scope, exception); + exception_to_err_result::<()>(scope, exception, false).unwrap_err() + } + ModuleError::Other(error) => error, + })?; } let mut load = @@ -1583,11 +1583,23 @@ impl JsRuntime { while let Some(load_result) = load.next().await { let (request, info) = load_result?; let scope = &mut self.handle_scope(); - load.register_and_recurse(scope, &request, &info)?; + load.register_and_recurse(scope, &request, &info).map_err( + |e| match e { + ModuleError::Exception(exception) => { + let exception = v8::Local::new(scope, exception); + exception_to_err_result::<()>(scope, exception, false).unwrap_err() + } + ModuleError::Other(error) => error, + }, + )?; } let root_id = load.root_module_id.expect("Root module should be loaded"); - self.instantiate_module(root_id)?; + self.instantiate_module(root_id).map_err(|e| { + let scope = &mut self.handle_scope(); + let exception = v8::Local::new(scope, e); + exception_to_err_result::<()>(scope, exception, false).unwrap_err() + })?; Ok(root_id) } @@ -1605,13 +1617,23 @@ impl JsRuntime { ) -> Result<ModuleId, Error> { let module_map_rc = Self::module_map(self.v8_isolate()); if let Some(code) = code { - module_map_rc.borrow_mut().new_es_module( - &mut self.handle_scope(), - // not main module - false, - specifier.as_str(), - &code, - )?; + let scope = &mut self.handle_scope(); + module_map_rc + .borrow_mut() + .new_es_module( + scope, + // not main module + false, + specifier.as_str(), + &code, + ) + .map_err(|e| match e { + ModuleError::Exception(exception) => { + let exception = v8::Local::new(scope, exception); + exception_to_err_result::<()>(scope, exception, false).unwrap_err() + } + ModuleError::Other(error) => error, + })?; } let mut load = @@ -1620,11 +1642,23 @@ impl JsRuntime { while let Some(load_result) = load.next().await { let (request, info) = load_result?; let scope = &mut self.handle_scope(); - load.register_and_recurse(scope, &request, &info)?; + load.register_and_recurse(scope, &request, &info).map_err( + |e| match e { + ModuleError::Exception(exception) => { + let exception = v8::Local::new(scope, exception); + exception_to_err_result::<()>(scope, exception, false).unwrap_err() + } + ModuleError::Other(error) => error, + }, + )?; } let root_id = load.root_module_id.expect("Root module should be loaded"); - self.instantiate_module(root_id)?; + self.instantiate_module(root_id).map_err(|e| { + let scope = &mut self.handle_scope(); + let exception = v8::Local::new(scope, e); + exception_to_err_result::<()>(scope, exception, false).unwrap_err() + })?; Ok(root_id) } @@ -1827,9 +1861,7 @@ impl JsRealm { /// /// The same `name` value can be used for multiple executions. /// - /// `Error` can be downcast to a type that exposes additional information - /// about the V8 exception. By default this type is `JsError`, however it may - /// be a different type if `RuntimeOptions::js_error_create_fn` has been set. + /// `Error` can usually be downcast to `JsError`. pub fn execute_script( &self, runtime: &mut JsRuntime, |