foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00
Code Issues Packages 1 Wiki Activity

refactor(core): Remove ErrWithV8Handle (#14394)

Browse source
This commit is contained in:
Nayeem Rahman 2022-04-26 14:28:42 +01:00 committed by GitHub
parent 3c7c586577
commit 74175c039a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 134 additions and 133 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
core/error.rs
View file

@ -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::*;

42
core/modules.rs
View file

@ -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,
)));
))));
}
}

168
core/runtime.rs
View file

@ -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,