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

Improve exception stack trace output (perhaps expose VM filename and lineOffset options?) #18

Open
mtmacdonald opened this issue Sep 8, 2022 · 2 comments

Comments

@mtmacdonald
Copy link

Hello, thanks for the great package. I would like to request an improvement to how the package handles stack traces when the imported script throws an exception during execution. In my current usage (importFromString without --experimental-vm-modules enabled, i.e. the one that falls back on requireFromString internally), if my imported string throws an exception, the stack trace is unhelpful because:

  • the filename defaults to evalmachine.<anonymous>
  • the line numbers are wrong because module-from-string prepends its own code and also seems to reformat the input string

Is it possible to please improve the stack traces?

One idea/suggestion I have for this is to expose more options from vm.runInNewContext to make this possible, particularly the filename and lineOffset options. I mean roughly like this (then the usages have more control over displaying a meaningful stack trace, although I'm not quite sure how to handle the line offsets given the reformatting of the input - perhaps you could also provide an option to preserve original formatting):

  it('script throws error', () => {
    const context = {
      payload: {foo: 123}
    };
    const migration = `
      const foo = 123;
      const migration = (payload) => {
        throw new Error("bad")
      };
    `;
    vm.runInNewContext(`${migration} migration(payload)`, context, {
      filename: 'real-migration-filename.js',
      lineOffset: 0
    });
  })
bad
real-migration-filename.js:4
        throw new Error("bad")
        ^

Error: bad
    at migration (real-migration-filename.js:104:15)
    at real-migration-filename.js:106:6
    at Script.runInContext (vm.js:144:12)
    at Script.runInNewContext (vm.js:149:17)

What do you think?

@exuanbo
Copy link
Owner

exuanbo commented Sep 8, 2022

Thanks for your suggestion. Actually, requireFromString has already used the filename option:

runInNewContext(code, contextObject, {
filename: moduleFilename,

But moduleFilename from the above code is by default generated as a UUID.

export const requireFromString = (
code: string,
{ dirname = getCallerDirname(), globals = {}, useCurrentGlobal = false }: Options | undefined = {}
): any => {
const moduleFilename = join(dirname, `${nanoid()}.js`)

I will expose an option filename to fix this, it should be just a few lines of code.

As for line offsets, I think it might be possible to reformat the error stack using the sourcemap emitted by esbuild's transform function, which importFromString without --experimental-vm-modules uses to transform ES Module to CommonJS.

@exuanbo
Copy link
Owner

exuanbo commented Sep 16, 2022

Option filename is added in v3.3.0. I will need more time for the line number issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants