patriksimek / vm2

Advanced vm/sandbox for Node.js
MIT License
3.86k stars 293 forks source link

VM and NodeVM behaves differently on await #513

Closed helloanoop closed 1 year ago

helloanoop commented 1 year ago

I have a requirement to change wait for vm2 to run asynchronous scripts. This works with VM but not with NodeVM.

Is this behaviour expected ?

const {VM, NodeVM} = require('vm2');
const axios = require('axios');

const vm = new VM({
  sandbox: {
    axios
  }
});

const nodevm = new NodeVM({
  sandbox: {
    axios
  }
});

(async () => {
  const res = await vm.run(`
    axios.get('http://httpbin.org/get')
        .then(x => x.data)
        .then(x => x.url);
  `);

  const res2 = await nodevm.run(`
    axios.get('http://httpbin.org/get')
        .then(x => x.data)
        .then(x => x.url);
  `);

  console.log(res); // => http://httpbin.org/get
  console.log(res2); // => {}
})();
JonasJonny commented 1 year ago

@helloanoop Can you try this? For my use case it works fine.

const { NodeVM } = require('vm2');
const { getAbcRequest } = require('...');

async getData(script) {
  try {
    const vm2 = new NodeVM({
      sandbox: {
        getAbcRequest,
      },
    });

    const asyncVM2 = vm2.run(`module.exports = async () => { ${script} }`, 'vm');
    return await asyncVM2();
  } catch(err) {
    return {
      err,
    };
  }
}
const response = await getData(`
  // script
  try {
    const response = await getAbcRequest();
    return response;  
  } catch(err) {
    throw err;
  }
`);

console.log('response', response);
XmiliaH commented 1 year ago

This is expected. As JonasJonny pointed out for NodeVM module.exports as the script is run as a module.

helloanoop commented 1 year ago

Thanks @JonasJonny @XmiliaH

We were able to use the solution proposed by @JonasJonny

In case if anyone in the future bumps into a similar issue, here is the PR that fixed the issue in Bruno: https://github.com/usebruno/bruno/pull/145/files