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.

Sign up for GitHub

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

[WIP] feat(sdk): use workers for inflight code #5547

Closed
wants to merge 1 commit into from
Closed

[WIP] feat(sdk): use workers for inflight code #5547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
226 changes: 114 additions & 112 deletions libs/wingsdk/src/shared/sandbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { mkdtemp, readFile } from "fs/promises";
import { tmpdir } from "os";
import * as path from "path";
import * as util from "util";
import * as vm from "vm";
import { Worker } from "worker_threads";
import { createBundle } from "./bundling";

export interface SandboxOptions {
Expand All @@ -13,86 +13,22 @@ export interface SandboxOptions {
}

export class Sandbox {
private loaded = false; // "true" after first run (module is loaded into context)
private createBundlePromise: Promise<void>;
private entrypoint: string;
private code: string | undefined;
private readonly timeouts: NodeJS.Timeout[] = [];
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();
this.createBundlePromise = this.createBundle(); // start bundle creation
}

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);
}
};
public async stop(): Promise<void> {
for (const timeout of this.timeouts) {
clearTimeout(timeout);
}

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() {
Expand All @@ -109,55 +45,121 @@ export class Sandbox {
}
}

private loadBundleOnce() {
if (this.loaded) {
return;
}

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,
});

this.loaded = true;
}

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

// load the bundle into context on the first run
// we don't do this earlier because bundled code may have side effects
// and we want to simulate that a function is "cold" on the first run
this.loadBundleOnce();

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

this.context.$resolve = (value: any) => {
cleanup();
$resolve(value);
};
// 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 shim = `
const { parentPort: $parent } = require('worker_threads');
const $resolve = (value) => $parent.postMessage({ type: 'resolve', value });
const $reject = (reason) => $parent.postMessage({ type: 'reject', reason });
console.log = (...args) => $parent.postMessage({ type: 'log', args });
console.debug = (...args) => $parent.postMessage({ type: 'debug', args });
console.info = (...args) => $parent.postMessage({ type: 'info', args });
console.warn = (...args) => $parent.postMessage({ type: 'warn', args });
console.error = (...args) => $parent.postMessage({ type: 'error', args });
process.exit = (exitCode) => $parent.postMessage({ type: 'exit', exitCode });
Object.defineProperty(globalThis, "__dirname", {
get: () => { throw new Error("__dirname cannot be used within bundled cloud functions"); },
});
Object.defineProperty(globalThis, "__filename", {
get: () => { throw new Error("__filename cannot be used within bundled cloud functions"); },
});
Comment on lines +65 to +70
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't esbuild already fail if we have any __dirname or __filename?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a major limitation we have today. There's a good chance that existing user code will depend on these.

Any thoughts on how to address this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarkMcCulloh From what I can tell esbuild doesn't reject code with __dirname, it just silently leaves it (leading to possibly wrong runtime behavior).

It looks like it could be possible to support it through an esbuild plugin? evanw/esbuild#859

${this.code}
$parent.on('message', async (message) => {
try {
const result = await exports[message.fn](...message.payload);
$resolve(result);
} catch (error) {
$reject(error);
}
});
`;
console.error(shim);

// currently, a fresh worker is used for every invocation
// it could be better to keep the worker alive and reuse it
// but this requires additional work to make sure logs between invocations
// are not mixed up, and timeouts are handled correctly
const worker = new Worker(shim, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a regression. I think we need to retain and reuse one worker for all invocations in order to simulate lambda host reuse (#5478).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether this is an optimization that holds water. See #5549

env: this.options.env,
eval: true,
});
Comment on lines +87 to +90
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To fix sourcemaps, a more typical way to do this is to use a preload script instead of an eval'd shim:

const worker = new Worker(this.entrypoint, {
  env: this.options.env,
  // create a local file called sandbox-shim.ts with all the extra stuff needed
  // This file will be executed first, then the entrypoint
  execArgv: ["-r", require.resolve("./sandbox-shim")],
});


this.context.$reject = (reason?: any) => {
cleanup();
$reject(reason);
};
let cleanupStarted = false;
const cleanupWorker = async () => {
cleanupStarted = true;
try {
await worker.terminate();
} catch (err) {
console.error("worker terminate error:", err);
}
};

const code = `exports.${fn}(${args.join(
","
)}).then($resolve).catch($reject);`;
vm.runInContext(code, this.context, {
filename: this.entrypoint,
timeout: this.options.timeout,
return new Promise((resolve, reject) => {
worker.postMessage({ type: "invoke", fn, payload: args });
worker.on("message", (message) => {
switch (message.type) {
case "resolve":
void cleanupWorker().then(() => resolve(message.value));
break;
case "reject":
console.error("rejecting", message);
void cleanupWorker().then(() => reject(message.reason));
break;
case "log":
case "debug":
case "info":
case "warn":
case "error":
this.options.log?.(
false,
message.type,
util.format(...message.args)
);
if (process.env.DEBUG) {
console.error(message);
}
break;
case "exit":
void cleanupWorker().then(() =>
reject(
new Error(
`process.exit() was called with exit code ${message.exitCode}`
)
)
);
break;
default:
console.error("Unknown message type", message);
}
});
worker.on("error", (error) => {
void cleanupWorker().then(() => reject(error));
});
worker.on("exit", (code) => {
if (cleanupStarted) {
// worker was terminated by us, so we don't need to reject
} else {
reject(new Error(`Worker stopped with exit code ${code}`));
}
});

if (this.options.timeout) {
const timeout = setTimeout(() => {
void cleanupWorker().then(() => {
reject(
new Error(
`Function timed out (it was configured to only run for ${this.options.timeout}ms)`
)
);
});
}, this.options.timeout);
this.timeouts.push(timeout);
}
});
}
}
6 changes: 3 additions & 3 deletions libs/wingsdk/src/target-sim/function.inflight.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class Function implements IFunctionClient, ISimulatorResourceInstance {
}

public async cleanup(): Promise<void> {
return;
await this.sandbox.stop();
}

public async save(): Promise<void> {}
Expand All @@ -55,7 +55,7 @@ export class Function implements IFunctionClient, ISimulatorResourceInstance {
return this.context.withTrace({
message: `Invoke (payload=${JSON.stringify(payload)}).`,
activity: async () => {
return this.sandbox.call("handler", JSON.stringify(payload)) ?? "";
return this.sandbox.call("handler", payload) ?? "";
},
});
}
Expand All @@ -65,7 +65,7 @@ export class Function implements IFunctionClient, ISimulatorResourceInstance {
message: `InvokeAsync (payload=${JSON.stringify(payload)}).`,
activity: async () => {
process.nextTick(() => {
void this.sandbox.call("handler", JSON.stringify(payload));
void this.sandbox.call("handler", payload);
});
},
});
Expand Down
11 changes: 3 additions & 8 deletions libs/wingsdk/test/simulator/__snapshots__/simulator.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,9 @@ exports[`run single test > happy path 1`] = `
exports[`run single test > test failure 1`] = `
{
"error": "Error: test failed
<sanitized>
<sanitized>
<sanitized>
at Script.runInContext (node:vm:<sanitized>)
at Object.runInContext (node:vm:<sanitized>)
<sanitized>
at new Promise (<anonymous>)
<sanitized>
at Handler.handle ([worker eval]:<sanitized>)
at exports.handler ([worker eval]:<sanitized>)
at MessagePort.<anonymous> ([worker eval]:<sanitized>)
<sanitized>
<sanitized>",
"pass": false,
Expand Down
3 changes: 1 addition & 2 deletions libs/wingsdk/test/target-sim/__snapshots__/topic.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ exports[`topic publishes messages as they are received 1`] = `
"Publish (message=Alpha).",
"Sending message (message=Alpha, subscriber=sim-0).",
"InvokeAsync (payload=\\"Alpha\\").",
"Received Alpha",
"Publish (message=Beta).",
"Sending message (message=Beta, subscriber=sim-0).",
"InvokeAsync (payload=\\"Beta\\").",
"Received Alpha",
"Received Beta",
"@winglang/sdk.sim.EventMapping deleted.",
"@winglang/sdk.cloud.Topic deleted.",
Expand All @@ -157,7 +157,6 @@ exports[`topic publishes messages to multiple subscribers 1`] = `
"Publish (message=Alpha).",
"Sending message (message=Alpha, subscriber=sim-0).",
"InvokeAsync (payload=\\"Alpha\\").",
"Received Alpha",
"Sending message (message=Alpha, subscriber=sim-3).",
"InvokeAsync (payload=\\"Alpha\\").",
"@winglang/sdk.sim.EventMapping deleted.",
Expand Down
Loading