Add support for "correct" stack traces for errors in the sandbox

[CapacitorSet]

It would be nice if vm2 could display "correct" stack traces when an error is thrown from the sandbox.

For instance, this is what is currently displayed:

/home/user/box-js/node_modules/vm2/lib/main.js:213
                        throw this._internal.Decontextify.value(e);
                        ^

Error: foobar
    at Object.log (/home/user/box-js/analyze.js:248:10)
    at Object.apply (/home/user/box-js/node_modules/vm2/lib/contextify.js:288:34)
    at vm.js:491:9
    at ContextifyScript.Script.runInContext (vm.js:53:29)
    at VM.run (/home/user/box-js/node_modules/vm2/lib/main.js:207:72)
    at Object.<anonymous> (/home/user/box-js/analyze.js:383:5)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)

Is there a way to figure out which line (in the sandboxed code) was responsible for calling the function that threw the error?

[tbenst]

This would be awesome. I'm trying to track down a coding mistake right now and having a fiendishly hard time without the line number

[CapacitorSet]

If you're running test cases that you know to be safe, you can temporarily replace vm2 with the native vm to track down the error.

[simon-o-matic]

I'd like this feature too. Happy to pitch in to write it if someone points me in the right direction.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tbenst]

Turns out it’s not trivial to replace vm2 with Native vm as there are api incompatibilities, hope this can stay open!

[CapacitorSet]

@patriksimek, I devised a simple implementation based on an exception parser/rewriter. I think it would be worth it to integrate it in vm2 as an optional error handler.

const StackTracey = require ('stacktracey');
const { VM } = require('vm2');

const sourceCode = `function f() {
	throw Exception('e');
}
f();`;
const scriptName = "hello_world.js";

process.on("uncaughtException",  e => {
	if (!e.stack.includes("/node_modules/vm2/")) {
		// This is not a vm2 error, so print it normally
		console.log(e);
		return;
	}
	const oldStack = new StackTracey(e);
	const newStack = [];
	for (const line of oldStack) {
		// Discard internal code
		if (line.file.includes("/cjs"))
			continue;
		if (line.thirdParty && line.file.includes("/node_modules/vm2/"))
			continue;
		if (line.callee == "Script.runInContext")
			continue;
		// Replace the default filename with the user-provided one
		if (line.fileName == "vm.js")
			line.fileRelative = line.fileShort = line.fileName = scriptName;
		newStack.push(line);
	}
	console.log("[vm2] A clean stack trace follows:");
	console.log(new StackTracey(newStack).pretty);
});

const vm = new VM();
vm.run(sourceCode, scriptName);

The above code would print:

ReferenceError: Exception is not defined
    at f (vm.js:2:2)
    at vm.js:4:1
    at Script.runInContext (vm.js:135:20)
    at VM.run (/tmp/vm2-test/node_modules/vm2/lib/main.js:210:72)
    at Object.<anonymous> (/tmp/vm2-test/index.js:33:4)
    at Module._compile (internal/modules/cjs/loader.js:805:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:816:10)
    at Module.load (internal/modules/cjs/loader.js:672:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:612:12)
    at Function.Module._load (internal/modules/cjs/loader.js:604:3)
[vm2] A clean stack trace follows:
at f            hello_world.js:2
at              hello_world.js:4
at <anonymous>  index.js:33       vm.run(sourceCode, scriptName);

[madjake]

I haven't had a chance to test this library but I recently used the vm module and accomplished accurate stack trace numbers by setting the appropriate line offset when creating a new vm.Script.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tbenst]

Figured I’d comment as this issue + suggestions are still relevant for me!

[edopalmieri]

Very relevant to me too.

@patriksimek can @CapacitorSet's implementation or something along those lines be integrated in the project? Thanks.

[CapacitorSet]

Patrik, does the implementation sketch look okay overall? If so I can make a PR and we can work from there.

[patriksimek]

@CapacitorSet Yes, I'll be happy to review a PR. Some notes:

• The vm.run method doesn't accept the second parameter - that's why the script name is ignored. Currently, you need to create a VMScript with the name specified first. But that can be easily improved.
• Is there a way to avoid using external lib? Would be nice to stick with zero deps.

[CapacitorSet]

Just leaving a comment to notify people who are subscribed to this issue -- I opened a PR, so you can track the progress there.