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: 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 | 32 ++++++++++++++++++++++---------- 7 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 cli/tests/standalone_error.ts (limited to 'cli') diff --git a/cli/ops/runtime.rs b/cli/ops/runtime.rs index 3fd73a67e..38b23f3b3 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 69c27c579..f582994c7 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 06c20bc2d..df7106c97 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 4b8d274c7..c99033020 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 000000000..279398113 --- /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 97db42279..12b79cb2d 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 68f0a2210..3068ab1f7 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,7 +49,21 @@ impl MainWorker { let module_loader = CliModuleLoader::new(program_state.maybe_import_map.clone()); - Self::from_options(program_state, main_module, permissions, module_loader) + let global_state_ = program_state.clone(); + + let js_error_create_fn = Box::new(move |core_js_error| { + let source_mapped_error = + apply_source_map(&core_js_error, global_state_.clone()); + PrettyJsError::create(source_mapped_error) + }); + + Self::from_options( + program_state, + main_module, + permissions, + module_loader, + Some(js_error_create_fn), + ) } pub fn from_options( @@ -56,19 +71,16 @@ impl MainWorker { main_module: ModuleSpecifier, permissions: Permissions, module_loader: Rc, + js_error_create_fn: Option>, ) -> Self { - let global_state_ = program_state.clone(); - - let js_error_create_fn = Box::new(move |core_js_error| { - let source_mapped_error = - apply_source_map(&core_js_error, global_state_.clone()); - PrettyJsError::create(source_mapped_error) - }); + // 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); -- cgit v1.2.3