Skip to content

Commit

Permalink
fix: stack traces for modules imported via std/node's require (denola…
Browse files Browse the repository at this point in the history
  • Loading branch information
seishun authored Mar 19, 2020
1 parent 74c37e7 commit 8c1c929
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 6 deletions.
6 changes: 4 additions & 2 deletions cli/js/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
8 changes: 7 additions & 1 deletion core/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -448,6 +449,9 @@ fn eval_context(
}
};

let url = v8::Local::<v8::String>::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
Expand All @@ -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, "<unknown>").unwrap();
let name =
v8::String::new(scope, url.as_ref().map_or("<unknown>", Url::as_str))
.unwrap();
let origin = script_origin(scope, name);
let maybe_script = v8::Script::compile(scope, context, source, Some(&origin));

Expand Down
4 changes: 2 additions & 2 deletions std/node/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
11 changes: 10 additions & 1 deletion std/node/module_test.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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");
}
});
5 changes: 5 additions & 0 deletions std/node/tests/cjs/cjs_throw.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function hello() {
throw new Error("bye");
}

module.exports = { hello };

0 comments on commit 8c1c929

Please sign in to comment.