vm.runInContext rewrites thrown error messages

[domenic]

"use strict";
const vm = require("vm");

try {
  vm.runInContext("throw new Error('boo');", vm.createContext({}));
} catch (e) {
  console.log(e.message);
}

Should give: boo

Instead gives:

$ iojs test.js
evalmachine.<anonymous>:1
throw new Error('boo');
      ^
boo

Modifying the actual .message property here seems very, very bad.

/cc @indutny since I believe you made changes to the error-message related stuff a while back.

[indutny]

Oh no, not this again :)

[chrisdickinson]

(Maybe of note: I also had to change the add-an-arrow-to-errors logic in node.cc to get code coverage to work. Eventually I just landed on bumping up sizeof(arrow) to 2048 or so.)

[indutny]

Very interesting... So @domenic am I right that you expect it to print stuff to stderr only if the exception was uncaught?

[indutny]

This test wants it to be completely different: https://github.com/nodejs/io.js/blob/master/test/sequential/test-vm-syntax-error-stderr.js

[indutny]

It is easy to fix the problem, if we are sure what the problem is :) It is just a matter of removing display_errors from node_contextify.cc, or by making it default to false.

Alternatively, you may want to pass displayErrors: false option to vm.runInContext(...) to fix your sample.

[domenic]

No, I want it to give "boo". A flag called displayErrors should not modify the .message property of objects returned from the vm.

[indutny]

Ok, I just got an idea how it could be fixed.

[indutny]

@domenic here you go: #2108 . Still it breaks the test, because it expects stack property to have the arrow in it. Please consider collaborating on that PR or this issue on how it should be resolved.

[domenic]

Fixed by #2108