Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compile): disable source mapping of errors #8581

Merged
merged 1 commit into from Dec 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 10 additions & 1 deletion cli/ops/runtime.rs
Expand Up @@ -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::<ModuleSpecifier>(main_module);
state.put::<ApplySourceMaps>(apply_source_maps);
}
super::reg_json_sync(rt, "op_start", op_start);
super::reg_json_sync(rt, "op_main_module", op_main_module);
Expand All @@ -29,10 +36,12 @@ fn op_start(
_args: Value,
_zero_copy: &mut [ZeroCopyBuf],
) -> Result<Value, AnyError> {
let apply_source_maps = *state.borrow::<ApplySourceMaps>();
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(),
Expand Down
7 changes: 6 additions & 1 deletion cli/rt/99_main.js
Expand Up @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions cli/standalone.rs
Expand Up @@ -117,6 +117,7 @@ async fn run(source_code: String, args: Vec<String>) -> Result<(), AnyError> {
main_module.clone(),
permissions,
module_loader,
None,
);
worker.execute_module(&main_module).await?;
worker.execute("window.dispatchEvent(new Event('load'))")?;
Expand Down
36 changes: 36 additions & 0 deletions cli/tests/integration_tests.rs
Expand Up @@ -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");
Expand Down
9 changes: 9 additions & 0 deletions cli/tests/standalone_error.ts
@@ -0,0 +1,9 @@
function boom() {
throw new Error("boom!");
}

function foo() {
boom();
}

foo();
2 changes: 1 addition & 1 deletion cli/web_worker.rs
Expand Up @@ -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));
Expand Down
32 changes: 22 additions & 10 deletions cli/worker.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -48,27 +49,38 @@ 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(
program_state: &Arc<ProgramState>,
main_module: ModuleSpecifier,
permissions: Permissions,
module_loader: Rc<dyn ModuleLoader>,
js_error_create_fn: Option<Box<JsErrorCreateFn>>,
) -> 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()
});
Expand Down Expand Up @@ -105,7 +117,7 @@ impl MainWorker {
op_state.put::<Permissions>(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);
Expand Down
1 change: 1 addition & 0 deletions core/lib.rs
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion core/runtime.rs
Expand Up @@ -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;
Expand Down