From 8c1c929034d46101b6a51ec5cf5e2f307ed0c271 Mon Sep 17 00:00:00 2001 From: Nikolai Vavilov Date: Thu, 19 Mar 2020 16:42:07 +0200 Subject: [PATCH] fix: stack traces for modules imported via std/node's require (#4035) --- cli/js/globals.ts | 6 ++++-- core/bindings.rs | 8 +++++++- std/node/module.ts | 4 ++-- std/node/module_test.ts | 11 ++++++++++- std/node/tests/cjs/cjs_throw.js | 5 +++++ 5 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 std/node/tests/cjs/cjs_throw.js diff --git a/cli/js/globals.ts b/cli/js/globals.ts index 21ce7e6198..1104762b91 100644 --- a/cli/js/globals.ts +++ b/cli/js/globals.ts @@ -93,8 +93,10 @@ declare global { shared: SharedArrayBuffer; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - evalContext(code: string): [any, EvalErrorInfo | null]; + evalContext( + code: string, + scriptName?: string + ): [unknown, EvalErrorInfo | null]; formatError: (e: Error) => string; diff --git a/core/bindings.rs b/core/bindings.rs index 9df9a78a0c..ae138bfbfc 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -10,6 +10,7 @@ use v8::MapFnTo; use std::convert::TryFrom; use std::option::Option; +use url::Url; lazy_static! { pub static ref EXTERNAL_REFERENCES: v8::ExternalReferences = @@ -448,6 +449,9 @@ fn eval_context( } }; + let url = v8::Local::::try_from(args.get(1)) + .map(|n| Url::from_file_path(n.to_rust_string_lossy(scope)).unwrap()); + let output = v8::Array::new(scope, 2); /* output[0] = result @@ -460,7 +464,9 @@ fn eval_context( */ let mut try_catch = v8::TryCatch::new(scope); let tc = try_catch.enter(); - let name = v8::String::new(scope, "").unwrap(); + let name = + v8::String::new(scope, url.as_ref().map_or("", Url::as_str)) + .unwrap(); let origin = script_origin(scope, name); let maybe_script = v8::Script::compile(scope, context, source, Some(&origin)); diff --git a/std/node/module.ts b/std/node/module.ts index 01f1f04774..21992f7431 100644 --- a/std/node/module.ts +++ b/std/node/module.ts @@ -1045,11 +1045,11 @@ type RequireWrapper = ( __dirname: string ) => void; -function wrapSafe(filename_: string, content: string): RequireWrapper { +function wrapSafe(filename: string, content: string): RequireWrapper { // TODO: fix this const wrapper = Module.wrap(content); // @ts-ignore - const [f, err] = Deno.core.evalContext(wrapper); + const [f, err] = Deno.core.evalContext(wrapper, filename); if (err) { throw err; } diff --git a/std/node/module_test.ts b/std/node/module_test.ts index 6de5bd66ec..be36a8b6d2 100644 --- a/std/node/module_test.ts +++ b/std/node/module_test.ts @@ -1,5 +1,5 @@ const { test } = Deno; -import { assertEquals, assert } from "../testing/asserts.ts"; +import { assertEquals, assert, assertStrContains } from "../testing/asserts.ts"; import { createRequire } from "./module.ts"; // TS compiler would try to resolve if function named "require" @@ -48,3 +48,12 @@ test(function requireNodeOs() { assert(os.arch); assert(typeof os.arch() == "string"); }); + +test(function requireStack() { + const { hello } = require_("./tests/cjs/cjs_throw"); + try { + hello(); + } catch (e) { + assertStrContains(e.stack, "/tests/cjs/cjs_throw.js"); + } +}); diff --git a/std/node/tests/cjs/cjs_throw.js b/std/node/tests/cjs/cjs_throw.js new file mode 100644 index 0000000000..a636f406b0 --- /dev/null +++ b/std/node/tests/cjs/cjs_throw.js @@ -0,0 +1,5 @@ +function hello() { + throw new Error("bye"); +} + +module.exports = { hello };