Skip to content

Commit

Permalink
fix(compiler): equality and external libraries bugs in simulator (#5554)
Browse files Browse the repository at this point in the history
This pull request fixes a range of funky JavaScript issues we've had running inflight code in the Wing simulator by switching internally from running JS code in a `vm` to running JS code using Node.js child processes.

The main issues with `vm` are:
- several third-party JavaScript dependencies behave differently when executed inside a `vm` and outside. This may be related to differences in vm behavior as described in nodejs/node#28823
- we cannot easily stop/kill code executing in a `vm` process, making it impossible to simulate cloud.Function timeouts

Fixes #1980
Fixes #4131
Fixes #4118
Fixes #4792

Closes #4725

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
Chriscbr authored Mar 6, 2024
1 parent 4ac907a commit 845360a
Show file tree
Hide file tree
Showing 26 changed files with 641 additions and 278 deletions.
8 changes: 2 additions & 6 deletions examples/tests/sdk_tests/function/timeout.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ new std.Test(inflight () => {
assert(e.contains("Task timed out after"));
}

if (util.env("WING_TARGET") != "sim") {
assert(c.peek() == 0);
}
assert(c.peek() == 0);

try {
f2.invoke("");
Expand All @@ -49,8 +47,6 @@ new std.Test(inflight () => {
assert(e.contains("Task timed out after"));
}

if (util.env("WING_TARGET") != "sim") {
assert(c.peek() == 1);
}
assert(c.peek() == 1);

}, std.TestProps {timeout: 2m}) as "timeout";
5 changes: 5 additions & 0 deletions examples/tests/valid/deep_equality.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ test "Json with different values" {
assert(!(jsonA != jsonB));
}

test "Json.values equality" {
let j = Json { hello: 123, world: [1, 2, 3] };
assert(Json.values(j) == [Json 123, Json [1, 2, 3]]);
}

//-----------------------------------------------------------------------------
// Set
//-----------------------------------------------------------------------------
Expand Down
10 changes: 3 additions & 7 deletions examples/tests/valid/inflight_handler_singleton.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,7 @@ test "single instance of Foo" {
expect.equal(x, "100");
expect.equal(z, "100-fn2"); // fn2 should have a separate instance

// the simulator intentionally reuses the sandbox across invocations
// but we can't trust that this will always happen on the cloud
if sim {
expect.equal(y, "101");
expect.equal(z, "100-fn2"); // fn2 should have a separate instance
log("client has been reused");
}
// y could be 100 or 101 depending on whether the execution environment
// was reused or not between the two calls.
assert(y == "100" || y == "101");
}
4 changes: 0 additions & 4 deletions libs/wingsdk/.projen/deps.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion libs/wingsdk/.projenrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ const project = new cdk.JsiiProject({
// simulator dependencies
"express",
"uuid",
"undici",
// using version 3 because starting from version 4, it no longer works with CommonJS.
"nanoid@^3.3.6",
"cron-parser",
Expand Down
2 changes: 0 additions & 2 deletions libs/wingsdk/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 9 additions & 6 deletions libs/wingsdk/src/shared/bundling.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as crypto from "crypto";
import { mkdirSync, writeFileSync } from "fs";
import { join, resolve } from "path";
import { mkdirSync, realpathSync, writeFileSync } from "fs";
import { posix, resolve } from "path";
import { normalPath } from "./misc";

const SDK_PATH = normalPath(resolve(__dirname, "..", ".."));
Expand All @@ -25,21 +25,24 @@ export function createBundle(
external: string[] = [],
outputDir?: string
): Bundle {
const outdir = resolve(outputDir ?? entrypoint + ".bundle");
const normalEntrypoint = normalPath(realpathSync(entrypoint));
const outdir = outputDir
? normalPath(realpathSync(outputDir))
: `${normalEntrypoint}.bundle`;
mkdirSync(outdir, { recursive: true });

const outfileName = "index.js";
const soucemapFilename = `${outfileName}.map`;

const outfile = join(outdir, outfileName);
const outfileMap = join(outdir, soucemapFilename);
const outfile = posix.join(outdir, outfileName);
const outfileMap = posix.join(outdir, soucemapFilename);

// eslint-disable-next-line import/no-extraneous-dependencies,@typescript-eslint/no-require-imports
const esbuilder: typeof import("esbuild") = require("esbuild");

let esbuild = esbuilder.buildSync({
bundle: true,
entryPoints: [normalPath(resolve(entrypoint))],
entryPoints: [normalEntrypoint],
outfile,
// otherwise there are problems with running azure cloud functions:
// https://stackoverflow.com/questions/70332883/webpack-azure-storage-blob-node-fetch-abortsignal-issue
Expand Down
143 changes: 143 additions & 0 deletions libs/wingsdk/src/shared/legacy-sandbox.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import { mkdtemp, readFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import path from "node:path";
import * as util from "node:util";
import * as vm from "node:vm";
import { createBundle } from "./bundling";
import { SandboxOptions } from "./sandbox";

export class LegacySandbox {
private createBundlePromise: Promise<void>;
private entrypoint: string;
private code: string | undefined;
private readonly options: SandboxOptions;
private readonly context: any = {};

constructor(entrypoint: string, options: SandboxOptions = {}) {
this.entrypoint = entrypoint;
this.options = options;
this.context = this.createContext();
this.createBundlePromise = this.createBundle();
}

private createContext() {
const sandboxProcess = {
...process,

// override process.exit to throw an exception instead of exiting the process
exit: (exitCode: number) => {
throw new Error("process.exit() was called with exit code " + exitCode);
},

env: this.options.env,
};

const sandboxConsole: any = {};
const levels = ["debug", "info", "log", "warn", "error"];
for (const level of levels) {
sandboxConsole[level] = (...args: any[]) => {
const message = util.format(...args);
this.options.log?.(false, level, message);
// also log to stderr if DEBUG is set
if (process.env.DEBUG) {
console.error(message);
}
};
}

const ctx: any = {};

// create a copy of all the globals from our current context.
for (const k of Object.getOwnPropertyNames(global)) {
try {
ctx[k] = (global as any)[k];
} catch {
// ignore unresolvable globals (see https://github.com/winglang/wing/pull/1923)
}
}

// append the user's context
for (const [k, v] of Object.entries(this.options.context ?? {})) {
ctx[k] = v;
}

const context = vm.createContext({
...ctx,
process: sandboxProcess,
console: sandboxConsole,
exports: {},
require, // to support requiring node.js sdk modules (others will be bundled)
});

// emit an explicit error when trying to access `__dirname` and `__filename` because we cannot
// resolve these when bundling (this is true both for simulator and the cloud since we are
// bundling there as well).
const forbidGlobal = (name: string) => {
Object.defineProperty(context, name, {
get: () => {
throw new Error(
`${name} cannot be used within bundled cloud functions`
);
},
});
};

forbidGlobal("__dirname");
forbidGlobal("__filename");

return context;
}

private async createBundle() {
// load bundle into context on first run
const workdir = await mkdtemp(path.join(tmpdir(), "wing-bundles-"));
const bundle = createBundle(this.entrypoint, [], workdir);
this.entrypoint = bundle.entrypointPath;

this.code = await readFile(this.entrypoint, "utf-8");

if (process.env.DEBUG) {
const bundleSize = Buffer.byteLength(this.code, "utf-8");
this.options.log?.(true, "log", `Bundled code (${bundleSize} bytes).`);
}
}

public async call(fn: string, ...args: any[]): Promise<any> {
// wait for the bundle to finish creation
await this.createBundlePromise;

if (!this.code) {
throw new Error("Bundle not created yet - please report this as a bug");
}

// this will add stuff to the "exports" object within our context
vm.runInContext(this.code!, this.context, {
filename: this.entrypoint,
});

return new Promise(($resolve, $reject) => {
const cleanup = () => {
delete this.context.$resolve;
delete this.context.$reject;
};

this.context.$resolve = (value: any) => {
cleanup();
$resolve(value);
};

this.context.$reject = (reason?: any) => {
cleanup();
$reject(reason);
};

const code = `exports.${fn}(${args
.map((arg) => JSON.stringify(arg))
.join(",")}).then($resolve).catch($reject);`;
vm.runInContext(code, this.context, {
filename: this.entrypoint,
timeout: this.options.timeout,
});
});
}
}
Loading

0 comments on commit 845360a

Please sign in to comment.