From 7a73a2ff172c0be26d1ceb4aff9a3f3f0eab8aff Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 11 Jul 2019 00:53:48 +0200 Subject: [PATCH] Refactor error to use dynamic dispatch and traits This is in preperation for dynamic import (#1789), which is more easily implemented when errors are dynamic. --- cli/compiler.rs | 25 +- cli/deno_dir.rs | 224 +++++++--------- cli/deno_error.rs | 518 +++++++++++++----------------------- cli/diagnostics.rs | 7 + cli/dispatch_minimal.rs | 7 +- cli/fmt_errors.rs | 84 ++++-- cli/fs.rs | 13 +- cli/http_util.rs | 35 +-- cli/import_map.rs | 10 + cli/main.rs | 21 +- cli/msg.fbs | 4 +- cli/msg_util.rs | 9 +- cli/ops.rs | 144 +++++----- cli/permissions.rs | 43 +-- cli/repl.rs | 17 +- cli/resolve_addr.rs | 34 +-- cli/resources.rs | 51 ++-- cli/shell.rs | 32 +-- cli/signal.rs | 16 +- cli/source_maps.rs | 54 ++-- cli/state.rs | 26 +- cli/worker.rs | 88 +++--- core/any_error.rs | 68 +++++ core/examples/http_bench.rs | 2 +- core/isolate.rs | 148 ++++++----- core/js_errors.rs | 173 ++++++------ core/lib.rs | 2 + core/modules.rs | 109 ++++---- 28 files changed, 904 insertions(+), 1060 deletions(-) create mode 100644 core/any_error.rs diff --git a/cli/compiler.rs b/cli/compiler.rs index c827c3ca1d8b57..2a6bd1adefc4e6 100644 --- a/cli/compiler.rs +++ b/cli/compiler.rs @@ -1,6 +1,4 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. -use crate::deno_error::err_check; -use crate::deno_error::DenoError; use crate::diagnostics::Diagnostic; use crate::msg; use crate::resources; @@ -9,6 +7,7 @@ use crate::state::*; use crate::tokio_util; use crate::worker::Worker; use deno::Buf; +use deno::ErrBox; use deno::ModuleSpecifier; use futures::Future; use futures::Stream; @@ -96,7 +95,7 @@ pub fn bundle_async( state: ThreadSafeState, module_name: String, out_file: String, -) -> impl Future { +) -> impl Future { debug!( "Invoking the compiler to bundle. module_name: {}", module_name @@ -116,9 +115,9 @@ pub fn bundle_async( // as was done previously. state.clone(), ); - err_check(worker.execute("denoMain()")); - err_check(worker.execute("workerMain()")); - err_check(worker.execute("compilerMain()")); + worker.execute("denoMain()").unwrap(); + worker.execute("workerMain()").unwrap(); + worker.execute("compilerMain()").unwrap(); let resource = worker.state.resource.clone(); let compiler_rid = resource.rid; @@ -144,7 +143,7 @@ pub fn bundle_async( let json_str = std::str::from_utf8(&msg).unwrap(); debug!("Message: {}", json_str); if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) { - return Err(DenoError::from(diagnostics)); + return Err(ErrBox::from(diagnostics)); } } @@ -156,7 +155,7 @@ pub fn bundle_async( pub fn compile_async( state: ThreadSafeState, module_meta_data: &ModuleMetaData, -) -> impl Future { +) -> impl Future { let module_name = module_meta_data.module_name.clone(); debug!( @@ -178,9 +177,9 @@ pub fn compile_async( // as was done previously. state.clone(), ); - err_check(worker.execute("denoMain()")); - err_check(worker.execute("workerMain()")); - err_check(worker.execute("compilerMain()")); + worker.execute("denoMain()").unwrap(); + worker.execute("workerMain()").unwrap(); + worker.execute("compilerMain()").unwrap(); let compiling_job = state.progress.add("Compile", &module_name); @@ -211,7 +210,7 @@ pub fn compile_async( let json_str = std::str::from_utf8(&msg).unwrap(); debug!("Message: {}", json_str); if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) { - return Err(DenoError::from(diagnostics)); + return Err(ErrBox::from(diagnostics)); } } @@ -242,7 +241,7 @@ pub fn compile_async( pub fn compile_sync( state: ThreadSafeState, module_meta_data: &ModuleMetaData, -) -> Result { +) -> Result { tokio_util::block_on(compile_async(state, module_meta_data)) } diff --git a/cli/deno_dir.rs b/cli/deno_dir.rs index 40962d768983bb..6515fc684714cf 100644 --- a/cli/deno_dir.rs +++ b/cli/deno_dir.rs @@ -1,9 +1,8 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. use crate::compiler::ModuleMetaData; -use crate::deno_error; use crate::deno_error::DenoError; -use crate::deno_error::DenoResult; use crate::deno_error::ErrorKind; +use crate::deno_error::GetErrorKind; use crate::fs as deno_fs; use crate::http_util; use crate::msg; @@ -11,6 +10,7 @@ use crate::progress::Progress; use crate::source_maps::SourceMapGetter; use crate::tokio_util; use crate::version; +use deno::ErrBox; use deno::ModuleSpecifier; use dirs; use futures::future::{loop_fn, Either, Loop}; @@ -20,8 +20,6 @@ use ring; use serde_json; use std; use std::collections::HashSet; -use std::fmt; -use std::fmt::Display; use std::fmt::Write; use std::fs; use std::path::Path; @@ -34,36 +32,6 @@ use std::sync::Mutex; use url; use url::Url; -// TODO: DenoDirError is temporary solution, should be upgraded during rewrite -#[derive(Debug, PartialEq)] -pub enum DenoDirErrorKind { - UnsupportedFetchScheme, -} - -#[derive(Debug)] -pub struct DenoDirError { - pub message: String, - pub kind: DenoDirErrorKind, -} - -impl DenoDirError { - pub fn new(message: String, kind: DenoDirErrorKind) -> Self { - DenoDirError { message, kind } - } -} - -impl std::error::Error for DenoDirError { - fn description(&self) -> &str { - &*self.message - } -} - -impl Display for DenoDirError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - writeln!(f, "{}", self.message) - } -} - const SUPPORTED_URL_SCHEMES: [&str; 3] = ["http", "https", "file"]; fn normalize_path(path: &Path) -> PathBuf { @@ -198,13 +166,13 @@ impl DenoDir { specifier: &ModuleSpecifier, use_cache: bool, no_fetch: bool, - ) -> impl Future { + ) -> impl Future { let module_url = specifier.as_url().to_owned(); debug!("fetch_module_meta_data. specifier {} ", &module_url); let result = self.url_to_deps_path(&module_url); if let Err(err) = result { - return Either::A(futures::future::err(DenoError::from(err))); + return Either::A(futures::future::err(err)); } let deps_filepath = result.unwrap(); @@ -223,20 +191,17 @@ impl DenoDir { use_cache, no_fetch, ).then(move |result| { - let mut out = match result { - Ok(out) => out, - Err(err) => { - if err.kind() == ErrorKind::NotFound { - // For NotFound, change the message to something better. - return Err(deno_error::new( - ErrorKind::NotFound, - format!("Cannot resolve module \"{}\"", module_url.to_string()), - )); - } else { - return Err(err); - } + let mut out = result.map_err(|err| { + if err.kind() == ErrorKind::NotFound { + // For NotFound, change the message to something better. + DenoError::new( + ErrorKind::NotFound, + format!("Cannot resolve module \"{}\"", module_url.to_string()), + ).into() + } else { + err } - }; + })?; if out.source_code.starts_with(b"#!") { out.source_code = filter_shebang(out.source_code); @@ -290,7 +255,7 @@ impl DenoDir { specifier: &ModuleSpecifier, use_cache: bool, no_fetch: bool, - ) -> Result { + ) -> Result { tokio_util::block_on( self.fetch_module_meta_data_async(specifier, use_cache, no_fetch), ) @@ -303,20 +268,17 @@ impl DenoDir { /// /// For specifier starting with `http://` and `https://` it returns /// path to DenoDir dependency directory. - pub fn url_to_deps_path( - self: &Self, - url: &Url, - ) -> Result { + pub fn url_to_deps_path(self: &Self, url: &Url) -> Result { let filename = match url.scheme() { "file" => url.to_file_path().unwrap(), "https" => get_cache_filename(self.deps_https.as_path(), &url), "http" => get_cache_filename(self.deps_http.as_path(), &url), scheme => { return Err( - DenoDirError::new( + DenoError::new( + ErrorKind::UnsupportedFetchScheme, format!("Unsupported scheme \"{}\" for module \"{}\". Supported schemes: {:#?}", scheme, url, SUPPORTED_URL_SCHEMES), - DenoDirErrorKind::UnsupportedFetchScheme - ) + ).into() ); } }; @@ -389,7 +351,7 @@ fn get_source_code_async( filepath: PathBuf, use_cache: bool, no_fetch: bool, -) -> impl Future { +) -> impl Future { let filename = filepath.to_str().unwrap().to_string(); let module_name = module_url.to_string(); let url_scheme = module_url.scheme(); @@ -425,23 +387,23 @@ fn get_source_code_async( // If not remote file stop here! if !is_module_remote { debug!("not remote file stop here"); - return Either::A(futures::future::err(DenoError::from( + return Either::A(futures::future::err( std::io::Error::new( std::io::ErrorKind::NotFound, format!("cannot find local file '{}'", filename), - ), - ))); + ).into(), + )); } // If remote downloads are not allowed stop here! if no_fetch { debug!("remote file with no_fetch stop here"); - return Either::A(futures::future::err(DenoError::from( + return Either::A(futures::future::err( std::io::Error::new( std::io::ErrorKind::NotFound, format!("cannot find remote file '{}' in cache", filename), - ), - ))); + ).into(), + )); } debug!("is remote but didn't find module"); @@ -456,10 +418,12 @@ fn get_source_code_async( download_cache.mark(&module_name); Ok(output) } - None => Err(DenoError::from(std::io::Error::new( - std::io::ErrorKind::NotFound, - format!("cannot find remote file '{}'", filename), - ))), + None => Err( + std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("cannot find remote file '{}'", filename), + ).into(), + ), }, ), ) @@ -474,7 +438,7 @@ fn get_source_code( filepath: PathBuf, use_cache: bool, no_fetch: bool, -) -> DenoResult { +) -> Result { tokio_util::block_on(get_source_code_async( deno_dir, module_url, filepath, use_cache, no_fetch, )) @@ -595,7 +559,7 @@ fn save_module_code_and_headers( source: &str, maybe_content_type: Option, maybe_initial_filepath: Option, -) -> DenoResult<()> { +) -> Result<(), ErrBox> { match filepath.parent() { Some(ref parent) => fs::create_dir_all(parent), None => Ok(()), @@ -636,7 +600,7 @@ fn fetch_remote_source_async( deno_dir: &DenoDir, module_url: &Url, filepath: &Path, -) -> impl Future, Error = DenoError> { +) -> impl Future, Error = ErrBox> { use crate::http_util::FetchOnceResult; let download_job = deno_dir.progress.add("Download", &module_url.to_string()); @@ -664,61 +628,65 @@ fn fetch_remote_source_async( )| { let module_uri = url_into_uri(&module_url); // Single pass fetch, either yields code or yields redirect. - http_util::fetch_string_once(module_uri).and_then(move |fetch_once_result| { - match fetch_once_result { - FetchOnceResult::Redirect(uri) => { - // If redirects, update module_name and filename for next looped call. - let new_module_url = Url::parse(&uri.to_string()).expect("http::uri::Uri should be parseable as Url"); - let new_filepath = dir.url_to_deps_path(&new_module_url)?; - - if maybe_initial_module_name.is_none() { - maybe_initial_module_name = Some(module_url.to_string()); - maybe_initial_filepath = Some(filepath.clone()); + http_util::fetch_string_once(module_uri).and_then( + move |fetch_once_result| { + match fetch_once_result { + FetchOnceResult::Redirect(uri) => { + // If redirects, update module_name and filename for next looped call. + let new_module_url = Url::parse(&uri.to_string()) + .expect("http::uri::Uri should be parseable as Url"); + let new_filepath = dir.url_to_deps_path(&new_module_url)?; + + if maybe_initial_module_name.is_none() { + maybe_initial_module_name = Some(module_url.to_string()); + maybe_initial_filepath = Some(filepath.clone()); + } + + // Not yet completed. Follow the redirect and loop. + Ok(Loop::Continue(( + dir, + maybe_initial_module_name, + maybe_initial_filepath, + new_module_url, + new_filepath, + ))) + } + FetchOnceResult::Code(source, maybe_content_type) => { + // We land on the code. + save_module_code_and_headers( + filepath.clone(), + &module_url, + &source, + maybe_content_type.clone(), + maybe_initial_filepath, + )?; + + let media_type = map_content_type( + &filepath, + maybe_content_type.as_ref().map(String::as_str), + ); + + // TODO: module_name should be renamed to URL + let module_meta_data = ModuleMetaData { + module_name: module_url.to_string(), + module_redirect_source_name: maybe_initial_module_name, + filename: filepath.clone(), + media_type, + source_code: source.as_bytes().to_owned(), + maybe_output_code_filename: None, + maybe_output_code: None, + maybe_source_map_filename: None, + maybe_source_map: None, + }; + + Ok(Loop::Break(Some(module_meta_data))) } - - // Not yet completed. Follow the redirect and loop. - Ok(Loop::Continue(( - dir, - maybe_initial_module_name, - maybe_initial_filepath, - new_module_url, - new_filepath, - ))) - } - FetchOnceResult::Code(source, maybe_content_type) => { - // We land on the code. - save_module_code_and_headers( - filepath.clone(), - &module_url, - &source, - maybe_content_type.clone(), - maybe_initial_filepath, - )?; - - let media_type = map_content_type( - &filepath, - maybe_content_type.as_ref().map(String::as_str), - ); - - // TODO: module_name should be renamed to URL - let module_meta_data = ModuleMetaData { - module_name: module_url.to_string(), - module_redirect_source_name: maybe_initial_module_name, - filename: filepath.clone(), - media_type, - source_code: source.as_bytes().to_owned(), - maybe_output_code_filename: None, - maybe_output_code: None, - maybe_source_map_filename: None, - maybe_source_map: None, - }; - - Ok(Loop::Break(Some(module_meta_data))) } - } - }) + }, + ) }, - ).then(move |r| { + ) + .then(move |r| { // Explicit drop to keep reference alive until future completes. drop(download_job); r @@ -731,7 +699,7 @@ fn fetch_remote_source( deno_dir: &DenoDir, module_url: &Url, filepath: &Path, -) -> DenoResult> { +) -> Result, ErrBox> { tokio_util::block_on(fetch_remote_source_async( deno_dir, module_url, filepath, )) @@ -751,7 +719,7 @@ fn fetch_local_source( module_url: &Url, filepath: &Path, module_initial_source_name: Option, -) -> DenoResult> { +) -> Result, ErrBox> { let source_code_headers = get_source_code_headers(&filepath); // If source code headers says that it would redirect elsewhere, // (meaning that the source file might not exist; only .headers.json is present) @@ -907,7 +875,7 @@ fn save_source_code_headers( // TODO(bartlomieju): this method should be moved, it doesn't belong to deno_dir.rs // it's a general utility -pub fn resolve_from_cwd(path: &str) -> Result<(PathBuf, String), DenoError> { +pub fn resolve_from_cwd(path: &str) -> Result<(PathBuf, String), ErrBox> { let candidate_path = Path::new(path); let resolved_path = if candidate_path.is_absolute() { @@ -1669,8 +1637,8 @@ mod tests { for &test in test_cases.iter() { let url = Url::parse(test).unwrap(); assert_eq!( - deno_dir.url_to_deps_path(&url).unwrap_err().kind, - DenoDirErrorKind::UnsupportedFetchScheme + deno_dir.url_to_deps_path(&url).unwrap_err().kind(), + ErrorKind::UnsupportedFetchScheme ); } } diff --git a/cli/deno_error.rs b/cli/deno_error.rs index 0410c35d2eb6c1..717924701a4c01 100644 --- a/cli/deno_error.rs +++ b/cli/deno_error.rs @@ -1,346 +1,239 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. -use crate::deno_dir; -use crate::diagnostics; -use crate::fmt_errors::JSErrorColor; -use crate::import_map; +use crate::diagnostics::Diagnostic; +use crate::fmt_errors::JSError; +use crate::import_map::ImportMapError; pub use crate::msg::ErrorKind; -use crate::resolve_addr::ResolveAddrError; -use crate::source_maps::apply_source_map; -use crate::source_maps::SourceMapGetter; -use deno::JSError; +use deno::AnyError; +use deno::ErrBox; use deno::ModuleResolutionError; +use http::uri; use hyper; -#[cfg(unix)] -use nix::{errno::Errno, Error as UnixError}; use std; +use std::error::Error; use std::fmt; use std::io; -use std::str; use url; -pub type DenoResult = std::result::Result; - #[derive(Debug)] pub struct DenoError { - repr: Repr, + kind: ErrorKind, + msg: String, } -#[derive(Debug)] -enum Repr { - Simple(ErrorKind, String), - IoErr(io::Error), - UrlErr(url::ParseError), - HyperErr(hyper::Error), - ImportMapErr(import_map::ImportMapError), - ModuleResolutionErr(ModuleResolutionError), - Diagnostic(diagnostics::Diagnostic), - JSError(JSError), - DenoDirErr(deno_dir::DenoDirError), +impl DenoError { + pub fn new(kind: ErrorKind, msg: String) -> Self { + Self { kind, msg } + } } -/// Create a new simple DenoError. -pub fn new(kind: ErrorKind, msg: String) -> DenoError { - DenoError { - repr: Repr::Simple(kind, msg), +impl Error for DenoError {} + +impl fmt::Display for DenoError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.pad(self.msg.as_str()) } } -impl DenoError { - fn url_error_kind(err: url::ParseError) -> ErrorKind { - use url::ParseError::*; - match err { - EmptyHost => ErrorKind::EmptyHost, - IdnaError => ErrorKind::IdnaError, - InvalidPort => ErrorKind::InvalidPort, - InvalidIpv4Address => ErrorKind::InvalidIpv4Address, - InvalidIpv6Address => ErrorKind::InvalidIpv6Address, - InvalidDomainCharacter => ErrorKind::InvalidDomainCharacter, - RelativeUrlWithoutBase => ErrorKind::RelativeUrlWithoutBase, - RelativeUrlWithCannotBeABaseBase => { - ErrorKind::RelativeUrlWithCannotBeABaseBase - } - SetHostOnCannotBeABaseUrl => ErrorKind::SetHostOnCannotBeABaseUrl, - Overflow => ErrorKind::Overflow, - } - } +#[derive(Debug)] +struct StaticError(ErrorKind, &'static str); - pub fn kind(&self) -> ErrorKind { - match self.repr { - Repr::Simple(kind, ref _msg) => kind, - // Repr::Simple(kind) => kind, - Repr::IoErr(ref err) => { - use std::io::ErrorKind::*; - match err.kind() { - NotFound => ErrorKind::NotFound, - PermissionDenied => ErrorKind::PermissionDenied, - ConnectionRefused => ErrorKind::ConnectionRefused, - ConnectionReset => ErrorKind::ConnectionReset, - ConnectionAborted => ErrorKind::ConnectionAborted, - NotConnected => ErrorKind::NotConnected, - AddrInUse => ErrorKind::AddrInUse, - AddrNotAvailable => ErrorKind::AddrNotAvailable, - BrokenPipe => ErrorKind::BrokenPipe, - AlreadyExists => ErrorKind::AlreadyExists, - WouldBlock => ErrorKind::WouldBlock, - InvalidInput => ErrorKind::InvalidInput, - InvalidData => ErrorKind::InvalidData, - TimedOut => ErrorKind::TimedOut, - Interrupted => ErrorKind::Interrupted, - WriteZero => ErrorKind::WriteZero, - Other => ErrorKind::Other, - UnexpectedEof => ErrorKind::UnexpectedEof, - _ => unreachable!(), - } - } - Repr::UrlErr(err) => Self::url_error_kind(err), - Repr::HyperErr(ref err) => { - // For some reason hyper::errors::Kind is private. - if err.is_parse() { - ErrorKind::HttpParse - } else if err.is_user() { - ErrorKind::HttpUser - } else if err.is_canceled() { - ErrorKind::HttpCanceled - } else if err.is_closed() { - ErrorKind::HttpClosed - } else { - ErrorKind::HttpOther - } - } - Repr::ImportMapErr(ref _err) => ErrorKind::ImportMapError, - Repr::ModuleResolutionErr(err) => { - use ModuleResolutionError::*; - match err { - InvalidUrl(err) | InvalidBaseUrl(err) => Self::url_error_kind(err), - InvalidPath => ErrorKind::InvalidPath, - ImportPrefixMissing => ErrorKind::ImportPrefixMissing, - } - } - Repr::Diagnostic(ref _err) => ErrorKind::Diagnostic, - Repr::JSError(ref _err) => ErrorKind::JSError, - Repr::DenoDirErr(ref _err) => ErrorKind::DenoDirError, - } - } +impl Error for StaticError {} - pub fn apply_source_map(self, getter: &G) -> Self { - if let Repr::JSError(js_error) = self.repr { - return DenoError { - repr: Repr::JSError(apply_source_map(&js_error, getter)), - }; - } else { - panic!("attempt to apply source map an unremappable error") - } +impl fmt::Display for StaticError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.pad(self.1) } } -impl fmt::Display for DenoError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self.repr { - Repr::Simple(_kind, ref err_str) => f.pad(err_str), - Repr::IoErr(ref err) => err.fmt(f), - Repr::UrlErr(ref err) => err.fmt(f), - Repr::HyperErr(ref err) => err.fmt(f), - Repr::ImportMapErr(ref err) => f.pad(&err.msg), - Repr::ModuleResolutionErr(ref err) => err.fmt(f), - Repr::Diagnostic(ref err) => err.fmt(f), - Repr::JSError(ref err) => JSErrorColor(err).fmt(f), - Repr::DenoDirErr(ref err) => err.fmt(f), - } - } +pub fn bad_resource() -> ErrBox { + StaticError(ErrorKind::BadResource, "bad resource id").into() } -impl std::error::Error for DenoError { - fn description(&self) -> &str { - match self.repr { - Repr::Simple(_kind, ref msg) => msg.as_str(), - Repr::IoErr(ref err) => err.description(), - Repr::UrlErr(ref err) => err.description(), - Repr::HyperErr(ref err) => err.description(), - Repr::ImportMapErr(ref err) => &err.msg, - Repr::ModuleResolutionErr(ref err) => err.description(), - Repr::Diagnostic(ref err) => &err.items[0].message, - Repr::JSError(ref err) => &err.description(), - Repr::DenoDirErr(ref err) => err.description(), - } - } +pub fn permission_denied() -> ErrBox { + StaticError(ErrorKind::PermissionDenied, "permission denied").into() +} - fn cause(&self) -> Option<&dyn std::error::Error> { - match self.repr { - Repr::Simple(_kind, ref _msg) => None, - Repr::IoErr(ref err) => Some(err), - Repr::UrlErr(ref err) => Some(err), - Repr::HyperErr(ref err) => Some(err), - Repr::ImportMapErr(ref _err) => None, - Repr::ModuleResolutionErr(ref err) => err.source(), - Repr::Diagnostic(ref _err) => None, - Repr::JSError(ref err) => Some(err), - Repr::DenoDirErr(ref err) => Some(err), - } - } +pub fn op_not_implemented() -> ErrBox { + StaticError(ErrorKind::OpNotAvailable, "op not implemented").into() } -impl From for DenoError { - #[inline] - fn from(err: io::Error) -> Self { - Self { - repr: Repr::IoErr(err), - } - } +pub fn no_buffer_specified() -> ErrBox { + StaticError(ErrorKind::InvalidInput, "no buffer specified").into() } -impl From for DenoError { - #[inline] - fn from(err: url::ParseError) -> Self { - Self { - repr: Repr::UrlErr(err), - } - } +pub fn no_async_support() -> ErrBox { + StaticError(ErrorKind::NoAsyncSupport, "op doesn't support async calls") + .into() } -impl From for DenoError { - #[inline] - fn from(err: hyper::Error) -> Self { - Self { - repr: Repr::HyperErr(err), - } - } +pub fn no_sync_support() -> ErrBox { + StaticError(ErrorKind::NoSyncSupport, "op doesn't support sync calls").into() } -impl From for DenoError { - fn from(e: ResolveAddrError) -> Self { - match e { - ResolveAddrError::Syntax => Self { - repr: Repr::Simple( - ErrorKind::InvalidInput, - "invalid address syntax".to_string(), - ), - }, - ResolveAddrError::Resolution(io_err) => Self { - repr: Repr::IoErr(io_err), - }, - } +pub fn invalid_address_syntax() -> ErrBox { + StaticError(ErrorKind::InvalidInput, "invalid address syntax").into() +} + +pub trait GetErrorKind { + fn kind(&self) -> ErrorKind; +} + +impl GetErrorKind for DenoError { + fn kind(&self) -> ErrorKind { + self.kind } } -#[cfg(unix)] -impl From for DenoError { - fn from(e: UnixError) -> Self { - match e { - UnixError::Sys(Errno::EPERM) => Self { - repr: Repr::Simple( - ErrorKind::PermissionDenied, - Errno::EPERM.desc().to_owned(), - ), - }, - UnixError::Sys(Errno::EINVAL) => Self { - repr: Repr::Simple( - ErrorKind::InvalidInput, - Errno::EINVAL.desc().to_owned(), - ), - }, - UnixError::Sys(Errno::ENOENT) => Self { - repr: Repr::Simple( - ErrorKind::NotFound, - Errno::ENOENT.desc().to_owned(), - ), - }, - UnixError::Sys(err) => Self { - repr: Repr::Simple(ErrorKind::UnixError, err.desc().to_owned()), - }, - _ => Self { - repr: Repr::Simple(ErrorKind::Other, format!("{}", e)), - }, - } +impl GetErrorKind for StaticError { + fn kind(&self) -> ErrorKind { + self.0 } } -impl From for DenoError { - fn from(err: import_map::ImportMapError) -> Self { - Self { - repr: Repr::ImportMapErr(err), - } +impl GetErrorKind for JSError { + fn kind(&self) -> ErrorKind { + ErrorKind::JSError } } -impl From for DenoError { - fn from(err: deno_dir::DenoDirError) -> Self { - Self { - repr: Repr::DenoDirErr(err), - } +impl GetErrorKind for Diagnostic { + fn kind(&self) -> ErrorKind { + ErrorKind::Diagnostic } } -impl From for DenoError { - fn from(err: ModuleResolutionError) -> Self { - Self { - repr: Repr::ModuleResolutionErr(err), - } +impl GetErrorKind for ImportMapError { + fn kind(&self) -> ErrorKind { + ErrorKind::ImportMapError } } -impl From for DenoError { - fn from(diagnostic: diagnostics::Diagnostic) -> Self { - Self { - repr: Repr::Diagnostic(diagnostic), +impl GetErrorKind for ModuleResolutionError { + fn kind(&self) -> ErrorKind { + use ModuleResolutionError::*; + match self { + InvalidUrl(ref err) | InvalidBaseUrl(ref err) => err.kind(), + InvalidPath => ErrorKind::InvalidPath, + ImportPrefixMissing => ErrorKind::ImportPrefixMissing, } } } -impl From for DenoError { - fn from(err: JSError) -> Self { - Self { - repr: Repr::JSError(err), +impl GetErrorKind for io::Error { + fn kind(&self) -> ErrorKind { + use io::ErrorKind::*; + match self.kind() { + NotFound => ErrorKind::NotFound, + PermissionDenied => ErrorKind::PermissionDenied, + ConnectionRefused => ErrorKind::ConnectionRefused, + ConnectionReset => ErrorKind::ConnectionReset, + ConnectionAborted => ErrorKind::ConnectionAborted, + NotConnected => ErrorKind::NotConnected, + AddrInUse => ErrorKind::AddrInUse, + AddrNotAvailable => ErrorKind::AddrNotAvailable, + BrokenPipe => ErrorKind::BrokenPipe, + AlreadyExists => ErrorKind::AlreadyExists, + WouldBlock => ErrorKind::WouldBlock, + InvalidInput => ErrorKind::InvalidInput, + InvalidData => ErrorKind::InvalidData, + TimedOut => ErrorKind::TimedOut, + Interrupted => ErrorKind::Interrupted, + WriteZero => ErrorKind::WriteZero, + UnexpectedEof => ErrorKind::UnexpectedEof, + _ => ErrorKind::Other, } } } -pub fn bad_resource() -> DenoError { - new(ErrorKind::BadResource, String::from("bad resource id")) +impl GetErrorKind for uri::InvalidUri { + fn kind(&self) -> ErrorKind { + // The http::uri::ErrorKind exists and is similar to url::ParseError. + // However it is also private, so we can't get any details out. + ErrorKind::InvalidUri + } } -pub fn permission_denied() -> DenoError { - new( - ErrorKind::PermissionDenied, - String::from("permission denied"), - ) +impl GetErrorKind for url::ParseError { + fn kind(&self) -> ErrorKind { + use url::ParseError::*; + match self { + EmptyHost => ErrorKind::EmptyHost, + IdnaError => ErrorKind::IdnaError, + InvalidDomainCharacter => ErrorKind::InvalidDomainCharacter, + InvalidIpv4Address => ErrorKind::InvalidIpv4Address, + InvalidIpv6Address => ErrorKind::InvalidIpv6Address, + InvalidPort => ErrorKind::InvalidPort, + Overflow => ErrorKind::Overflow, + RelativeUrlWithCannotBeABaseBase => { + ErrorKind::RelativeUrlWithCannotBeABaseBase + } + RelativeUrlWithoutBase => ErrorKind::RelativeUrlWithoutBase, + SetHostOnCannotBeABaseUrl => ErrorKind::SetHostOnCannotBeABaseUrl, + } + } } -pub fn op_not_implemented() -> DenoError { - new( - ErrorKind::OpNotAvailable, - String::from("op not implemented"), - ) +impl GetErrorKind for hyper::Error { + fn kind(&self) -> ErrorKind { + match self { + e if e.is_canceled() => ErrorKind::HttpCanceled, + e if e.is_closed() => ErrorKind::HttpClosed, + e if e.is_parse() => ErrorKind::HttpParse, + e if e.is_user() => ErrorKind::HttpUser, + _ => ErrorKind::HttpOther, + } + } } -pub fn worker_init_failed() -> DenoError { - // TODO(afinch7) pass worker error data through here - new( - ErrorKind::WorkerInitFailed, - String::from("worker init failed"), - ) +#[cfg(unix)] +mod unix { + use super::{ErrorKind, GetErrorKind}; + use nix::errno::Errno::*; + pub use nix::Error; + use nix::Error::Sys; + + impl GetErrorKind for Error { + fn kind(&self) -> ErrorKind { + match self { + Sys(EPERM) => ErrorKind::PermissionDenied, + Sys(EINVAL) => ErrorKind::InvalidInput, + Sys(ENOENT) => ErrorKind::NotFound, + Sys(_) => ErrorKind::UnixError, + _ => ErrorKind::Other, + } + } + } } -pub fn no_buffer_specified() -> DenoError { - new(ErrorKind::InvalidInput, String::from("no buffer specified")) -} +impl GetErrorKind for dyn AnyError { + fn kind(&self) -> ErrorKind { + use self::GetErrorKind as Get; -pub fn no_async_support() -> DenoError { - new( - ErrorKind::NoAsyncSupport, - String::from("op doesn't support async calls"), - ) -} + #[cfg(unix)] + fn unix_error_kind(err: &dyn AnyError) -> Option { + err.downcast_ref::().map(Get::kind) + } -pub fn no_sync_support() -> DenoError { - new( - ErrorKind::NoSyncSupport, - String::from("op doesn't support sync calls"), - ) -} + #[cfg(not(unix))] + fn unix_error_kind(_: &dyn AnyError) -> Option { + None + } -pub fn err_check(r: Result) { - if let Err(e) = r { - panic!(e.to_string()); + None + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| self.downcast_ref::().map(Get::kind)) + .or_else(|| unix_error_kind(self)) + .unwrap_or_else(|| { + panic!("Can't get ErrorKind for {:?}", self); + }) } } @@ -348,16 +241,15 @@ pub fn err_check(r: Result) { mod tests { use super::*; use crate::ansi::strip_ansi_codes; - use crate::deno_dir::DenoDirError; - use crate::deno_dir::DenoDirErrorKind; use crate::diagnostics::Diagnostic; use crate::diagnostics::DiagnosticCategory; use crate::diagnostics::DiagnosticItem; - use crate::import_map::ImportMapError; + use deno::ErrBox; use deno::StackFrame; + use deno::V8Exception; fn js_error() -> JSError { - JSError { + JSError::new(V8Exception { message: "Error: foo bar".to_string(), source_line: None, script_resource_name: None, @@ -396,7 +288,7 @@ mod tests { is_wasm: false, }, ], - } + }) } fn diagnostic() -> Diagnostic { @@ -436,22 +328,6 @@ mod tests { } } - struct MockSourceMapGetter {} - - impl SourceMapGetter for MockSourceMapGetter { - fn get_source_map(&self, _script_name: &str) -> Option> { - Some(vec![]) - } - - fn get_source_line( - &self, - _script_name: &str, - _line: usize, - ) -> Option { - None - } - } - fn io_error() -> io::Error { io::Error::from(io::ErrorKind::NotFound) } @@ -466,30 +342,24 @@ mod tests { } } - fn deno_dir_error() -> DenoDirError { - DenoDirError::new( - "a deno dir error".to_string(), - DenoDirErrorKind::UnsupportedFetchScheme, - ) - } - #[test] fn test_simple_error() { - let err = new(ErrorKind::NoError, "foo".to_string()); + let err = + ErrBox::from(DenoError::new(ErrorKind::NoError, "foo".to_string())); assert_eq!(err.kind(), ErrorKind::NoError); assert_eq!(err.to_string(), "foo"); } #[test] fn test_io_error() { - let err = DenoError::from(io_error()); + let err = ErrBox::from(io_error()); assert_eq!(err.kind(), ErrorKind::NotFound); assert_eq!(err.to_string(), "entity not found"); } #[test] fn test_url_error() { - let err = DenoError::from(url_error()); + let err = ErrBox::from(url_error()); assert_eq!(err.kind(), ErrorKind::EmptyHost); assert_eq!(err.to_string(), "empty host"); } @@ -498,32 +368,25 @@ mod tests { #[test] fn test_diagnostic() { - let err = DenoError::from(diagnostic()); + let err = ErrBox::from(diagnostic()); assert_eq!(err.kind(), ErrorKind::Diagnostic); assert_eq!(strip_ansi_codes(&err.to_string()), "error TS2322: Example 1\n\n► deno/tests/complex_diagnostics.ts:19:3\n\n19 values: o => [\n ~~~~~~\n\nerror TS2000: Example 2\n\n► /foo/bar.ts:129:3\n\n129 values: undefined,\n ~~~~~~\n\n\nFound 2 errors.\n"); } #[test] fn test_js_error() { - let err = DenoError::from(js_error()); + let err = ErrBox::from(js_error()); assert_eq!(err.kind(), ErrorKind::JSError); assert_eq!(strip_ansi_codes(&err.to_string()), "error: Error: foo bar\n at foo (foo_bar.ts:5:17)\n at qat (bar_baz.ts:6:21)\n at deno_main.js:2:2"); } #[test] fn test_import_map_error() { - let err = DenoError::from(import_map_error()); + let err = ErrBox::from(import_map_error()); assert_eq!(err.kind(), ErrorKind::ImportMapError); assert_eq!(err.to_string(), "an import map error"); } - #[test] - fn test_deno_dir_error() { - let err = DenoError::from(deno_dir_error()); - assert_eq!(err.kind(), ErrorKind::DenoDirError); - assert_eq!(err.to_string(), "a deno dir error\n"); - } - #[test] fn test_bad_resource() { let err = bad_resource(); @@ -545,13 +408,6 @@ mod tests { assert_eq!(err.to_string(), "op not implemented"); } - #[test] - fn test_worker_init_failed() { - let err = worker_init_failed(); - assert_eq!(err.kind(), ErrorKind::WorkerInitFailed); - assert_eq!(err.to_string(), "worker init failed"); - } - #[test] fn test_no_buffer_specified() { let err = no_buffer_specified(); @@ -572,20 +428,4 @@ mod tests { assert_eq!(err.kind(), ErrorKind::NoSyncSupport); assert_eq!(err.to_string(), "op doesn't support sync calls"); } - - #[test] - #[should_panic] - fn test_apply_source_map_invalid() { - let getter = MockSourceMapGetter {}; - let err = new(ErrorKind::NotFound, "not found".to_string()); - err.apply_source_map(&getter); - } - - #[test] - #[should_panic] - fn test_err_check() { - err_check( - Err(new(ErrorKind::NotFound, "foo".to_string())) as Result<(), DenoError> - ); - } } diff --git a/cli/diagnostics.rs b/cli/diagnostics.rs index 69b58459e330cd..14f5ba2c410113 100644 --- a/cli/diagnostics.rs +++ b/cli/diagnostics.rs @@ -7,6 +7,7 @@ use crate::fmt_errors::format_maybe_source_name; use crate::fmt_errors::DisplayFormatter; use serde_json; use serde_json::value::Value; +use std::error::Error; use std::fmt; #[derive(Debug, PartialEq, Clone)] @@ -66,6 +67,12 @@ impl fmt::Display for Diagnostic { } } +impl Error for Diagnostic { + fn description(&self) -> &str { + &self.items[0].message + } +} + #[derive(Debug, PartialEq, Clone)] pub struct DiagnosticItem { /// The top level message relating to the diagnostic item. diff --git a/cli/dispatch_minimal.rs b/cli/dispatch_minimal.rs index bda9ea535eda65..9d82595d15f652 100644 --- a/cli/dispatch_minimal.rs +++ b/cli/dispatch_minimal.rs @@ -127,10 +127,11 @@ mod ops { use crate::deno_error; use crate::resources; use crate::tokio_write; + use deno::ErrBox; use deno::PinnedBuf; use futures::Future; - type MinimalOp = dyn Future + Send; + type MinimalOp = dyn Future + Send; pub fn read(rid: i32, zero_copy: Option) -> Box { debug!("read rid={}", rid); @@ -144,7 +145,7 @@ mod ops { None => Box::new(futures::future::err(deno_error::bad_resource())), Some(resource) => Box::new( tokio::io::read(resource, zero_copy) - .map_err(deno_error::DenoError::from) + .map_err(ErrBox::from) .and_then(move |(_resource, _buf, nread)| Ok(nread as i32)), ), } @@ -162,7 +163,7 @@ mod ops { None => Box::new(futures::future::err(deno_error::bad_resource())), Some(resource) => Box::new( tokio_write::write(resource, zero_copy) - .map_err(deno_error::DenoError::from) + .map_err(ErrBox::from) .and_then(move |(_resource, _buf, nwritten)| Ok(nwritten as i32)), ), } diff --git a/cli/fmt_errors.rs b/cli/fmt_errors.rs index 957e799ca4dc2f..60b4e3356cee3f 100644 --- a/cli/fmt_errors.rs +++ b/cli/fmt_errors.rs @@ -1,8 +1,12 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. //! This mod provides DenoError to unify errors across Deno. use crate::ansi; -use deno::JSError; +use crate::source_maps::apply_source_map; +use crate::source_maps::SourceMapGetter; +use deno::ErrBox; use deno::StackFrame; +use deno::V8Exception; +use std::error::Error; use std::fmt; /// A trait which specifies parts of a diagnostic like item needs to be able to @@ -104,10 +108,50 @@ pub fn format_error_message(msg: String) -> String { format!("{} {}", preamble, msg) } -/// Wrapper around JSError which provides color to_string. -pub struct JSErrorColor<'a>(pub &'a JSError); +fn format_stack_frame(frame: &StackFrame) -> String { + // Note when we print to string, we change from 0-indexed to 1-indexed. + let function_name = ansi::italic_bold(frame.function_name.clone()); + let source_loc = + format_source_name(frame.script_name.clone(), frame.line, frame.column); -impl<'a> DisplayFormatter for JSErrorColor<'a> { + if !frame.function_name.is_empty() { + format!(" at {} ({})", function_name, source_loc) + } else if frame.is_eval { + format!(" at eval ({})", source_loc) + } else { + format!(" at {}", source_loc) + } +} + +/// Wrapper around V8Exception which provides color to_string. +#[derive(Debug)] +pub struct JSError(V8Exception); + +impl JSError { + pub fn new(v8_exception: V8Exception) -> Self { + Self(v8_exception) + } + + pub fn from_json( + json_str: &str, + source_map_getter: &impl SourceMapGetter, + ) -> ErrBox { + let unmapped_exception = V8Exception::from_json(json_str).unwrap(); + Self::from_v8_exception(unmapped_exception, source_map_getter) + } + + pub fn from_v8_exception( + unmapped_exception: V8Exception, + source_map_getter: &impl SourceMapGetter, + ) -> ErrBox { + let mapped_exception = + apply_source_map(&unmapped_exception, source_map_getter); + let js_error = Self(mapped_exception); + ErrBox::from(js_error) + } +} + +impl DisplayFormatter for JSError { fn format_category_and_code(&self) -> String { "".to_string() } @@ -136,7 +180,7 @@ impl<'a> DisplayFormatter for JSErrorColor<'a> { } fn format_source_name(&self) -> String { - let e = self.0; + let e = &self.0; if e.script_resource_name.is_none() { return "".to_string(); } @@ -152,7 +196,7 @@ impl<'a> DisplayFormatter for JSErrorColor<'a> { } } -impl<'a> fmt::Display for JSErrorColor<'a> { +impl fmt::Display for JSError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!( f, @@ -163,39 +207,21 @@ impl<'a> fmt::Display for JSErrorColor<'a> { )?; for frame in &self.0.frames { - write!(f, "\n{}", StackFrameColor(&frame).to_string())?; + write!(f, "\n{}", format_stack_frame(&frame))?; } Ok(()) } } -struct StackFrameColor<'a>(&'a StackFrame); - -impl<'a> fmt::Display for StackFrameColor<'a> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let frame = self.0; - // Note when we print to string, we change from 0-indexed to 1-indexed. - let function_name = ansi::italic_bold(frame.function_name.clone()); - let script_line_column = - format_source_name(frame.script_name.clone(), frame.line, frame.column); - - if !frame.function_name.is_empty() { - write!(f, " at {} ({})", function_name, script_line_column) - } else if frame.is_eval { - write!(f, " at eval ({})", script_line_column) - } else { - write!(f, " at {}", script_line_column) - } - } -} +impl Error for JSError {} #[cfg(test)] mod tests { use super::*; use crate::ansi::strip_ansi_codes; - fn error1() -> JSError { - JSError { + fn error1() -> V8Exception { + V8Exception { message: "Error: foo bar".to_string(), source_line: None, script_resource_name: None, @@ -240,7 +266,7 @@ mod tests { #[test] fn js_error_to_string() { let e = error1(); - assert_eq!("error: Error: foo bar\n at foo (foo_bar.ts:5:17)\n at qat (bar_baz.ts:6:21)\n at deno_main.js:2:2", strip_ansi_codes(&JSErrorColor(&e).to_string())); + assert_eq!("error: Error: foo bar\n at foo (foo_bar.ts:5:17)\n at qat (bar_baz.ts:6:21)\n at deno_main.js:2:2", strip_ansi_codes(&JSError(e).to_string())); } #[test] diff --git a/cli/fs.rs b/cli/fs.rs index 3b5709804d2cf2..6fe89114152487 100644 --- a/cli/fs.rs +++ b/cli/fs.rs @@ -5,6 +5,7 @@ use std::io::ErrorKind; use std::io::Write; use std::path::{Path, PathBuf}; +use deno::ErrBox; use rand; use rand::Rng; @@ -15,8 +16,6 @@ use std::os::unix::fs::DirBuilderExt; #[cfg(any(unix))] use std::os::unix::fs::PermissionsExt; -use crate::deno_error::DenoResult; - pub fn write_file>( filename: &Path, data: T, @@ -114,18 +113,16 @@ pub fn normalize_path(path: &Path) -> String { } #[cfg(unix)] -pub fn chown(path: &str, uid: u32, gid: u32) -> DenoResult<()> { - use crate::deno_error::DenoError; +pub fn chown(path: &str, uid: u32, gid: u32) -> Result<(), ErrBox> { let nix_uid = Uid::from_raw(uid); let nix_gid = Gid::from_raw(gid); unix_chown(path, Option::Some(nix_uid), Option::Some(nix_gid)) - .map_err(DenoError::from) + .map_err(ErrBox::from) } #[cfg(not(unix))] -pub fn chown(_path: &str, _uid: u32, _gid: u32) -> DenoResult<()> { +pub fn chown(_path: &str, _uid: u32, _gid: u32) -> Result<(), ErrBox> { // Noop // TODO: implement chown for Windows - use crate::deno_error; - Err(deno_error::op_not_implemented()) + Err(crate::deno_error::op_not_implemented()) } diff --git a/cli/http_util.rs b/cli/http_util.rs index a3310b1bf7a594..16c41930510904 100644 --- a/cli/http_util.rs +++ b/cli/http_util.rs @@ -1,6 +1,7 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. use crate::deno_error; use crate::deno_error::DenoError; +use deno::ErrBox; #[cfg(test)] use futures::future::{loop_fn, Loop}; use futures::{future, Future, Stream}; @@ -57,13 +58,13 @@ fn resolve_uri_from_location(base_uri: &Uri, location: &str) -> Uri { } } -#[cfg(test)] -use crate::deno_error::DenoResult; #[cfg(test)] use crate::tokio_util; #[cfg(test)] /// Synchronously fetchs the given HTTP URL. Returns (content, media_type). -pub fn fetch_sync_string(module_name: &str) -> DenoResult<(String, String)> { +pub fn fetch_sync_string( + module_name: &str, +) -> Result<(String, String), ErrBox> { tokio_util::block_on(fetch_string(module_name)) } @@ -80,15 +81,15 @@ pub enum FetchOnceResult { /// yields Redirect(url). pub fn fetch_string_once( url: http::uri::Uri, -) -> impl Future { +) -> impl Future { type FetchAttempt = (Option, Option, Option); let client = get_client(); client .get(url.clone()) - .map_err(DenoError::from) + .map_err(ErrBox::from) .and_then( move |response| -> Box< - dyn Future + Send, + dyn Future + Send, > { if response.status().is_redirection() { let location_string = response @@ -108,10 +109,10 @@ pub fn fetch_string_once( } else if response.status().is_client_error() || response.status().is_server_error() { - return Box::new(future::err(deno_error::new( + return Box::new(future::err(DenoError::new( deno_error::ErrorKind::Other, format!("Import '{}' failed: {}", &url, response.status()), - ))); + ).into())); } let content_type = response .headers() @@ -121,7 +122,7 @@ pub fn fetch_string_once( .into_body() .concat2() .map(|body| String::from_utf8(body.to_vec()).ok()) - .map_err(DenoError::from); + .map_err(ErrBox::from); Box::new(body.join3(future::ok(content_type), future::ok(None))) }, ) @@ -142,7 +143,7 @@ pub fn fetch_string_once( /// Asynchronously fetchs the given HTTP URL. Returns (content, media_type). pub fn fetch_string( module_name: &str, -) -> impl Future { +) -> impl Future { let url = module_name.parse::().unwrap(); let client = get_client(); // TODO(kevinkassimo): consider set a max redirection counter @@ -150,7 +151,7 @@ pub fn fetch_string( loop_fn((client, url), |(client, url)| { client .get(url.clone()) - .map_err(DenoError::from) + .map_err(ErrBox::from) .and_then(move |response| { if response.status().is_redirection() { let location_string = response @@ -165,10 +166,12 @@ pub fn fetch_string( return Ok(Loop::Continue((client, new_url))); } if !response.status().is_success() { - return Err(deno_error::new( - deno_error::ErrorKind::NotFound, - "module not found".to_string(), - )); + return Err( + DenoError::new( + deno_error::ErrorKind::NotFound, + "module not found".to_string(), + ).into(), + ); } Ok(Loop::Break(response)) }) @@ -181,7 +184,7 @@ pub fn fetch_string( .into_body() .concat2() .map(|body| String::from_utf8(body.to_vec()).unwrap()) - .map_err(DenoError::from); + .map_err(ErrBox::from); body.join(future::ok(content_type)) }).and_then(|(body_string, maybe_content_type)| { future::ok((body_string, maybe_content_type.unwrap())) diff --git a/cli/import_map.rs b/cli/import_map.rs index 1f136a3e989fbd..abe9835f8cf850 100644 --- a/cli/import_map.rs +++ b/cli/import_map.rs @@ -3,6 +3,8 @@ use indexmap::IndexMap; use serde_json::Map; use serde_json::Value; use std::cmp::Ordering; +use std::error::Error; +use std::fmt; use std::fs; use url::Url; @@ -19,6 +21,14 @@ impl ImportMapError { } } +impl fmt::Display for ImportMapError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.pad(&self.msg) + } +} + +impl Error for ImportMapError {} + // NOTE: here is difference between deno and reference implementation - deno currently // can't resolve URL with other schemes (eg. data:, about:, blob:) const SUPPORTED_FETCH_SCHEMES: [&str; 3] = ["http", "https", "file"]; diff --git a/cli/main.rs b/cli/main.rs index 02409d7506611e..50d65c06dd09d4 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -46,11 +46,11 @@ pub mod version; pub mod worker; use crate::compiler::bundle_async; -use crate::deno_error::DenoError; use crate::progress::Progress; use crate::state::ThreadSafeState; use crate::worker::Worker; use deno::v8_set_flags; +use deno::ErrBox; use deno::ModuleSpecifier; use flags::DenoFlags; use flags::DenoSubcommand; @@ -86,17 +86,14 @@ impl log::Log for Logger { fn flush(&self) {} } -fn print_err_and_exit(err: DenoError) { +fn print_err_and_exit(err: ErrBox) { eprintln!("{}", err.to_string()); std::process::exit(1); } -fn js_check(r: Result<(), E>) -where - E: Into, -{ +fn js_check(r: Result<(), ErrBox>) { if let Err(err) = r { - print_err_and_exit(err.into()); + print_err_and_exit(err); } } @@ -264,7 +261,7 @@ fn xeval_command(flags: DenoFlags, argv: Vec) { .then(|result| { js_check(result); Ok(()) - }).map_err(|(err, _worker): (DenoError, Worker)| print_err_and_exit(err)) + }).map_err(print_err_and_exit) }); tokio_util::run(main_future); } @@ -277,10 +274,10 @@ fn bundle_command(flags: DenoFlags, argv: Vec) { let out_file = state.argv[2].clone(); debug!(">>>>> bundle_async START"); let bundle_future = bundle_async(state, main_module.to_string(), out_file) - .map_err(|e| { + .map_err(|err| { debug!("diagnostics returned, exiting!"); - eprintln!("\n{}", e.to_string()); - std::process::exit(1); + eprintln!(""); + print_err_and_exit(err); }).and_then(move |_| { debug!(">>>>> bundle_async END"); Ok(()) @@ -299,7 +296,7 @@ fn run_repl(flags: DenoFlags, argv: Vec) { .then(|result| { js_check(result); Ok(()) - }).map_err(|(err, _worker): (DenoError, Worker)| print_err_and_exit(err)) + }).map_err(|(err, _worker): (ErrBox, Worker)| print_err_and_exit(err)) }); tokio_util::run(main_future); } diff --git a/cli/msg.fbs b/cli/msg.fbs index 08da7c3b55e203..9b531147dfd3fc 100644 --- a/cli/msg.fbs +++ b/cli/msg.fbs @@ -102,7 +102,6 @@ enum ErrorKind: byte { WouldBlock, InvalidInput, InvalidData, - InvalidPath, TimedOut, Interrupted, WriteZero, @@ -142,8 +141,9 @@ enum ErrorKind: byte { NoAsyncSupport, NoSyncSupport, ImportMapError, + InvalidPath, ImportPrefixMissing, - DenoDirError, + UnsupportedFetchScheme, // other kinds Diagnostic, diff --git a/cli/msg_util.rs b/cli/msg_util.rs index 0ecb7f0d634cc6..e37a91f3a6ba56 100644 --- a/cli/msg_util.rs +++ b/cli/msg_util.rs @@ -1,9 +1,7 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. // Helpers for serialization. -use crate::deno_error; -use crate::deno_error::DenoResult; use crate::msg; - +use deno::ErrBox; use flatbuffers; use http::header::HeaderName; use http::uri::Uri; @@ -97,14 +95,13 @@ pub fn serialize_http_response<'bldr>( pub fn deserialize_request( header_msg: msg::HttpHeader<'_>, body: Body, -) -> DenoResult> { +) -> Result, ErrBox> { let mut r = Request::new(body); assert!(header_msg.is_request()); let u = header_msg.url().unwrap(); - let u = Uri::from_str(u) - .map_err(|e| deno_error::new(msg::ErrorKind::InvalidUri, e.to_string()))?; + let u = Uri::from_str(u).map_err(ErrBox::from)?; *r.uri_mut() = u; if let Some(method) = header_msg.method() { diff --git a/cli/ops.rs b/cli/ops.rs index 770baf4edf579c..018d2ea09082d5 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -3,12 +3,12 @@ use atty; use crate::ansi; use crate::deno_dir::resolve_from_cwd; use crate::deno_error; -use crate::deno_error::err_check; use crate::deno_error::DenoError; -use crate::deno_error::DenoResult; use crate::deno_error::ErrorKind; +use crate::deno_error::GetErrorKind; use crate::dispatch_minimal::dispatch_minimal; use crate::dispatch_minimal::parse_min_record; +use crate::fmt_errors::JSError; use crate::fs as deno_fs; use crate::http_util; use crate::msg; @@ -28,7 +28,7 @@ use crate::version; use crate::worker::Worker; use deno::Buf; use deno::CoreOp; -use deno::JSError; +use deno::ErrBox; use deno::Loader; use deno::ModuleSpecifier; use deno::Op; @@ -65,7 +65,7 @@ use std::os::unix::fs::PermissionsExt; #[cfg(unix)] use std::os::unix::process::ExitStatusExt; -type CliOpResult = OpResult; +type CliOpResult = OpResult; type CliDispatchFn = fn(state: &ThreadSafeState, base: &msg::Base<'_>, data: Option) @@ -132,7 +132,7 @@ pub fn dispatch_all_legacy( } Ok(Op::Async(fut)) => { let result_fut = Box::new( - fut.or_else(move |err: DenoError| -> Result { + fut.or_else(move |err: ErrBox| -> Result { debug!("op err {}", err); // No matter whether we got an Err or Ok, we want a serialized message to // send back. So transform the DenoError into a Buf. @@ -410,11 +410,9 @@ fn op_format_error( ) -> CliOpResult { assert!(data.is_none()); let inner = base.inner_as_format_error().unwrap(); - let orig_error = String::from(inner.error().unwrap()); - - let js_error = JSError::from_v8_exception(&orig_error).unwrap(); - let error_mapped = DenoError::from(js_error).apply_source_map(&state.dir); - let error_string = error_mapped.to_string(); + let json_str = inner.error().unwrap(); + let error = JSError::from_json(json_str, &state.dir); + let error_string = error.to_string(); let mut builder = FlatBufferBuilder::new(); let new_error = builder.create_string(&error_string); @@ -495,10 +493,10 @@ fn op_cache( if extension == ".map" { debug!("cache {:?}", source_map_path); - fs::write(source_map_path, contents).map_err(DenoError::from)?; + fs::write(source_map_path, contents).map_err(ErrBox::from)?; } else if extension == ".js" { debug!("cache {:?}", js_cache_path); - fs::write(js_cache_path, contents).map_err(DenoError::from)?; + fs::write(js_cache_path, contents).map_err(ErrBox::from)?; } else { unreachable!(); } @@ -735,39 +733,38 @@ fn op_fetch( let req = msg_util::deserialize_request(header, body)?; - let url_ = url::Url::parse(url).map_err(DenoError::from)?; + let url_ = url::Url::parse(url).map_err(ErrBox::from)?; state.check_net_url(url_)?; let client = http_util::get_client(); debug!("Before fetch {}", url); - let future = - client - .request(req) - .map_err(DenoError::from) - .and_then(move |res| { - let builder = &mut FlatBufferBuilder::new(); - let header_off = msg_util::serialize_http_response(builder, &res); - let body = res.into_body(); - let body_resource = resources::add_hyper_body(body); - let inner = msg::FetchRes::create( - builder, - &msg::FetchResArgs { - header: Some(header_off), - body_rid: body_resource.rid, - }, - ); + let future = client + .request(req) + .map_err(ErrBox::from) + .and_then(move |res| { + let builder = &mut FlatBufferBuilder::new(); + let header_off = msg_util::serialize_http_response(builder, &res); + let body = res.into_body(); + let body_resource = resources::add_hyper_body(body); + let inner = msg::FetchRes::create( + builder, + &msg::FetchResArgs { + header: Some(header_off), + body_rid: body_resource.rid, + }, + ); - Ok(serialize_response( - cmd_id, - builder, - msg::BaseArgs { - inner: Some(inner.as_union_value()), - inner_type: msg::Any::FetchRes, - ..Default::default() - }, - )) - }); + Ok(serialize_response( + cmd_id, + builder, + msg::BaseArgs { + inner: Some(inner.as_union_value()), + inner_type: msg::Any::FetchRes, + ..Default::default() + }, + )) + }); if base.sync() { let result_buf = future.wait()?; Ok(Op::Sync(result_buf)) @@ -778,9 +775,9 @@ fn op_fetch( // This is just type conversion. Implement From trait? // See https://github.com/tokio-rs/tokio/blob/ffd73a64e7ec497622b7f939e38017afe7124dc4/tokio-fs/src/lib.rs#L76-L85 -fn convert_blocking(f: F) -> Poll +fn convert_blocking(f: F) -> Poll where - F: FnOnce() -> DenoResult, + F: FnOnce() -> Result, { use futures::Async::*; match tokio_threadpool::blocking(f) { @@ -793,7 +790,7 @@ where fn blocking(is_sync: bool, f: F) -> CliOpResult where - F: 'static + Send + FnOnce() -> DenoResult, + F: 'static + Send + FnOnce() -> Result, { if is_sync { let result_buf = f()?; @@ -981,10 +978,8 @@ fn op_open( } } - let op = open_options - .open(filename) - .map_err(DenoError::from) - .and_then(move |fs_file| { + let op = open_options.open(filename).map_err(ErrBox::from).and_then( + move |fs_file| { let resource = resources::add_fs_file(fs_file); let builder = &mut FlatBufferBuilder::new(); let inner = @@ -998,7 +993,8 @@ fn op_open( ..Default::default() }, )) - }); + }, + ); if base.sync() { let buf = op.wait()?; Ok(Op::Sync(buf)) @@ -1076,7 +1072,7 @@ fn op_read( None => Err(deno_error::bad_resource()), Some(resource) => { let op = tokio::io::read(resource, data.unwrap()) - .map_err(DenoError::from) + .map_err(ErrBox::from) .and_then(move |(_resource, _buf, nread)| { let builder = &mut FlatBufferBuilder::new(); let inner = msg::ReadRes::create( @@ -1119,7 +1115,7 @@ fn op_write( None => Err(deno_error::bad_resource()), Some(resource) => { let op = tokio_write::write(resource, data.unwrap()) - .map_err(DenoError::from) + .map_err(ErrBox::from) .and_then(move |(_resource, _buf, nwritten)| { let builder = &mut FlatBufferBuilder::new(); let inner = msg::WriteRes::create( @@ -1219,10 +1215,10 @@ fn op_copy_file( // See https://github.com/rust-lang/rust/issues/54800 // Once the issue is reolved, we should remove this workaround. if cfg!(unix) && !from.is_file() { - return Err(deno_error::new( - ErrorKind::NotFound, - "File not found".to_string(), - )); + return Err( + DenoError::new(ErrorKind::NotFound, "File not found".to_string()) + .into(), + ); } fs::copy(&from, &to)?; @@ -1431,10 +1427,9 @@ fn op_symlink( state.check_write(&newname_)?; // TODO Use type for Windows. if cfg!(windows) { - return Err(deno_error::new( - ErrorKind::Other, - "Not implemented".to_string(), - )); + return Err( + DenoError::new(ErrorKind::Other, "Not implemented".to_string()).into(), + ); } blocking(base.sync(), move || { debug!("op_symlink {} {}", oldname.display(), newname.display()); @@ -1621,7 +1616,7 @@ fn op_listen( ok_buf(response_buf) } -fn new_conn(cmd_id: u32, tcp_stream: TcpStream) -> DenoResult { +fn new_conn(cmd_id: u32, tcp_stream: TcpStream) -> Result { let tcp_stream_resource = resources::add_tcp_stream(tcp_stream); // TODO forward socket_addr to client. @@ -1658,7 +1653,7 @@ fn op_accept( None => Err(deno_error::bad_resource()), Some(server_resource) => { let op = tokio_util::accept(server_resource) - .map_err(DenoError::from) + .map_err(ErrBox::from) .and_then(move |(tcp_stream, _socket_addr)| { new_conn(cmd_id, tcp_stream) }); @@ -1686,14 +1681,11 @@ fn op_dial( state.check_net(&address)?; - let op = - resolve_addr(address) - .map_err(DenoError::from) - .and_then(move |addr| { - TcpStream::connect(&addr) - .map_err(DenoError::from) - .and_then(move |tcp_stream| new_conn(cmd_id, tcp_stream)) - }); + let op = resolve_addr(address).and_then(move |addr| { + TcpStream::connect(&addr) + .map_err(ErrBox::from) + .and_then(move |tcp_stream| new_conn(cmd_id, tcp_stream)) + }); if base.sync() { let buf = op.wait()?; Ok(Op::Sync(buf)) @@ -1858,7 +1850,7 @@ fn op_run( } // Spawn the command. - let child = c.spawn_async().map_err(DenoError::from)?; + let child = c.spawn_async().map_err(ErrBox::from)?; let pid = child.id(); let resources = resources::add_child(child); @@ -1977,8 +1969,8 @@ fn op_worker_get_message( let op = GetMessageFuture { state: state.clone(), }; - let op = op.map_err(move |_| -> DenoError { unimplemented!() }); - let op = op.and_then(move |maybe_buf| -> DenoResult { + let op = op.map_err(move |_| -> ErrBox { unimplemented!() }); + let op = op.and_then(move |maybe_buf| -> Result { debug!("op_worker_get_message"); let builder = &mut FlatBufferBuilder::new(); @@ -2015,7 +2007,7 @@ fn op_worker_post_message( }; tx.send(d) .wait() - .map_err(|e| deno_error::new(ErrorKind::Other, e.to_string()))?; + .map_err(|e| DenoError::new(ErrorKind::Other, e.to_string()))?; let builder = &mut FlatBufferBuilder::new(); ok_buf(serialize_response( @@ -2051,8 +2043,8 @@ fn op_create_worker( let mut worker = Worker::new(name, startup_data::deno_isolate_init(), child_state); - err_check(worker.execute("denoMain()")); - err_check(worker.execute("workerMain()")); + worker.execute("denoMain()").unwrap(); + worker.execute("workerMain()").unwrap(); let module_specifier = ModuleSpecifier::resolve_url_or_path(specifier)?; @@ -2132,8 +2124,8 @@ fn op_host_get_message( let rid = inner.rid(); let op = resources::get_message_from_worker(rid); - let op = op.map_err(move |_| -> DenoError { unimplemented!() }); - let op = op.and_then(move |maybe_buf| -> DenoResult { + let op = op.map_err(move |_| -> ErrBox { unimplemented!() }); + let op = op.and_then(move |maybe_buf| -> Result { let builder = &mut FlatBufferBuilder::new(); let data = maybe_buf.as_ref().map(|buf| builder.create_vector(buf)); @@ -2168,7 +2160,7 @@ fn op_host_post_message( resources::post_message_to_worker(rid, d) .wait() - .map_err(|e| deno_error::new(ErrorKind::Other, e.to_string()))?; + .map_err(|e| DenoError::new(ErrorKind::Other, e.to_string()))?; let builder = &mut FlatBufferBuilder::new(); ok_buf(serialize_response( diff --git a/cli/permissions.rs b/cli/permissions.rs index c1878b6ff807f6..8549e97795bdeb 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -1,11 +1,9 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. -use atty; - -use crate::flags::DenoFlags; - use ansi_term::Style; +use atty; use crate::deno_error::permission_denied; -use crate::deno_error::DenoResult; +use crate::flags::DenoFlags; +use deno::ErrBox; use log; use std::collections::HashSet; use std::fmt; @@ -160,7 +158,7 @@ impl DenoPermissions { } } - pub fn check_run(&self) -> DenoResult<()> { + pub fn check_run(&self) -> Result<(), ErrBox> { let msg = "access to run a subprocess"; match self.allow_run.get_state() { @@ -181,7 +179,7 @@ impl DenoPermissions { } } - pub fn check_read(&self, filename: &str) -> DenoResult<()> { + pub fn check_read(&self, filename: &str) -> Result<(), ErrBox> { let msg = &format!("read access to \"{}\"", filename); match self.allow_read.get_state() { PermissionAccessorState::Allow => { @@ -213,7 +211,7 @@ impl DenoPermissions { } } - pub fn check_write(&self, filename: &str) -> DenoResult<()> { + pub fn check_write(&self, filename: &str) -> Result<(), ErrBox> { let msg = &format!("write access to \"{}\"", filename); match self.allow_write.get_state() { PermissionAccessorState::Allow => { @@ -245,7 +243,7 @@ impl DenoPermissions { } } - pub fn check_net(&self, host_and_port: &str) -> DenoResult<()> { + pub fn check_net(&self, host_and_port: &str) -> Result<(), ErrBox> { let msg = &format!("network access to \"{}\"", host_and_port); match self.allow_net.get_state() { PermissionAccessorState::Allow => { @@ -276,7 +274,7 @@ impl DenoPermissions { } } - pub fn check_net_url(&self, url: url::Url) -> DenoResult<()> { + pub fn check_net_url(&self, url: url::Url) -> Result<(), ErrBox> { let msg = &format!("network access to \"{}\"", url); match self.allow_net.get_state() { PermissionAccessorState::Allow => { @@ -311,7 +309,7 @@ impl DenoPermissions { &self, state: PermissionAccessorState, prompt_str: &str, - ) -> DenoResult<()> { + ) -> Result<(), ErrBox> { match state { PermissionAccessorState::Ask => { match self.try_permissions_prompt(prompt_str) { @@ -329,7 +327,7 @@ impl DenoPermissions { } } - pub fn check_env(&self) -> DenoResult<()> { + pub fn check_env(&self) -> Result<(), ErrBox> { let msg = "access to environment variables"; match self.allow_env.get_state() { PermissionAccessorState::Allow => { @@ -351,7 +349,10 @@ impl DenoPermissions { /// Try to present the user with a permission prompt /// will error with permission_denied if no_prompts is enabled - fn try_permissions_prompt(&self, message: &str) -> DenoResult { + fn try_permissions_prompt( + &self, + message: &str, + ) -> Result { if self.no_prompts.load(Ordering::SeqCst) { return Err(permission_denied()); } @@ -396,31 +397,31 @@ impl DenoPermissions { self.allow_hrtime.is_allow() } - pub fn revoke_run(&self) -> DenoResult<()> { + pub fn revoke_run(&self) -> Result<(), ErrBox> { self.allow_run.revoke(); Ok(()) } - pub fn revoke_read(&self) -> DenoResult<()> { + pub fn revoke_read(&self) -> Result<(), ErrBox> { self.allow_read.revoke(); Ok(()) } - pub fn revoke_write(&self) -> DenoResult<()> { + pub fn revoke_write(&self) -> Result<(), ErrBox> { self.allow_write.revoke(); Ok(()) } - pub fn revoke_net(&self) -> DenoResult<()> { + pub fn revoke_net(&self) -> Result<(), ErrBox> { self.allow_net.revoke(); Ok(()) } - pub fn revoke_env(&self) -> DenoResult<()> { + pub fn revoke_env(&self) -> Result<(), ErrBox> { self.allow_env.revoke(); Ok(()) } - pub fn revoke_hrtime(&self) -> DenoResult<()> { + pub fn revoke_hrtime(&self) -> Result<(), ErrBox> { self.allow_hrtime.revoke(); Ok(()) } @@ -437,7 +438,7 @@ pub enum PromptResult { impl PromptResult { /// If value is any form of deny this will error with permission_denied - pub fn check(&self) -> DenoResult<()> { + pub fn check(&self) -> Result<(), ErrBox> { match self { PromptResult::DenyOnce => Err(permission_denied()), PromptResult::DenyAlways => Err(permission_denied()), @@ -457,7 +458,7 @@ impl fmt::Display for PromptResult { } } -fn permission_prompt(message: &str) -> DenoResult { +fn permission_prompt(message: &str) -> Result { let msg = format!("️{} Deno requests {}. Grant? [a/y/n/d (a = allow always, y = allow once, n = deny once, d = deny always)] ", PERMISSION_EMOJI, message); // print to stderr so that if deno is > to a file this is still displayed. eprint!("{}", Style::new().bold().paint(msg)); diff --git a/cli/repl.rs b/cli/repl.rs index e0d029439bd2e9..a253e4435d664a 100644 --- a/cli/repl.rs +++ b/cli/repl.rs @@ -1,12 +1,7 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. -use rustyline; - -use crate::msg::ErrorKind; -use std::error::Error; - use crate::deno_dir::DenoDir; -use crate::deno_error::new as deno_error; -use crate::deno_error::DenoResult; +use deno::ErrBox; +use rustyline; use std::path::PathBuf; #[cfg(not(windows))] @@ -78,25 +73,25 @@ impl Repl { .unwrap_or(()) } - fn save_history(&mut self) -> DenoResult<()> { + fn save_history(&mut self) -> Result<(), ErrBox> { self .editor .save_history(&self.history_file.to_str().unwrap()) .map(|_| debug!("Saved REPL history to: {:?}", self.history_file)) .map_err(|e| { eprintln!("Unable to save REPL history: {:?} {}", self.history_file, e); - deno_error(ErrorKind::Other, e.description().to_string()) + ErrBox::from(e) }) } - pub fn readline(&mut self, prompt: &str) -> DenoResult { + pub fn readline(&mut self, prompt: &str) -> Result { self .editor .readline(&prompt) .map(|line| { self.editor.add_history_entry(line.clone()); line - }).map_err(|e| deno_error(ErrorKind::Other, e.description().to_string())) + }).map_err(ErrBox::from) // Forward error to TS side for processing } } diff --git a/cli/resolve_addr.rs b/cli/resolve_addr.rs index f26655be1f6854..b783444d86dfa7 100644 --- a/cli/resolve_addr.rs +++ b/cli/resolve_addr.rs @@ -1,10 +1,9 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. - +use crate::deno_error; +use deno::ErrBox; use futures::Async; use futures::Future; use futures::Poll; -use std::error::Error; -use std::fmt; use std::net::SocketAddr; use std::net::ToSocketAddrs; @@ -21,46 +20,23 @@ pub fn resolve_addr(address: &str) -> ResolveAddrFuture { } } -#[derive(Debug)] -pub enum ResolveAddrError { - Syntax, - Resolution(std::io::Error), -} - -impl fmt::Display for ResolveAddrError { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt.write_str(self.description()) - } -} - -impl Error for ResolveAddrError { - fn description(&self) -> &str { - match self { - ResolveAddrError::Syntax => "invalid address syntax", - ResolveAddrError::Resolution(e) => e.description(), - } - } -} - pub struct ResolveAddrFuture { address: String, } impl Future for ResolveAddrFuture { type Item = SocketAddr; - type Error = ResolveAddrError; + type Error = ErrBox; fn poll(&mut self) -> Poll { // The implementation of this is not actually async at the moment, // however we intend to use async DNS resolution in the future and // so we expose this as a future instead of Result. match split(&self.address) { - None => Err(ResolveAddrError::Syntax), + None => Err(deno_error::invalid_address_syntax()), Some(addr_port_pair) => { // I absolutely despise the .to_socket_addrs() API. - let r = addr_port_pair - .to_socket_addrs() - .map_err(ResolveAddrError::Resolution); + let r = addr_port_pair.to_socket_addrs().map_err(ErrBox::from); r.and_then(|mut iter| match iter.next() { Some(a) => Ok(Async::Ready(a)), diff --git a/cli/resources.rs b/cli/resources.rs index 13fd8f6fe933f4..9ef12a4293de65 100644 --- a/cli/resources.rs +++ b/cli/resources.rs @@ -10,13 +10,12 @@ use crate::deno_error; use crate::deno_error::bad_resource; -use crate::deno_error::DenoError; -use crate::deno_error::DenoResult; use crate::http_body::HttpBody; use crate::repl::Repl; use crate::state::WorkerChannels; use deno::Buf; +use deno::ErrBox; use futures; use futures::Future; @@ -216,14 +215,14 @@ impl Resource { } } - pub fn shutdown(&mut self, how: Shutdown) -> Result<(), DenoError> { + pub fn shutdown(&mut self, how: Shutdown) -> Result<(), ErrBox> { let mut table = RESOURCE_TABLE.lock().unwrap(); let maybe_repr = table.get_mut(&self.rid); match maybe_repr { None => panic!("bad rid"), Some(repr) => match repr { Repr::TcpStream(ref mut f) => { - TcpStream::shutdown(f, how).map_err(DenoError::from) + TcpStream::shutdown(f, how).map_err(ErrBox::from) } _ => panic!("Cannot shutdown"), }, @@ -366,15 +365,13 @@ pub struct WorkerReceiver { // Invert the dumbness that tokio_process causes by making Child itself a future. impl Future for WorkerReceiver { type Item = Option; - type Error = DenoError; + type Error = ErrBox; - fn poll(&mut self) -> Poll, DenoError> { + fn poll(&mut self) -> Poll, ErrBox> { let mut table = RESOURCE_TABLE.lock().unwrap(); let maybe_repr = table.get_mut(&self.rid); match maybe_repr { - Some(Repr::Worker(ref mut wc)) => wc.1.poll().map_err(|err| { - deno_error::new(deno_error::ErrorKind::Other, err.to_string()) - }), + Some(Repr::Worker(ref mut wc)) => wc.1.poll().map_err(ErrBox::from), _ => Err(bad_resource()), } } @@ -391,15 +388,13 @@ pub struct WorkerReceiverStream { // Invert the dumbness that tokio_process causes by making Child itself a future. impl Stream for WorkerReceiverStream { type Item = Buf; - type Error = DenoError; + type Error = ErrBox; - fn poll(&mut self) -> Poll, DenoError> { + fn poll(&mut self) -> Poll, ErrBox> { let mut table = RESOURCE_TABLE.lock().unwrap(); let maybe_repr = table.get_mut(&self.rid); match maybe_repr { - Some(Repr::Worker(ref mut wc)) => wc.1.poll().map_err(|err| { - deno_error::new(deno_error::ErrorKind::Other, err.to_string()) - }), + Some(Repr::Worker(ref mut wc)) => wc.1.poll().map_err(ErrBox::from), _ => Err(bad_resource()), } } @@ -462,19 +457,19 @@ pub struct ChildStatus { // Invert the dumbness that tokio_process causes by making Child itself a future. impl Future for ChildStatus { type Item = ExitStatus; - type Error = DenoError; + type Error = ErrBox; - fn poll(&mut self) -> Poll { + fn poll(&mut self) -> Poll { let mut table = RESOURCE_TABLE.lock().unwrap(); let maybe_repr = table.get_mut(&self.rid); match maybe_repr { - Some(Repr::Child(ref mut child)) => child.poll().map_err(DenoError::from), + Some(Repr::Child(ref mut child)) => child.poll().map_err(ErrBox::from), _ => Err(bad_resource()), } } } -pub fn child_status(rid: ResourceId) -> DenoResult { +pub fn child_status(rid: ResourceId) -> Result { let mut table = RESOURCE_TABLE.lock().unwrap(); let maybe_repr = table.get_mut(&rid); match maybe_repr { @@ -483,7 +478,7 @@ pub fn child_status(rid: ResourceId) -> DenoResult { } } -pub fn get_repl(rid: ResourceId) -> DenoResult>> { +pub fn get_repl(rid: ResourceId) -> Result>, ErrBox> { let mut table = RESOURCE_TABLE.lock().unwrap(); let maybe_repr = table.get_mut(&rid); match maybe_repr { @@ -494,7 +489,7 @@ pub fn get_repl(rid: ResourceId) -> DenoResult>> { // TODO: revamp this after the following lands: // https://github.com/tokio-rs/tokio/pull/785 -pub fn get_file(rid: ResourceId) -> DenoResult { +pub fn get_file(rid: ResourceId) -> Result { let mut table = RESOURCE_TABLE.lock().unwrap(); // We take ownership of File here. // It is put back below while still holding the lock. @@ -516,7 +511,7 @@ pub fn get_file(rid: ResourceId) -> DenoResult { table.insert(rid, Repr::FsFile(tokio_fs::File::from_std(std_file))); if maybe_std_file_copy.is_err() { - return Err(DenoError::from(maybe_std_file_copy.unwrap_err())); + return Err(ErrBox::from(maybe_std_file_copy.unwrap_err())); } let std_file_copy = maybe_std_file_copy.unwrap(); @@ -537,23 +532,25 @@ pub fn seek( resource: Resource, offset: i32, whence: u32, -) -> Box + Send> { +) -> Box + Send> { // Translate seek mode to Rust repr. let seek_from = match whence { 0 => SeekFrom::Start(offset as u64), 1 => SeekFrom::Current(i64::from(offset)), 2 => SeekFrom::End(i64::from(offset)), _ => { - return Box::new(futures::future::err(deno_error::new( - deno_error::ErrorKind::InvalidSeekMode, - format!("Invalid seek mode: {}", whence), - ))); + return Box::new(futures::future::err( + deno_error::DenoError::new( + deno_error::ErrorKind::InvalidSeekMode, + format!("Invalid seek mode: {}", whence), + ).into(), + )); } }; match get_file(resource.rid) { Ok(mut file) => Box::new(futures::future::lazy(move || { - let result = file.seek(seek_from).map(|_| {}).map_err(DenoError::from); + let result = file.seek(seek_from).map(|_| {}).map_err(ErrBox::from); futures::future::result(result) })), Err(err) => Box::new(futures::future::err(err)), diff --git a/cli/shell.rs b/cli/shell.rs index 4dc388f39ebe13..9a66efe715099b 100644 --- a/cli/shell.rs +++ b/cli/shell.rs @@ -10,11 +10,10 @@ use std::fmt; use std::io::prelude::*; use atty; +use deno::ErrBox; use termcolor::Color::{Cyan, Green, Red, Yellow}; use termcolor::{self, Color, ColorSpec, StandardStream, WriteColor}; -use crate::deno_error::DenoResult; - /// The requested verbosity of output. #[derive(Debug, Clone, Copy, PartialEq)] pub enum Verbosity { @@ -116,7 +115,7 @@ impl Shell { message: Option<&dyn fmt::Display>, color: Color, justified: bool, - ) -> DenoResult<()> { + ) -> Result<(), ErrBox> { match self.verbosity { Verbosity::Quiet => Ok(()), _ => { @@ -171,7 +170,7 @@ impl Shell { } /// Shortcut to right-align and color green a status message. - pub fn status(&mut self, status: T, message: U) -> DenoResult<()> + pub fn status(&mut self, status: T, message: U) -> Result<(), ErrBox> where T: fmt::Display, U: fmt::Display, @@ -179,7 +178,7 @@ impl Shell { self.print(&status, Some(&message), Green, false) } - pub fn status_header(&mut self, status: T) -> DenoResult<()> + pub fn status_header(&mut self, status: T) -> Result<(), ErrBox> where T: fmt::Display, { @@ -192,7 +191,7 @@ impl Shell { status: T, message: U, color: Color, - ) -> DenoResult<()> + ) -> Result<(), ErrBox> where T: fmt::Display, U: fmt::Display, @@ -201,9 +200,9 @@ impl Shell { } /// Runs the callback only if we are in verbose mode. - pub fn verbose(&mut self, mut callback: F) -> DenoResult<()> + pub fn verbose(&mut self, mut callback: F) -> Result<(), ErrBox> where - F: FnMut(&mut Shell) -> DenoResult<()>, + F: FnMut(&mut Shell) -> Result<(), ErrBox>, { match self.verbosity { Verbosity::Verbose => callback(self), @@ -212,9 +211,9 @@ impl Shell { } /// Runs the callback if we are not in verbose mode. - pub fn concise(&mut self, mut callback: F) -> DenoResult<()> + pub fn concise(&mut self, mut callback: F) -> Result<(), ErrBox> where - F: FnMut(&mut Shell) -> DenoResult<()>, + F: FnMut(&mut Shell) -> Result<(), ErrBox>, { match self.verbosity { Verbosity::Verbose => Ok(()), @@ -223,12 +222,12 @@ impl Shell { } /// Prints a red 'error' message. - pub fn error(&mut self, message: T) -> DenoResult<()> { + pub fn error(&mut self, message: T) -> Result<(), ErrBox> { self.print(&"error:", Some(&message), Red, false) } /// Prints an amber 'warning' message. - pub fn warn(&mut self, message: T) -> DenoResult<()> { + pub fn warn(&mut self, message: T) -> Result<(), ErrBox> { match self.verbosity { Verbosity::Quiet => Ok(()), _ => self.print(&"warning:", Some(&message), Yellow, false), @@ -246,7 +245,10 @@ impl Shell { } /// Updates the color choice (always, never, or auto) from a string.. - pub fn set_color_choice(&mut self, color: Option<&str>) -> DenoResult<()> { + pub fn set_color_choice( + &mut self, + color: Option<&str>, + ) -> Result<(), ErrBox> { if let ShellOut::Stream { ref mut stream, ref mut color_choice, @@ -291,7 +293,7 @@ impl Shell { } /// Prints a message and translates ANSI escape code into console colors. - pub fn print_ansi(&mut self, message: &[u8]) -> DenoResult<()> { + pub fn print_ansi(&mut self, message: &[u8]) -> Result<(), ErrBox> { if self.needs_clear { self.err_erase_line(); } @@ -323,7 +325,7 @@ impl ShellOut { message: Option<&dyn fmt::Display>, color: Color, justified: bool, - ) -> DenoResult<()> { + ) -> Result<(), ErrBox> { match *self { ShellOut::Stream { ref mut stream, .. } => { stream.reset()?; diff --git a/cli/signal.rs b/cli/signal.rs index bedb43ad25005a..22aa90be3b641d 100644 --- a/cli/signal.rs +++ b/cli/signal.rs @@ -1,19 +1,15 @@ -#[cfg(unix)] -use nix::sys::signal::{kill as unix_kill, Signal}; -#[cfg(unix)] -use nix::unistd::Pid; - -use crate::deno_error::DenoResult; +use deno::ErrBox; #[cfg(unix)] -pub fn kill(pid: i32, signo: i32) -> DenoResult<()> { - use crate::deno_error::DenoError; +pub fn kill(pid: i32, signo: i32) -> Result<(), ErrBox> { + use nix::sys::signal::{kill as unix_kill, Signal}; + use nix::unistd::Pid; let sig = Signal::from_c_int(signo)?; - unix_kill(Pid::from_raw(pid), Option::Some(sig)).map_err(DenoError::from) + unix_kill(Pid::from_raw(pid), Option::Some(sig)).map_err(ErrBox::from) } #[cfg(not(unix))] -pub fn kill(_pid: i32, _signal: i32) -> DenoResult<()> { +pub fn kill(_pid: i32, _signal: i32) -> Result<(), ErrBox> { // NOOP // TODO: implement this for windows Ok(()) diff --git a/cli/source_maps.rs b/cli/source_maps.rs index 11c3a35325967b..6b453d883cd8d9 100644 --- a/cli/source_maps.rs +++ b/cli/source_maps.rs @@ -1,7 +1,7 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. -//! This mod provides functions to remap a deno::JSError based on a source map -use deno::JSError; +//! This mod provides functions to remap a deno::V8Exception based on a source map use deno::StackFrame; +use deno::V8Exception; use serde_json; use source_map_mappings::parse_mappings; use source_map_mappings::Bias; @@ -90,36 +90,36 @@ fn builtin_source_map(script_name: &str) -> Option> { } } -/// Apply a source map to a JSError, returning a JSError where the filenames, +/// Apply a source map to a V8Exception, returning a V8Exception where the filenames, /// the lines and the columns point to their original source location, not their /// transpiled location if applicable. pub fn apply_source_map( - js_error: &JSError, + v8_exception: &V8Exception, getter: &G, -) -> JSError { +) -> V8Exception { let mut mappings_map: CachedMaps = HashMap::new(); let mut frames = Vec::::new(); - for frame in &js_error.frames { + for frame in &v8_exception.frames { let f = frame_apply_source_map(&frame, &mut mappings_map, getter); frames.push(f); } let (script_resource_name, line_number, start_column) = get_maybe_orig_position( - js_error.script_resource_name.clone(), - js_error.line_number, - js_error.start_column, + v8_exception.script_resource_name.clone(), + v8_exception.line_number, + v8_exception.start_column, &mut mappings_map, getter, ); // It is better to just move end_column to be the same distance away from // start column because sometimes the code point is not available in the // source file map. - let end_column = match js_error.end_column { + let end_column = match v8_exception.end_column { Some(ec) => { if start_column.is_some() { - Some(ec - (js_error.start_column.unwrap() - start_column.unwrap())) + Some(ec - (v8_exception.start_column.unwrap() - start_column.unwrap())) } else { None } @@ -128,22 +128,22 @@ pub fn apply_source_map( }; // if there is a source line that we might be different in the source file, we // will go fetch it from the getter - let source_line = if js_error.source_line.is_some() + let source_line = if v8_exception.source_line.is_some() && script_resource_name.is_some() && line_number.is_some() { getter.get_source_line( - &js_error.script_resource_name.clone().unwrap(), + &v8_exception.script_resource_name.clone().unwrap(), line_number.unwrap() as usize, ) } else { - js_error.source_line.clone() + v8_exception.source_line.clone() }; - JSError { - message: js_error.message.clone(), + V8Exception { + message: v8_exception.message.clone(), frames, - error_level: js_error.error_level, + error_level: v8_exception.error_level, source_line, script_resource_name, line_number, @@ -151,8 +151,8 @@ pub fn apply_source_map( end_column, // These are difficult to map to their original position and they are not // currently used in any output, so we don't remap them. - start_position: js_error.start_position, - end_position: js_error.end_position, + start_position: v8_exception.start_position, + end_position: v8_exception.end_position, } } @@ -297,8 +297,8 @@ mod tests { } } - fn error1() -> JSError { - JSError { + fn error1() -> V8Exception { + V8Exception { message: "Error: foo bar".to_string(), source_line: None, script_resource_name: None, @@ -341,11 +341,11 @@ mod tests { } #[test] - fn js_error_apply_source_map_1() { + fn v8_exception_apply_source_map_1() { let e = error1(); let getter = MockSourceMapGetter {}; let actual = apply_source_map(&e, &getter); - let expected = JSError { + let expected = V8Exception { message: "Error: foo bar".to_string(), source_line: None, script_resource_name: None, @@ -389,8 +389,8 @@ mod tests { } #[test] - fn js_error_apply_source_map_2() { - let e = JSError { + fn v8_exception_apply_source_map_2() { + let e = V8Exception { message: "TypeError: baz".to_string(), source_line: None, script_resource_name: None, @@ -419,8 +419,8 @@ mod tests { } #[test] - fn js_error_apply_source_map_line() { - let e = JSError { + fn v8_exception_apply_source_map_line() { + let e = V8Exception { message: "TypeError: baz".to_string(), source_line: Some("foo".to_string()), script_resource_name: Some("foo_bar.ts".to_string()), diff --git a/cli/state.rs b/cli/state.rs index 9f6c5996922b66..8d3d116d95906d 100644 --- a/cli/state.rs +++ b/cli/state.rs @@ -2,8 +2,6 @@ use crate::compiler::compile_async; use crate::compiler::ModuleMetaData; use crate::deno_dir; -use crate::deno_error::DenoError; -use crate::deno_error::DenoResult; use crate::flags; use crate::global_timer::GlobalTimer; use crate::import_map::ImportMap; @@ -16,6 +14,7 @@ use crate::resources::ResourceId; use crate::worker::Worker; use deno::Buf; use deno::CoreOp; +use deno::ErrBox; use deno::Loader; use deno::ModuleSpecifier; use deno::PinnedBuf; @@ -118,7 +117,7 @@ impl ThreadSafeState { pub fn fetch_module_meta_data_and_maybe_compile_async( state: &ThreadSafeState, module_specifier: &ModuleSpecifier, -) -> impl Future { +) -> impl Future { let state_ = state.clone(); let use_cache = !state_.flags.reload || state_.has_compiled(&module_specifier.to_string()); @@ -150,14 +149,12 @@ pub fn fetch_module_meta_data_and_maybe_compile_async( } impl Loader for ThreadSafeState { - type Error = DenoError; - fn resolve( &self, specifier: &str, referrer: &str, is_root: bool, - ) -> Result { + ) -> Result { if !is_root { if let Some(import_map) = &self.import_map { let result = import_map.resolve(specifier, referrer)?; @@ -167,15 +164,14 @@ impl Loader for ThreadSafeState { } } - ModuleSpecifier::resolve_import(specifier, referrer) - .map_err(DenoError::from) + ModuleSpecifier::resolve_import(specifier, referrer).map_err(ErrBox::from) } /// Given an absolute url, load its source code. fn load( &self, module_specifier: &ModuleSpecifier, - ) -> Box> { + ) -> Box { self.metrics.resolve_count.fetch_add(1, Ordering::SeqCst); Box::new( fetch_module_meta_data_and_maybe_compile_async(self, module_specifier) @@ -324,32 +320,32 @@ impl ThreadSafeState { } #[inline] - pub fn check_read(&self, filename: &str) -> DenoResult<()> { + pub fn check_read(&self, filename: &str) -> Result<(), ErrBox> { self.permissions.check_read(filename) } #[inline] - pub fn check_write(&self, filename: &str) -> DenoResult<()> { + pub fn check_write(&self, filename: &str) -> Result<(), ErrBox> { self.permissions.check_write(filename) } #[inline] - pub fn check_env(&self) -> DenoResult<()> { + pub fn check_env(&self) -> Result<(), ErrBox> { self.permissions.check_env() } #[inline] - pub fn check_net(&self, host_and_port: &str) -> DenoResult<()> { + pub fn check_net(&self, host_and_port: &str) -> Result<(), ErrBox> { self.permissions.check_net(host_and_port) } #[inline] - pub fn check_net_url(&self, url: url::Url) -> DenoResult<()> { + pub fn check_net_url(&self, url: url::Url) -> Result<(), ErrBox> { self.permissions.check_net_url(url) } #[inline] - pub fn check_run(&self) -> DenoResult<()> { + pub fn check_run(&self) -> Result<(), ErrBox> { self.permissions.check_run() } diff --git a/cli/worker.rs b/cli/worker.rs index 1cf38e295102e8..c41184c6936c91 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -1,8 +1,9 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. -use crate::deno_error::DenoError; +use crate::fmt_errors::JSError; use crate::state::ThreadSafeState; use crate::tokio_util; use deno; +use deno::ErrBox; use deno::ModuleSpecifier; use deno::StartupData; use futures::Async; @@ -24,19 +25,23 @@ impl Worker { startup_data: StartupData, state: ThreadSafeState, ) -> Worker { - let state_ = state.clone(); let isolate = Arc::new(Mutex::new(deno::Isolate::new(startup_data, false))); { let mut i = isolate.lock().unwrap(); + let state_ = state.clone(); i.set_dispatch(move |control_buf, zero_copy_buf| { state_.dispatch(control_buf, zero_copy_buf) }); + let state_ = state.clone(); + i.set_js_error_create(move |v8_exception| { + JSError::from_v8_exception(v8_exception, &state_.dir) + }) } Self { isolate, state } } /// Same as execute2() but the filename defaults to "". - pub fn execute(&mut self, js_source: &str) -> Result<(), DenoError> { + pub fn execute(&mut self, js_source: &str) -> Result<(), ErrBox> { self.execute2("", js_source) } @@ -46,12 +51,9 @@ impl Worker { &mut self, js_filename: &str, js_source: &str, - ) -> Result<(), DenoError> { + ) -> Result<(), ErrBox> { let mut isolate = self.isolate.lock().unwrap(); - match isolate.execute(js_filename, js_source) { - Ok(_) => Ok(()), - Err(err) => Err(DenoError::from(err)), - } + isolate.execute(js_filename, js_source) } /// Executes the provided JavaScript module. @@ -59,9 +61,8 @@ impl Worker { &mut self, module_specifier: &ModuleSpecifier, is_prefetch: bool, - ) -> impl Future { + ) -> impl Future { let worker = self.clone(); - let worker_ = worker.clone(); let loader = self.state.clone(); let isolate = self.isolate.clone(); let modules = self.state.modules.clone(); @@ -71,30 +72,15 @@ impl Worker { isolate, modules, ); - recursive_load - .and_then(move |id| -> Result<(), deno::JSErrorOr> { - worker.state.progress.done(); - if is_prefetch { - Ok(()) - } else { - let mut isolate = worker.isolate.lock().unwrap(); - let result = isolate.mod_evaluate(id); - if let Err(err) = result { - Err(deno::JSErrorOr::JSError(err)) - } else { - Ok(()) - } - } - }).map_err(move |err| { - worker_.state.progress.done(); - // Convert to DenoError AND apply_source_map. - match err { - deno::JSErrorOr::JSError(err) => { - worker_.apply_source_map(DenoError::from(err)) - } - deno::JSErrorOr::Other(err) => err, - } - }) + recursive_load.and_then(move |id| -> Result<(), ErrBox> { + worker.state.progress.done(); + if is_prefetch { + Ok(()) + } else { + let mut isolate = worker.isolate.lock().unwrap(); + isolate.mod_evaluate(id) + } + }) } /// Executes the provided JavaScript module. @@ -102,32 +88,24 @@ impl Worker { &mut self, module_specifier: &ModuleSpecifier, is_prefetch: bool, - ) -> Result<(), DenoError> { + ) -> Result<(), ErrBox> { tokio_util::block_on(self.execute_mod_async(module_specifier, is_prefetch)) } - - /// Applies source map to the error. - fn apply_source_map(&self, err: DenoError) -> DenoError { - err.apply_source_map(&self.state.dir) - } } impl Future for Worker { type Item = (); - type Error = DenoError; + type Error = ErrBox; - fn poll(&mut self) -> Result, Self::Error> { + fn poll(&mut self) -> Result, ErrBox> { let mut isolate = self.isolate.lock().unwrap(); - isolate - .poll() - .map_err(|err| self.apply_source_map(DenoError::from(err))) + isolate.poll() } } #[cfg(test)] mod tests { use super::*; - use crate::deno_error::err_check; use crate::flags; use crate::ops::op_selector_std; use crate::progress::Progress; @@ -210,7 +188,7 @@ mod tests { startup_data::deno_isolate_init(), state, ); - err_check(worker.execute("denoMain()")); + worker.execute("denoMain()").unwrap(); let result = worker.execute_mod(&module_specifier, false); if let Err(err) = result { eprintln!("execute_mod err {:?}", err); @@ -231,8 +209,8 @@ mod tests { ]); let mut worker = Worker::new("TEST".to_string(), startup_data::deno_isolate_init(), state); - err_check(worker.execute("denoMain()")); - err_check(worker.execute("workerMain()")); + worker.execute("denoMain()").unwrap(); + worker.execute("workerMain()").unwrap(); worker } @@ -253,7 +231,7 @@ mod tests { console.log("after postMessage"); } "#; - err_check(worker.execute(source)); + worker.execute(source).unwrap(); let resource = worker.state.resource.clone(); let resource_ = resource.clone(); @@ -261,7 +239,7 @@ mod tests { tokio::spawn(lazy(move || { worker.then(move |r| -> Result<(), ()> { resource_.close(); - err_check(r); + r.unwrap(); Ok(()) }) })); @@ -291,9 +269,9 @@ mod tests { fn removed_from_resource_table_on_close() { tokio_util::init(|| { let mut worker = create_test_worker(); - err_check( - worker.execute("onmessage = () => { delete window.onmessage; }"), - ); + worker + .execute("onmessage = () => { delete window.onmessage; }") + .unwrap(); let resource = worker.state.resource.clone(); let rid = resource.rid; @@ -302,7 +280,7 @@ mod tests { .then(move |r| -> Result<(), ()> { resource.close(); println!("workers.rs after resource close"); - err_check(r); + r.unwrap(); Ok(()) }).shared(); diff --git a/core/any_error.rs b/core/any_error.rs new file mode 100644 index 00000000000000..001ca54eba53ab --- /dev/null +++ b/core/any_error.rs @@ -0,0 +1,68 @@ +use std::any::{Any, TypeId}; +use std::convert::From; +use std::error::Error; +use std::fmt; +use std::ops::Deref; + +// The Send and Sync traits are required because deno is multithreaded and we +// need to beable to handle errors across threads. +pub trait AnyError: Any + Error + Send + Sync + 'static {} +impl AnyError for T where T: Any + Error + Send + Sync + Sized + 'static {} + +#[derive(Debug)] +pub struct ErrBox(Box); + +impl dyn AnyError { + pub fn downcast_ref(&self) -> Option<&T> { + if Any::type_id(self) == TypeId::of::() { + let target = self as *const Self as *const T; + let target = unsafe { &*target }; + Some(target) + } else { + None + } + } +} + +impl ErrBox { + pub fn downcast(self) -> Result { + if Any::type_id(&*self.0) == TypeId::of::() { + let target = Box::into_raw(self.0) as *mut T; + let target = unsafe { Box::from_raw(target) }; + Ok(*target) + } else { + Err(self) + } + } +} + +impl AsRef for ErrBox { + fn as_ref(&self) -> &dyn AnyError { + self.0.as_ref() + } +} + +impl Deref for ErrBox { + type Target = Box; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl From for ErrBox { + fn from(error: T) -> Self { + Self(Box::new(error)) + } +} + +impl From> for ErrBox { + fn from(boxed: Box) -> Self { + Self(boxed) + } +} + +impl fmt::Display for ErrBox { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.0.fmt(f) + } +} diff --git a/core/examples/http_bench.rs b/core/examples/http_bench.rs index ee8de50864f2c9..98f11bc4d1557c 100644 --- a/core/examples/http_bench.rs +++ b/core/examples/http_bench.rs @@ -310,7 +310,7 @@ fn op_write(rid: i32, zero_copy_buf: Option) -> Box { ) } -fn js_check(r: Result<(), JSError>) { +fn js_check(r: Result<(), ErrBox>) { if let Err(e) = r { panic!(e.to_string()); } diff --git a/core/isolate.rs b/core/isolate.rs index 2e6406e56fcd90..38f7b798797ef7 100644 --- a/core/isolate.rs +++ b/core/isolate.rs @@ -4,7 +4,9 @@ // isolate to keep the Isolate struct from becoming too bloating for users who // do not need asynchronous module loading. -use crate::js_errors::JSError; +use crate::any_error::ErrBox; +use crate::js_errors::CoreJSError; +use crate::js_errors::V8Exception; use crate::libdeno; use crate::libdeno::deno_buf; use crate::libdeno::deno_dyn_import_id; @@ -81,6 +83,8 @@ type CoreDispatchFn = dyn Fn(&[u8], Option) -> CoreOp; pub type DynImportFuture = Box + Send>; type DynImportFn = dyn Fn(&str, &str) -> DynImportFuture; +type JSErrorCreateFn = dyn Fn(V8Exception) -> ErrBox; + /// Wraps DynImportFuture to include the deno_dyn_import_id, so that it doesn't /// need to be exposed. struct DynImport { @@ -114,6 +118,7 @@ pub struct Isolate { shared_libdeno_isolate: Arc>>, dispatch: Option>, dyn_import: Option>, + js_error_create: Arc, needs_init: bool, shared: SharedQueue, pending_ops: FuturesUnordered, @@ -178,6 +183,7 @@ impl Isolate { shared_libdeno_isolate: Arc::new(Mutex::new(Some(libdeno_isolate))), dispatch: None, dyn_import: None, + js_error_create: Arc::new(CoreJSError::from_v8_exception), shared, needs_init, pending_ops: FuturesUnordered::new(), @@ -204,6 +210,16 @@ impl Isolate { self.dyn_import = Some(Arc::new(f)); } + /// Allows a callback to be set whenever a V8 exception is made. This allows + /// the caller to wrap the V8Exception into an error. By default this callback + /// is set to CoreJSError::from_v8_exception. + pub fn set_js_error_create(&mut self, f: F) + where + F: Fn(V8Exception) -> ErrBox + 'static, + { + self.js_error_create = Arc::new(f); + } + /// Get a thread safe handle on the isolate. pub fn shared_isolate_handle(&mut self) -> IsolateHandle { IsolateHandle { @@ -307,11 +323,16 @@ impl Isolate { self as *const _ as *const c_void } + /// Executes traditional JavaScript code (traditional = not ES modules) + /// + /// ErrBox can be downcast to a type that exposes additional information about + /// the V8 exception. By default this type is CoreJSError, however it may be a + /// different type if Isolate::set_js_error_create() has been used. pub fn execute( &mut self, js_filename: &str, js_source: &str, - ) -> Result<(), JSError> { + ) -> Result<(), ErrBox> { self.shared_init(); let filename = CString::new(js_filename).unwrap(); let source = CString::new(js_source).unwrap(); @@ -323,22 +344,20 @@ impl Isolate { source.as_ptr(), ) }; - if let Some(err) = self.last_exception() { - return Err(err); - } - Ok(()) + self.check_last_exception() } - fn last_exception(&self) -> Option { + fn check_last_exception(&self) -> Result<(), ErrBox> { let ptr = unsafe { libdeno::deno_last_exception(self.libdeno_isolate) }; if ptr.is_null() { - None + Ok(()) } else { + let js_error_create = &*self.js_error_create; let cstr = unsafe { CStr::from_ptr(ptr) }; - let v8_exception = cstr.to_str().unwrap(); - debug!("v8_exception\n{}\n", v8_exception); - let js_error = JSError::from_v8_exception(v8_exception).unwrap(); - Some(js_error) + let json_str = cstr.to_str().unwrap(); + let v8_exception = V8Exception::from_json(json_str).unwrap(); + let js_error = js_error_create(v8_exception); + Err(js_error) } } @@ -348,7 +367,7 @@ impl Isolate { } } - fn respond(&mut self, maybe_buf: Option<&[u8]>) -> Result<(), JSError> { + fn respond(&mut self, maybe_buf: Option<&[u8]>) -> Result<(), ErrBox> { let buf = match maybe_buf { None => deno_buf::empty(), Some(r) => deno_buf::from(r), @@ -356,11 +375,7 @@ impl Isolate { unsafe { libdeno::deno_respond(self.libdeno_isolate, self.as_raw_ptr(), buf) } - if let Some(err) = self.last_exception() { - Err(err) - } else { - Ok(()) - } + self.check_last_exception() } /// Low-level module creation. @@ -369,7 +384,7 @@ impl Isolate { main: bool, name: &str, source: &str, - ) -> Result { + ) -> Result { let name_ = CString::new(name.to_string()).unwrap(); let name_ptr = name_.as_ptr() as *const libc::c_char; @@ -379,12 +394,11 @@ impl Isolate { let id = unsafe { libdeno::deno_mod_new(self.libdeno_isolate, main, name_ptr, source_ptr) }; - if let Some(js_error) = self.last_exception() { - assert_eq!(id, 0); - return Err(js_error); - } - Ok(id) + self.check_last_exception().map(|_| id).map_err(|err| { + assert_eq!(id, 0); + err + }) } pub fn mod_get_imports(&self, id: deno_mod) -> Vec { @@ -402,23 +416,29 @@ impl Isolate { out } - pub fn snapshot(&self) -> Result, JSError> { + /// Takes a snapshot. The isolate should have been created with will_snapshot + /// set to true. + /// + /// ErrBox can be downcast to a type that exposes additional information about + /// the V8 exception. By default this type is CoreJSError, however it may be a + /// different type if Isolate::set_js_error_create() has been used. + pub fn snapshot(&self) -> Result, ErrBox> { let snapshot = unsafe { libdeno::deno_snapshot_new(self.libdeno_isolate) }; - if let Some(js_error) = self.last_exception() { - assert_eq!(snapshot.data_ptr, null()); - assert_eq!(snapshot.data_len, 0); - return Err(js_error); + match self.check_last_exception() { + Ok(..) => Ok(snapshot), + Err(err) => { + assert_eq!(snapshot.data_ptr, null()); + assert_eq!(snapshot.data_len, 0); + Err(err) + } } - assert_ne!(snapshot.data_ptr, null()); - assert_ne!(snapshot.data_len, 0); - Ok(snapshot) } fn dyn_import_done( &self, id: libdeno::deno_dyn_import_id, mod_id: deno_mod, - ) -> Result<(), JSError> { + ) -> Result<(), ErrBox> { debug!("dyn_import_done {} {}", id, mod_id); unsafe { libdeno::deno_dyn_import( @@ -428,11 +448,10 @@ impl Isolate { mod_id, ) }; - if let Some(js_error) = self.last_exception() { + self.check_last_exception().map_err(|err| { assert_eq!(id, 0); - return Err(js_error); - } - Ok(()) + err + }) } } @@ -458,11 +477,16 @@ impl<'a> ResolveContext<'a> { } impl Isolate { + /// Instanciates a ES module + /// + /// ErrBox can be downcast to a type that exposes additional information about + /// the V8 exception. By default this type is CoreJSError, however it may be a + /// different type if Isolate::set_js_error_create() has been used. pub fn mod_instantiate( &mut self, id: deno_mod, resolve_fn: &mut ResolveFn, - ) -> Result<(), JSError> { + ) -> Result<(), ErrBox> { let libdeno_isolate = self.libdeno_isolate; let mut ctx = ResolveContext { resolve_fn }; unsafe { @@ -473,11 +497,7 @@ impl Isolate { Self::resolve_cb, ) }; - - if let Some(js_error) = self.last_exception() { - return Err(js_error); - } - Ok(()) + self.check_last_exception() } /// Called during mod_instantiate() only. @@ -494,15 +514,17 @@ impl Isolate { resolve_fn(specifier, referrer) } - pub fn mod_evaluate(&mut self, id: deno_mod) -> Result<(), JSError> { + /// Evaluates an already instantiated ES module. + /// + /// ErrBox can be downcast to a type that exposes additional information about + /// the V8 exception. By default this type is CoreJSError, however it may be a + /// different type if Isolate::set_js_error_create() has been used. + pub fn mod_evaluate(&mut self, id: deno_mod) -> Result<(), ErrBox> { self.shared_init(); unsafe { libdeno::deno_mod_evaluate(self.libdeno_isolate, self.as_raw_ptr(), id) }; - if let Some(js_error) = self.last_exception() { - return Err(js_error); - } - Ok(()) + self.check_last_exception() } } @@ -525,9 +547,9 @@ impl Drop for LockerScope { impl Future for Isolate { type Item = (); - type Error = JSError; + type Error = ErrBox; - fn poll(&mut self) -> Poll<(), JSError> { + fn poll(&mut self) -> Poll<(), ErrBox> { // Lock the current thread for V8. let _locker = LockerScope::new(self.libdeno_isolate); @@ -579,9 +601,7 @@ impl Future for Isolate { } self.check_promise_errors(); - if let Some(err) = self.last_exception() { - return Err(err); - } + self.check_last_exception()?; // We're idle if pending_ops is empty. if self.pending_ops.is_empty() { @@ -616,7 +636,7 @@ impl IsolateHandle { } } -pub fn js_check(r: Result<(), JSError>) { +pub fn js_check(r: Result<(), ErrBox>) { if let Err(e) = r { panic!(e.to_string()); } @@ -814,7 +834,7 @@ pub mod tests { "#, )); assert_eq!(dispatch_count.load(Ordering::Relaxed), 1); - assert_eq!(Ok(Async::Ready(())), isolate.poll()); + assert_eq!(Async::Ready(()), isolate.poll().unwrap()); assert_eq!(dispatch_count.load(Ordering::Relaxed), 1); js_check(isolate.execute( "check2.js", @@ -825,11 +845,11 @@ pub mod tests { "#, )); assert_eq!(dispatch_count.load(Ordering::Relaxed), 2); - assert_eq!(Ok(Async::Ready(())), isolate.poll()); + assert_eq!(Async::Ready(()), isolate.poll().unwrap()); js_check(isolate.execute("check3.js", "assert(nrecv == 2)")); assert_eq!(dispatch_count.load(Ordering::Relaxed), 2); // We are idle, so the next poll should be the last. - assert_eq!(Ok(Async::Ready(())), isolate.poll()); + assert_eq!(Async::Ready(()), isolate.poll().unwrap()); }); } @@ -865,7 +885,7 @@ pub mod tests { "#, )); assert_eq!(dispatch_count.load(Ordering::Relaxed), 2); - assert_eq!(Ok(Async::Ready(())), isolate.poll()); + assert_eq!(Async::Ready(()), isolate.poll().unwrap()); js_check(isolate.execute("send1.js", "assert(nrecv === 2);")); }); } @@ -949,9 +969,9 @@ pub mod tests { )); assert_eq!(count.load(Ordering::Relaxed), 1); - assert_eq!(Ok(Ready(())), isolate.poll()); + assert_eq!(Ready(()), isolate.poll().unwrap()); assert_eq!(count.load(Ordering::Relaxed), 2); - assert_eq!(Ok(Ready(())), isolate.poll()); + assert_eq!(Ready(()), isolate.poll().unwrap()); assert_eq!(count.load(Ordering::Relaxed), 2); }) } @@ -1090,7 +1110,7 @@ pub mod tests { "#, )); assert_eq!(dispatch_count.load(Ordering::Relaxed), 1); - assert_eq!(Ok(Async::Ready(())), isolate.poll()); + assert_eq!(Async::Ready(()), isolate.poll().unwrap()); js_check(isolate.execute("check.js", "assert(asyncRecv == 1);")); }); } @@ -1118,7 +1138,7 @@ pub mod tests { "#, )); assert_eq!(dispatch_count.load(Ordering::Relaxed), 1); - assert_eq!(Ok(()), poll_until_ready(&mut isolate, 3)); + poll_until_ready(&mut isolate, 3).unwrap(); js_check(isolate.execute("check.js", "assert(asyncRecv == 1);")); }); } @@ -1149,7 +1169,7 @@ pub mod tests { "#, )); assert_eq!(dispatch_count.load(Ordering::Relaxed), 2); - assert_eq!(Ok(()), poll_until_ready(&mut isolate, 3)); + poll_until_ready(&mut isolate, 3).unwrap(); js_check(isolate.execute("check.js", "assert(asyncRecv == 2);")); }); } @@ -1164,7 +1184,7 @@ pub mod tests { include_str!("shared_queue_test.js"), ), ); - assert_eq!(Ok(Async::Ready(())), isolate.poll()); + assert_eq!(Async::Ready(()), isolate.poll().unwrap()); }); } diff --git a/core/js_errors.rs b/core/js_errors.rs index ca5cf80850682d..08cb00a7289b0e 100644 --- a/core/js_errors.rs +++ b/core/js_errors.rs @@ -9,8 +9,10 @@ // console.log(err.stack); // It would require calling into Rust from Error.prototype.prepareStackTrace. +use crate::any_error::ErrBox; use serde_json; use serde_json::value::Value; +use std::error::Error; use std::fmt; use std::str; @@ -26,7 +28,7 @@ pub struct StackFrame { } #[derive(Debug, PartialEq, Clone)] -pub struct JSError { +pub struct V8Exception { pub message: String, pub source_line: Option, @@ -41,77 +43,8 @@ pub struct JSError { pub frames: Vec, } -impl std::error::Error for JSError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - None - } -} - -impl fmt::Display for StackFrame { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - // Note when we print to string, we change from 0-indexed to 1-indexed. - let function_name = self.function_name.clone(); - let script_line_column = - format_script_line_column(&self.script_name, self.line, self.column); - - if !self.function_name.is_empty() { - write!(f, " at {} ({})", function_name, script_line_column) - } else if self.is_eval { - write!(f, " at eval ({})", script_line_column) - } else { - write!(f, " at {}", script_line_column) - } - } -} - -fn format_script_line_column( - script_name: &str, - line: i64, - column: i64, -) -> String { - // TODO match this style with how typescript displays errors. - let line = (1 + line).to_string(); - let column = (1 + column).to_string(); - let script_name = script_name.to_string(); - format!("{}:{}:{}", script_name, line, column) -} - -impl fmt::Display for JSError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if self.script_resource_name.is_some() { - let script_resource_name = self.script_resource_name.as_ref().unwrap(); - if self.line_number.is_some() && self.start_column.is_some() { - assert!(self.line_number.is_some()); - assert!(self.start_column.is_some()); - let script_line_column = format_script_line_column( - script_resource_name, - self.line_number.unwrap() - 1, - self.start_column.unwrap() - 1, - ); - write!(f, "{}", script_line_column)?; - } - if self.source_line.is_some() { - write!(f, "\n{}\n", self.source_line.as_ref().unwrap())?; - let mut s = String::new(); - for i in 0..self.end_column.unwrap() { - if i >= self.start_column.unwrap() { - s.push('^'); - } else { - s.push(' '); - } - } - writeln!(f, "{}", s)?; - } - } - - write!(f, "{}", self.message.clone())?; - - for frame in &self.frames { - write!(f, "\n{}", &frame.to_string())?; - } - Ok(()) - } -} +#[derive(Debug, PartialEq, Clone)] +pub struct CoreJSError(V8Exception); impl StackFrame { // TODO Maybe use serde_derive? @@ -186,9 +119,9 @@ impl StackFrame { } } -impl JSError { - /// Creates a new JSError by parsing the raw exception JSON string from V8. - pub fn from_v8_exception(json_str: &str) -> Option { +impl V8Exception { + /// Creates a new V8Exception by parsing the raw exception JSON string from V8. + pub fn from_json(json_str: &str) -> Option { let v = serde_json::from_str::(json_str); if v.is_err() { return None; @@ -236,7 +169,7 @@ impl JSError { } } - Some(JSError { + Some(V8Exception { message, source_line, script_resource_name, @@ -251,12 +184,79 @@ impl JSError { } } +impl CoreJSError { + pub fn from_v8_exception(v8_exception: V8Exception) -> ErrBox { + let error = Self(v8_exception); + ErrBox::from(error) + } +} + +fn format_source_loc(script_name: &str, line: i64, column: i64) -> String { + // TODO match this style with how typescript displays errors. + let line = line + 1; + let column = column + 1; + format!("{}:{}:{}", script_name, line, column) +} + +fn format_stack_frame(frame: &StackFrame) -> String { + // Note when we print to string, we change from 0-indexed to 1-indexed. + let source_loc = + format_source_loc(&frame.script_name, frame.line, frame.column); + + if !frame.function_name.is_empty() { + format!(" at {} ({})", frame.function_name, source_loc) + } else if frame.is_eval { + format!(" at eval ({})", source_loc) + } else { + format!(" at {}", source_loc) + } +} + +impl fmt::Display for CoreJSError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + if self.0.script_resource_name.is_some() { + let script_resource_name = self.0.script_resource_name.as_ref().unwrap(); + if self.0.line_number.is_some() && self.0.start_column.is_some() { + assert!(self.0.line_number.is_some()); + assert!(self.0.start_column.is_some()); + let source_loc = format_source_loc( + script_resource_name, + self.0.line_number.unwrap() - 1, + self.0.start_column.unwrap() - 1, + ); + write!(f, "{}", source_loc)?; + } + if self.0.source_line.is_some() { + write!(f, "\n{}\n", self.0.source_line.as_ref().unwrap())?; + let mut s = String::new(); + for i in 0..self.0.end_column.unwrap() { + if i >= self.0.start_column.unwrap() { + s.push('^'); + } else { + s.push(' '); + } + } + writeln!(f, "{}", s)?; + } + } + + write!(f, "{}", self.0.message)?; + + for frame in &self.0.frames { + write!(f, "\n{}", format_stack_frame(frame))?; + } + Ok(()) + } +} + +impl Error for CoreJSError {} + #[cfg(test)] mod tests { use super::*; - fn error1() -> JSError { - JSError { + fn error1() -> V8Exception { + V8Exception { message: "Error: foo bar".to_string(), source_line: None, script_resource_name: None, @@ -344,8 +344,8 @@ mod tests { } #[test] - fn js_error_from_v8_exception() { - let r = JSError::from_v8_exception( + fn v8_exception_from_json() { + let r = V8Exception::from_json( r#"{ "message":"Uncaught Error: bad", "frames":[ @@ -387,8 +387,8 @@ mod tests { } #[test] - fn js_error_from_v8_exception2() { - let r = JSError::from_v8_exception( + fn v8_exception_from_json_2() { + let r = V8Exception::from_json( "{\"message\":\"Error: boo\",\"sourceLine\":\"throw Error('boo');\",\"scriptResourceName\":\"a.js\",\"lineNumber\":3,\"startPosition\":8,\"endPosition\":9,\"errorLevel\":8,\"startColumn\":6,\"endColumn\":7,\"isSharedCrossOrigin\":false,\"isOpaque\":false,\"frames\":[{\"line\":3,\"column\":7,\"functionName\":\"\",\"scriptName\":\"a.js\",\"isEval\":false,\"isConstructor\":false,\"isWasm\":false}]}" ); assert!(r.is_some()); @@ -405,16 +405,9 @@ mod tests { assert_eq!(e.frames.len(), 1); } - #[test] - fn stack_frame_to_string() { - let e = error1(); - assert_eq!(" at foo (foo_bar.ts:5:17)", &e.frames[0].to_string()); - assert_eq!(" at qat (bar_baz.ts:6:21)", &e.frames[1].to_string()); - } - #[test] fn js_error_to_string() { - let e = error1(); + let e = CoreJSError(error1()); let expected = "Error: foo bar\n at foo (foo_bar.ts:5:17)\n at qat (bar_baz.ts:6:21)\n at deno_main.js:2:2"; assert_eq!(expected, &e.to_string()); } diff --git a/core/lib.rs b/core/lib.rs index 5bbe2fb8620bed..61521aecb00022 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -4,6 +4,7 @@ extern crate log; extern crate futures; extern crate libc; +mod any_error; mod flags; mod isolate; mod js_errors; @@ -12,6 +13,7 @@ mod module_specifier; mod modules; mod shared_queue; +pub use crate::any_error::*; pub use crate::flags::v8_set_flags; pub use crate::isolate::*; pub use crate::js_errors::*; diff --git a/core/modules.rs b/core/modules.rs index c1fa5d7336564a..4b0d128f2a1553 100644 --- a/core/modules.rs +++ b/core/modules.rs @@ -6,8 +6,8 @@ // small and simple for users who do not use modules or if they do can load them // synchronously. The isolate.rs module should never depend on this module. +use crate::any_error::ErrBox; use crate::isolate::Isolate; -use crate::js_errors::JSError; use crate::libdeno::deno_mod; use crate::module_specifier::ModuleSpecifier; use futures::Async; @@ -15,7 +15,6 @@ use futures::Future; use futures::Poll; use std::collections::HashMap; use std::collections::HashSet; -use std::error::Error; use std::fmt; use std::marker::PhantomData; use std::sync::Arc; @@ -34,12 +33,10 @@ pub struct SourceCodeInfo { pub code: String, } -pub type SourceCodeInfoFuture = - dyn Future + Send; +pub type SourceCodeInfoFuture = + dyn Future + Send; pub trait Loader: Send + Sync { - type Error: std::error::Error + 'static; - /// Returns an absolute URL. /// When implementing an spec-complaint VM, this should be exactly the /// algorithm described here: @@ -49,19 +46,19 @@ pub trait Loader: Send + Sync { specifier: &str, referrer: &str, is_root: bool, - ) -> Result; + ) -> Result; /// Given ModuleSpecifier, load its source code. fn load( &self, module_specifier: &ModuleSpecifier, - ) -> Box>; + ) -> Box; } -struct PendingLoad { +struct PendingLoad { url: String, is_root: bool, - source_code_info_future: Box>, + source_code_info_future: Box, } /// This future is used to implement parallel async module loading without @@ -71,7 +68,7 @@ pub struct RecursiveLoad { loader: L, isolate: Arc>, modules: Arc>, - pending: Vec>, + pending: Vec, is_pending: HashSet, phantom: PhantomData, // TODO(ry) The following can all be combined into a single enum State type. @@ -106,7 +103,7 @@ impl RecursiveLoad { specifier: &str, referrer: &str, parent_id: Option, - ) -> Result { + ) -> Result { let is_root = parent_id.is_none(); let module_specifier = self.loader.resolve(specifier, referrer, is_root)?; let module_name = module_specifier.to_string(); @@ -142,22 +139,16 @@ impl RecursiveLoad { } } -#[derive(Debug, PartialEq)] -pub enum JSErrorOr { - JSError(JSError), - Other(E), -} - impl Future for RecursiveLoad { type Item = deno_mod; - type Error = JSErrorOr; + type Error = ErrBox; fn poll(&mut self) -> Poll { if self.root.is_none() && self.root_specifier.is_some() { let s = self.root_specifier.take().unwrap(); match self.add(&s, ".", None) { Err(err) => { - return Err(JSErrorOr::Other(err)); + return Err(err); } Ok(root) => { self.root = Some(root); @@ -172,7 +163,7 @@ impl Future for RecursiveLoad { let pending = &mut self.pending[i]; match pending.source_code_info_future.poll() { Err(err) => { - return Err(JSErrorOr::Other(err)); + return Err(err); } Ok(Async::NotReady) => { i += 1; @@ -200,18 +191,15 @@ impl Future for RecursiveLoad { if !is_module_registered { let module_name = &source_code_info.module_name; - let result = { + let mod_id = { let isolate = self.isolate.lock().unwrap(); isolate.mod_new( completed.is_root, module_name, &source_code_info.code, ) - }; - if let Err(err) = result { - return Err(JSErrorOr::JSError(err)); - } - let mod_id = result.unwrap(); + }?; + if completed.is_root { assert!(self.root_id.is_none()); self.root_id = Some(mod_id); @@ -235,9 +223,7 @@ impl Future for RecursiveLoad { }; let referrer = module_name; for specifier in imports { - self - .add(&specifier, referrer, Some(mod_id)) - .map_err(JSErrorOr::Other)?; + self.add(&specifier, referrer, Some(mod_id))?; } } else if need_alias { let mut modules = self.modules.lock().unwrap(); @@ -252,31 +238,26 @@ impl Future for RecursiveLoad { } let root_id = self.root_id.unwrap(); - let result = { - let mut resolve_cb = - |specifier: &str, referrer_id: deno_mod| -> deno_mod { - let modules = self.modules.lock().unwrap(); - let referrer = modules.get_name(referrer_id).unwrap(); - // this callback is only called for non-root modules - match self.loader.resolve(specifier, &referrer, false) { - Ok(specifier) => match modules.get_id(&specifier.to_string()) { - Some(id) => id, - None => 0, - }, - // We should have already resolved and loaded this module, so - // resolve() will not fail this time. - Err(_err) => unreachable!(), - } - }; - let mut isolate = self.isolate.lock().unwrap(); - isolate.mod_instantiate(root_id, &mut resolve_cb) + let mut resolve_cb = |specifier: &str, referrer_id: deno_mod| -> deno_mod { + let modules = self.modules.lock().unwrap(); + let referrer = modules.get_name(referrer_id).unwrap(); + // this callback is only called for non-root modules + match self.loader.resolve(specifier, &referrer, false) { + Ok(specifier) => match modules.get_id(&specifier.to_string()) { + Some(id) => id, + None => 0, + }, + // We should have already resolved and loaded this module, so + // resolve() will not fail this time. + Err(_err) => unreachable!(), + } }; - match result { - Err(err) => Err(JSErrorOr::JSError(err)), - Ok(()) => Ok(Async::Ready(root_id)), - } + let mut isolate = self.isolate.lock().unwrap(); + isolate + .mod_instantiate(root_id, &mut resolve_cb) + .map(|_| Async::Ready(root_id)) } } @@ -537,6 +518,7 @@ mod tests { use super::*; use crate::isolate::js_check; use crate::isolate::tests::*; + use std::error::Error; use std::fmt; struct MockLoader { @@ -607,9 +589,9 @@ mod tests { impl Future for DelayedSourceCodeFuture { type Item = SourceCodeInfo; - type Error = MockError; + type Error = ErrBox; - fn poll(&mut self) -> Poll { + fn poll(&mut self) -> Poll { self.counter += 1; if self.url == "file:///never_ready.js" || (self.url == "file:///slow.js" && self.counter < 2) @@ -621,20 +603,18 @@ mod tests { code: src.0.to_owned(), module_name: src.1.to_owned(), })), - None => Err(MockError::LoadErr), + None => Err(MockError::LoadErr.into()), } } } impl Loader for MockLoader { - type Error = MockError; - fn resolve( &self, specifier: &str, referrer: &str, _is_root: bool, - ) -> Result { + ) -> Result { let referrer = if referrer == "." { "file:///" } else { @@ -646,20 +626,20 @@ mod tests { let output_specifier = match ModuleSpecifier::resolve_import(specifier, referrer) { Ok(specifier) => specifier, - Err(..) => return Err(MockError::ResolveErr), + Err(..) => return Err(MockError::ResolveErr.into()), }; if mock_source_code(&output_specifier.to_string()).is_some() { Ok(output_specifier) } else { - Err(MockError::ResolveErr) + Err(MockError::ResolveErr.into()) } } fn load( &self, module_specifier: &ModuleSpecifier, - ) -> Box> { + ) -> Box { let mut loads = self.loads.lock().unwrap(); loads.push(module_specifier.to_string()); let url = module_specifier.to_string(); @@ -962,8 +942,11 @@ mod tests { RecursiveLoad::new("/bad_import.js", loader, isolate, modules); let result = recursive_load.poll(); assert!(result.is_err()); - let either_err = result.err().unwrap(); - assert_eq!(either_err, JSErrorOr::Other(MockError::ResolveErr)); + let err = result.err().unwrap(); + assert_eq!( + err.downcast_ref::().unwrap(), + &MockError::ResolveErr + ); } #[test]