From f49d9556015a7d4c04e9db3f0c85d4399f6125ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 1 Dec 2020 23:33:44 +0100 Subject: [PATCH] fix(compile): disable source mapping of errors (#8581) This commit disables source mapping of errors for standalone binaries. Since applying source maps relies on using file fetcher infrastructure it's not feasible to use it for standalone binaries that are not supposed to use that infrastructure. --- cli/ops/runtime.rs | 11 ++++++++++- cli/rt/99_main.js | 7 ++++++- cli/standalone.rs | 1 + cli/tests/integration_tests.rs | 36 ++++++++++++++++++++++++++++++++++ cli/tests/standalone_error.ts | 9 +++++++++ cli/web_worker.rs | 2 +- cli/worker.rs | 34 +++++++++++++++++++++----------- core/lib.rs | 1 + core/runtime.rs | 2 +- 9 files changed, 88 insertions(+), 15 deletions(-) create mode 100644 cli/tests/standalone_error.ts diff --git a/cli/ops/runtime.rs b/cli/ops/runtime.rs index 3fd73a67e3..38b23f3b30 100644 --- a/cli/ops/runtime.rs +++ b/cli/ops/runtime.rs @@ -13,11 +13,18 @@ use deno_core::OpState; use deno_core::ZeroCopyBuf; use std::env; -pub fn init(rt: &mut deno_core::JsRuntime, main_module: ModuleSpecifier) { +type ApplySourceMaps = bool; + +pub fn init( + rt: &mut deno_core::JsRuntime, + main_module: ModuleSpecifier, + apply_source_maps: bool, +) { { let op_state = rt.op_state(); let mut state = op_state.borrow_mut(); state.put::(main_module); + state.put::(apply_source_maps); } super::reg_json_sync(rt, "op_start", op_start); super::reg_json_sync(rt, "op_main_module", op_main_module); @@ -29,10 +36,12 @@ fn op_start( _args: Value, _zero_copy: &mut [ZeroCopyBuf], ) -> Result { + let apply_source_maps = *state.borrow::(); let gs = &super::program_state(state); Ok(json!({ "args": gs.flags.argv.clone(), + "applySourceMaps": apply_source_maps, "debugFlag": gs.flags.log_level.map_or(false, |l| l == log::Level::Debug), "denoVersion": version::deno(), "noColor": !colors::use_color(), diff --git a/cli/rt/99_main.js b/cli/rt/99_main.js index 69c27c579e..f582994c7f 100644 --- a/cli/rt/99_main.js +++ b/cli/rt/99_main.js @@ -160,7 +160,12 @@ delete Object.prototype.__proto__; version.setVersions(s.denoVersion, s.v8Version, s.tsVersion); build.setBuildInfo(s.target); util.setLogDebug(s.debugFlag, source); - errorStack.setPrepareStackTrace(Error); + // TODO(bartlomieju): a very crude way to disable + // source mapping of errors. This condition is true + // only for compiled standalone binaries. + if (s.applySourceMaps) { + errorStack.setPrepareStackTrace(Error); + } return s; } diff --git a/cli/standalone.rs b/cli/standalone.rs index 06c20bc2dd..df7106c970 100644 --- a/cli/standalone.rs +++ b/cli/standalone.rs @@ -117,6 +117,7 @@ async fn run(source_code: String, args: Vec) -> Result<(), AnyError> { main_module.clone(), permissions, module_loader, + None, ); worker.execute_module(&main_module).await?; worker.execute("window.dispatchEvent(new Event('load'))")?; diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 4b8d274c78..c990330207 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -4600,6 +4600,42 @@ fn standalone_args() { assert_eq!(output.stdout, b"foo\n--bar\n--unstable\n"); } +#[test] +fn standalone_error() { + let dir = TempDir::new().expect("tempdir fail"); + let exe = if cfg!(windows) { + dir.path().join("error.exe") + } else { + dir.path().join("error") + }; + let output = util::deno_cmd() + .current_dir(util::root_path()) + .arg("compile") + .arg("--unstable") + .arg("--output") + .arg(&exe) + .arg("./cli/tests/standalone_error.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + let output = Command::new(exe) + .env("NO_COLOR", "1") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(!output.status.success()); + assert_eq!(output.stdout, b""); + let expected_stderr = "error: Error: boom!\n at boom (file://$deno$/bundle.js:2:11)\n at foo (file://$deno$/bundle.js:5:5)\n at file://$deno$/bundle.js:7:1\n"; + let stderr = String::from_utf8(output.stderr).unwrap(); + assert_eq!(stderr, expected_stderr); +} + #[test] fn standalone_no_module_load() { let dir = TempDir::new().expect("tempdir fail"); diff --git a/cli/tests/standalone_error.ts b/cli/tests/standalone_error.ts new file mode 100644 index 0000000000..2793981138 --- /dev/null +++ b/cli/tests/standalone_error.ts @@ -0,0 +1,9 @@ +function boom() { + throw new Error("boom!"); +} + +function foo() { + boom(); +} + +foo(); diff --git a/cli/web_worker.rs b/cli/web_worker.rs index 97db42279a..12b79cb2d6 100644 --- a/cli/web_worker.rs +++ b/cli/web_worker.rs @@ -195,7 +195,7 @@ impl WebWorker { } ops::web_worker::init(js_runtime, sender.clone(), handle); - ops::runtime::init(js_runtime, main_module); + ops::runtime::init(js_runtime, main_module, true); ops::fetch::init(js_runtime, program_state.flags.ca_file.as_deref()); ops::timers::init(js_runtime); ops::worker_host::init(js_runtime, Some(sender)); diff --git a/cli/worker.rs b/cli/worker.rs index 68f0a2210e..3068ab1f79 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -15,6 +15,7 @@ use deno_core::error::AnyError; use deno_core::futures::future::poll_fn; use deno_core::futures::future::FutureExt; use deno_core::url::Url; +use deno_core::JsErrorCreateFn; use deno_core::JsRuntime; use deno_core::ModuleId; use deno_core::ModuleLoader; @@ -48,15 +49,6 @@ impl MainWorker { let module_loader = CliModuleLoader::new(program_state.maybe_import_map.clone()); - Self::from_options(program_state, main_module, permissions, module_loader) - } - - pub fn from_options( - program_state: &Arc, - main_module: ModuleSpecifier, - permissions: Permissions, - module_loader: Rc, - ) -> Self { let global_state_ = program_state.clone(); let js_error_create_fn = Box::new(move |core_js_error| { @@ -65,10 +57,30 @@ impl MainWorker { PrettyJsError::create(source_mapped_error) }); + Self::from_options( + program_state, + main_module, + permissions, + module_loader, + Some(js_error_create_fn), + ) + } + + pub fn from_options( + program_state: &Arc, + main_module: ModuleSpecifier, + permissions: Permissions, + module_loader: Rc, + js_error_create_fn: Option>, + ) -> Self { + // TODO(bartlomieju): this is hacky way to not apply source + // maps in JS + let apply_source_maps = js_error_create_fn.is_some(); + let mut js_runtime = JsRuntime::new(RuntimeOptions { module_loader: Some(module_loader), startup_snapshot: Some(js::deno_isolate_init()), - js_error_create_fn: Some(js_error_create_fn), + js_error_create_fn, get_error_class_fn: Some(&crate::errors::get_error_class_name), ..Default::default() }); @@ -105,7 +117,7 @@ impl MainWorker { op_state.put::(permissions); } - ops::runtime::init(js_runtime, main_module); + ops::runtime::init(js_runtime, main_module, apply_source_maps); ops::fetch::init(js_runtime, program_state.flags.ca_file.as_deref()); ops::timers::init(js_runtime); ops::worker_host::init(js_runtime, None); diff --git a/core/lib.rs b/core/lib.rs index 732c5d7452..372cd558ec 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -60,6 +60,7 @@ pub use crate::resources2::Resource; pub use crate::resources2::ResourceId; pub use crate::resources2::ResourceTable2; pub use crate::runtime::GetErrorClassFn; +pub use crate::runtime::JsErrorCreateFn; pub use crate::runtime::JsRuntime; pub use crate::runtime::RuntimeOptions; pub use crate::runtime::Snapshot; diff --git a/core/runtime.rs b/core/runtime.rs index c079783f30..5044072b65 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -53,7 +53,7 @@ pub enum Snapshot { Boxed(Box<[u8]>), } -type JsErrorCreateFn = dyn Fn(JsError) -> AnyError; +pub type JsErrorCreateFn = dyn Fn(JsError) -> AnyError; pub type GetErrorClassFn = &'static dyn for<'e> Fn(&'e AnyError) -> &'static str;