exuanbo / module-from-string

Load module from string using require or import.
MIT License
47 stars 2 forks source link

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

Open mtmacdonald opened 1 year ago

mtmacdonald commented 1 year ago

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:

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 commented 1 year ago

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

https://github.com/exuanbo/module-from-string/blob/2d6b66186eb811434a2195b88b332ff7efd642ea/src/require.ts#L43-L44

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

https://github.com/exuanbo/module-from-string/blob/2d6b66186eb811434a2195b88b332ff7efd642ea/src/require.ts#L19-L23

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 commented 1 year ago

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