From 7ce9756d8c7b70656c44ed7a25665f21252e58da Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 9 Feb 2024 16:36:43 +0100 Subject: [PATCH] Revert jsRunner changes to vm --- packages/backend-core/src/context/types.ts | 2 + packages/backend-core/src/timers/timers.ts | 40 +++++++++++ packages/server/src/environment.ts | 4 +- packages/server/src/jsRunner/index.ts | 79 ++++++++++++++-------- 4 files changed, 93 insertions(+), 32 deletions(-) diff --git a/packages/backend-core/src/context/types.ts b/packages/backend-core/src/context/types.ts index 0f4c2106d0..6fb9f44fad 100644 --- a/packages/backend-core/src/context/types.ts +++ b/packages/backend-core/src/context/types.ts @@ -1,4 +1,5 @@ import { IdentityContext, VM } from "@budibase/types" +import { ExecutionTimeTracker } from "../timers" // keep this out of Budibase types, don't want to expose context info export type ContextMap = { @@ -9,5 +10,6 @@ export type ContextMap = { isScim?: boolean automationId?: string isMigrating?: boolean + jsExecutionTracker?: ExecutionTimeTracker vm?: VM } diff --git a/packages/backend-core/src/timers/timers.ts b/packages/backend-core/src/timers/timers.ts index 000be74821..baebc57974 100644 --- a/packages/backend-core/src/timers/timers.ts +++ b/packages/backend-core/src/timers/timers.ts @@ -20,3 +20,43 @@ export function cleanup() { } intervals = [] } + + + +export class ExecutionTimeoutError extends Error { + public readonly name = "ExecutionTimeoutError" +} + +export class ExecutionTimeTracker { + static withLimit(limitMs: number) { + return new ExecutionTimeTracker(limitMs) + } + + constructor(readonly limitMs: number) {} + + private totalTimeMs = 0 + + track(f: () => T): T { + this.checkLimit() + const start = process.hrtime.bigint() + try { + return f() + } finally { + const end = process.hrtime.bigint() + this.totalTimeMs += Number(end - start) / 1e6 + this.checkLimit() + } + } + + get elapsedMS() { + return this.totalTimeMs + } + + checkLimit() { + if (this.totalTimeMs > this.limitMs) { + throw new ExecutionTimeoutError( + `Execution time limit of ${this.limitMs}ms exceeded: ${this.totalTimeMs}ms` + ) + } + } +} \ No newline at end of file diff --git a/packages/server/src/environment.ts b/packages/server/src/environment.ts index b5d468ec00..ba3aa280e2 100644 --- a/packages/server/src/environment.ts +++ b/packages/server/src/environment.ts @@ -72,9 +72,9 @@ const environment = { HTTP_MB_LIMIT: process.env.HTTP_MB_LIMIT, FORKED_PROCESS_NAME: process.env.FORKED_PROCESS_NAME || "main", JS_PER_INVOCATION_TIMEOUT_MS: - parseIntSafe(process.env.JS_PER_INVOCATION_TIMEOUT_MS) || 1000, + parseIntSafe(process.env.JS_PER_EXECUTION_TIME_LIMIT_MS) || 1000, JS_PER_REQUEST_TIMEOUT_MS: parseIntSafe( - process.env.JS_PER_REQUEST_TIMEOUT_MS + process.env.JS_PER_REQUEST_TIME_LIMIT_MS ), // old CLIENT_ID: process.env.CLIENT_ID, diff --git a/packages/server/src/jsRunner/index.ts b/packages/server/src/jsRunner/index.ts index 90cc0e2564..1936b0ef45 100644 --- a/packages/server/src/jsRunner/index.ts +++ b/packages/server/src/jsRunner/index.ts @@ -1,42 +1,61 @@ +import vm from "vm" import env from "../environment" -import { setJSRunner, JsErrorTimeout } from "@budibase/string-templates" +import { setJSRunner } from "@budibase/string-templates" +import { context, timers } from "@budibase/backend-core" import tracer from "dd-trace" -import { IsolatedVM } from "./vm" -import { context } from "@budibase/backend-core" +type TrackerFn = (f: () => T) => T export function init() { - setJSRunner((js: string, ctx: Record) => { + setJSRunner((js: string, ctx: vm.Context) => { return tracer.trace("runJS", {}, span => { - try { - const bbCtx = context.getCurrentContext()! - - let { vm } = bbCtx - if (!vm) { - // Can't copy the native helpers into the isolate. We just ignore them as they are handled properly from the helpersSource - const { helpers, ...ctxToPass } = ctx - - vm = new IsolatedVM({ - memoryLimit: env.JS_RUNNER_MEMORY_LIMIT, - invocationTimeout: env.JS_PER_INVOCATION_TIMEOUT_MS, - isolateAccumulatedTimeout: env.JS_PER_REQUEST_TIMEOUT_MS, + const perRequestLimit = env.JS_PER_REQUEST_TIMEOUT_MS + let track: TrackerFn = f => f() + if (perRequestLimit) { + const bbCtx = tracer.trace("runJS.getCurrentContext", {}, span => + context.getCurrentContext() + ) + if (bbCtx) { + if (!bbCtx.jsExecutionTracker) { + span?.addTags({ + createdExecutionTracker: true, + }) + bbCtx.jsExecutionTracker = tracer.trace( + "runJS.createExecutionTimeTracker", + {}, + span => timers.ExecutionTimeTracker.withLimit(perRequestLimit) + ) + } + span?.addTags({ + js: { + limitMS: bbCtx.jsExecutionTracker.limitMs, + elapsedMS: bbCtx.jsExecutionTracker.elapsedMS, + }, + }) + // We call checkLimit() here to prevent paying the cost of creating + // a new VM context below when we don't need to. + tracer.trace("runJS.checkLimitAndBind", {}, span => { + bbCtx.jsExecutionTracker!.checkLimit() + track = bbCtx.jsExecutionTracker!.track.bind( + bbCtx.jsExecutionTracker + ) }) - .withContext(ctxToPass) - .withHelpers() - - bbCtx.vm = vm } - - const result = vm.execute(js) - - return result - } catch (error: any) { - if (error.message === "Script execution timed out.") { - throw new JsErrorTimeout() - } - - throw error } + + ctx = { + ...ctx, + alert: undefined, + setInterval: undefined, + setTimeout: undefined, + } + + vm.createContext(ctx) + return track(() => + vm.runInNewContext(js, ctx, { + timeout: env.JS_PER_INVOCATION_TIMEOUT_MS, + }) + ) }) }) }