-
Notifications
You must be signed in to change notification settings - Fork 36
Description
Description
When executing JavaScript code with async/await patterns using sandbox.runCode(), Promises are not awaited and result.results[0] contains an empty object instead of the resolved value.
Reproduction
See minimal reproduction repo: https://github.com/seanrcollings/cloudflare-worker-async-js
const code = `
(async () => {
const response = await fetch("https://api.github.com");
const data = await response.json();
return data;
})()
`;
const result = await sandbox.runCode(code, {
language: 'javascript'
});
// Expected: result.results[0].json contains API response
// Actual: result.results[0].json = {}
console.log(result.results[0]);Issue: The returned expression from the isolate instance is a promise, but this promise is never awaited so we can't get the result.
Investigation
Through experimentation, I noticed the JavaScript execution environment appears to use Node.js vm.runInContext() internally.
Node.js docs: https://nodejs.org/api/vm.html#timeout-interactions-with-asynchronous-tasks-and-promises
Potential Solution
I've attempted a fix in my fork: seanrcollings#1
Change: Check if the result is promise-like and await it if that is the case. in brief this
- Configure the VM with
microtaskMode: 'afterEvaluate'which gives each context it's own microtask queue - Adds additional timeout handling for the async portion of the code flow
- Manually drains the context's microtask queue until the promise settles
- returns the resolve value of the promise.
In my local testing, this allows:
- Promises to be resolved before returning
- Timeouts to apply to async code
- The resolved value to be captured in results
However, I'm not deeply familiar with the JavaScript runtime implementation, so there may be other considerations or better approaches.
Impact
High - Currently, any modern JavaScript using async/await cannot be executed properly, limiting the Sandbox SDK to synchronous code only.
Questions
- Is async/await support intended for
runCode()? - If my approach looks reasonable, would you accept a PR with this fix?
- Are there other considerations or alternative approaches I should be aware of?