diff --git a/packages/builder/src/components/common/bindings/BindingPanel.svelte b/packages/builder/src/components/common/bindings/BindingPanel.svelte index d8edf0cbb1..2f39dc6f53 100644 --- a/packages/builder/src/components/common/bindings/BindingPanel.svelte +++ b/packages/builder/src/components/common/bindings/BindingPanel.svelte @@ -66,6 +66,7 @@ let insertAtPos let targetMode = null let expressionResult + let expressionError let evaluating = false $: useSnippets = allowSnippets && !$licensing.isFreePlan @@ -142,10 +143,16 @@ } const debouncedEval = Utils.debounce((expression, context, snippets) => { - expressionResult = processStringSync(expression || "", { - ...context, - snippets, - }) + try { + expressionError = null + expressionResult = processStringSync(expression || "", { + ...context, + snippets, + }) + } catch (err) { + expressionResult = null + expressionError = err + } evaluating = false }, 260) @@ -370,6 +377,7 @@ {:else if sidePanel === SidePanels.Evaluation} diff --git a/packages/builder/src/components/common/bindings/EvaluationSidePanel.svelte b/packages/builder/src/components/common/bindings/EvaluationSidePanel.svelte index 2c4e6a0991..ffb8f45297 100644 --- a/packages/builder/src/components/common/bindings/EvaluationSidePanel.svelte +++ b/packages/builder/src/components/common/bindings/EvaluationSidePanel.svelte @@ -3,26 +3,37 @@ import { Icon, ProgressCircle, notifications } from "@budibase/bbui" import { copyToClipboard } from "@budibase/bbui/helpers" import { fade } from "svelte/transition" + import { UserScriptError } from "@budibase/string-templates" export let expressionResult + export let expressionError export let evaluating = false export let expression = null - $: error = expressionResult === "Error while executing JS" + $: error = expressionError != null $: empty = expression == null || expression?.trim() === "" $: success = !error && !empty $: highlightedResult = highlight(expressionResult) + const formatError = err => { + if (err.code === UserScriptError.code) { + return err.userScriptError.toString() + } + return err.toString() + } + const highlight = json => { if (json == null) { return "" } - // Attempt to parse and then stringify, in case this is valid JSON + + // Attempt to parse and then stringify, in case this is valid result try { json = JSON.stringify(JSON.parse(json), null, 2) } catch (err) { // Ignore } + return formatHighlight(json, { keyColor: "#e06c75", numberColor: "#e5c07b", @@ -34,7 +45,7 @@ } const copy = () => { - let clipboardVal = expressionResult + let clipboardVal = expressionResult.result if (typeof clipboardVal === "object") { clipboardVal = JSON.stringify(clipboardVal, null, 2) } @@ -73,6 +84,8 @@
{#if empty} Your expression will be evaluated here + {:else if error} + {formatError(expressionError)} {:else} {@html highlightedResult} diff --git a/packages/server/src/api/controllers/script.ts b/packages/server/src/api/controllers/script.ts index 4317f5fcde..0565b30f74 100644 --- a/packages/server/src/api/controllers/script.ts +++ b/packages/server/src/api/controllers/script.ts @@ -1,11 +1,18 @@ import { Ctx } from "@budibase/types" import { IsolatedVM } from "../../jsRunner/vm" -import { iifeWrapper } from "@budibase/string-templates" +import { iifeWrapper, UserScriptError } from "@budibase/string-templates" export async function execute(ctx: Ctx) { const { script, context } = ctx.request.body const vm = new IsolatedVM() - ctx.body = vm.withContext(context, () => vm.execute(iifeWrapper(script))) + try { + ctx.body = vm.withContext(context, () => vm.execute(iifeWrapper(script))) + } catch (err: any) { + if (err.code === UserScriptError.code) { + throw err.userScriptError + } + throw err + } } export async function save(ctx: Ctx) { diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index f751942df9..5222069460 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -49,6 +49,7 @@ import * as uuid from "uuid" import { Knex } from "knex" import { InternalTables } from "../../../db/utils" import { withEnv } from "../../../environment" +import { JsTimeoutError } from "@budibase/string-templates" const timestamp = new Date("2023-01-26T11:48:57.597Z").toISOString() tk.freeze(timestamp) @@ -3013,7 +3014,7 @@ describe.each([ let i = 0 for (; i < 10; i++) { const row = rows[i] - if (row.formula !== "Timed out while executing JS") { + if (row.formula !== JsTimeoutError.message) { break } } @@ -3027,7 +3028,7 @@ describe.each([ for (; i < 10; i++) { const row = rows[i] expect(row.text).toBe("foo") - expect(row.formula).toBe("Request JS execution limit hit") + expect(row.formula).toStartWith("CPU time limit exceeded ") } } } diff --git a/packages/server/src/jsRunner/index.ts b/packages/server/src/jsRunner/index.ts index ccfca7cb4c..e17529a687 100644 --- a/packages/server/src/jsRunner/index.ts +++ b/packages/server/src/jsRunner/index.ts @@ -1,7 +1,7 @@ import { serializeError } from "serialize-error" import env from "../environment" import { - JsErrorTimeout, + JsTimeoutError, setJSRunner, setOnErrorLog, } from "@budibase/string-templates" @@ -40,7 +40,7 @@ export function init() { return vm.withContext(rest, () => vm.execute(js)) } catch (error: any) { if (error.message === "Script execution timed out.") { - throw new JsErrorTimeout() + throw new JsTimeoutError() } throw error } diff --git a/packages/server/src/jsRunner/tests/jsRunner.spec.ts b/packages/server/src/jsRunner/tests/jsRunner.spec.ts index 525f6d865e..7d2df17c1f 100644 --- a/packages/server/src/jsRunner/tests/jsRunner.spec.ts +++ b/packages/server/src/jsRunner/tests/jsRunner.spec.ts @@ -41,10 +41,11 @@ describe("jsRunner (using isolated-vm)", () => { }) it("should prevent sandbox escape", async () => { - const output = await processJS( - `return this.constructor.constructor("return process.env")()` + await expect( + processJS(`return this.constructor.constructor("return process.env")()`) + ).rejects.toThrow( + "error while running user-supplied JavaScript: ReferenceError: process is not defined" ) - expect(output).toBe("Error while executing JS") }) describe("helpers", () => { diff --git a/packages/server/src/jsRunner/vm/isolated-vm.ts b/packages/server/src/jsRunner/vm/isolated-vm.ts index 87d5b966c2..b8ed90aa23 100644 --- a/packages/server/src/jsRunner/vm/isolated-vm.ts +++ b/packages/server/src/jsRunner/vm/isolated-vm.ts @@ -7,14 +7,12 @@ import querystring from "querystring" import { BundleType, loadBundle } from "../bundles" import { Snippet, VM } from "@budibase/types" -import { iifeWrapper } from "@budibase/string-templates" +import { iifeWrapper, UserScriptError } from "@budibase/string-templates" import environment from "../../environment" -class ExecutionTimeoutError extends Error { - constructor(message: string) { - super(message) - this.name = "ExecutionTimeoutError" - } +export class JsRequestTimeoutError extends Error { + static code = "JS_REQUEST_TIMEOUT_ERROR" + code = JsRequestTimeoutError.code } export class IsolatedVM implements VM { @@ -29,6 +27,7 @@ export class IsolatedVM implements VM { private readonly resultKey = "results" private runResultKey: string + private runErrorKey: string constructor({ memoryLimit, @@ -47,6 +46,7 @@ export class IsolatedVM implements VM { this.jail.setSync("global", this.jail.derefInto()) this.runResultKey = crypto.randomUUID() + this.runErrorKey = crypto.randomUUID() this.addToContext({ [this.resultKey]: { [this.runResultKey]: "" }, }) @@ -210,13 +210,19 @@ export class IsolatedVM implements VM { if (this.isolateAccumulatedTimeout) { const cpuMs = Number(this.isolate.cpuTime) / 1e6 if (cpuMs > this.isolateAccumulatedTimeout) { - throw new ExecutionTimeoutError( + throw new JsRequestTimeoutError( `CPU time limit exceeded (${cpuMs}ms > ${this.isolateAccumulatedTimeout}ms)` ) } } - code = `results['${this.runResultKey}']=${this.codeWrapper(code)}` + code = ` + try { + results['${this.runResultKey}']=${this.codeWrapper(code)} + } catch (e) { + results['${this.runErrorKey}']=e + } + ` const script = this.isolate.compileScriptSync(code) @@ -227,6 +233,9 @@ export class IsolatedVM implements VM { // We can't rely on the script run result as it will not work for non-transferable values const result = this.getFromContext(this.resultKey) + if (result[this.runErrorKey]) { + throw new UserScriptError(result[this.runErrorKey]) + } return result[this.runResultKey] } diff --git a/packages/server/src/utilities/rowProcessor/utils.ts b/packages/server/src/utilities/rowProcessor/utils.ts index 0fa6f62807..29a5de221f 100644 --- a/packages/server/src/utilities/rowProcessor/utils.ts +++ b/packages/server/src/utilities/rowProcessor/utils.ts @@ -1,5 +1,5 @@ import { AutoFieldDefaultNames } from "../../constants" -import { processStringSync } from "@budibase/string-templates" +import { processStringSync, UserScriptError } from "@budibase/string-templates" import { AutoColumnFieldMetadata, FieldSchema, @@ -81,7 +81,14 @@ export async function processFormulas( ...row, [column]: tracer.trace("processStringSync", {}, span => { span?.addTags({ table_id: table._id, column, static: isStatic }) - return processStringSync(formula, context) + try { + return processStringSync(formula, context) + } catch (err: any) { + if (err.code === UserScriptError.code) { + return err.userScriptError.toString() + } + throw err + } }), } } diff --git a/packages/string-templates/src/errors.ts b/packages/string-templates/src/errors.ts index 79a8a525ef..77526c1b96 100644 --- a/packages/string-templates/src/errors.ts +++ b/packages/string-templates/src/errors.ts @@ -1,3 +1,20 @@ -export class JsErrorTimeout extends Error { - code = "ERR_SCRIPT_EXECUTION_TIMEOUT" +export class JsTimeoutError extends Error { + static message = "Timed out while executing JS" + static code = "JS_TIMEOUT_ERROR" + code: string = JsTimeoutError.code + + constructor() { + super(JsTimeoutError.message) + } +} + +export class UserScriptError extends Error { + static code = "USER_SCRIPT_ERROR" + code: string = UserScriptError.code + + constructor(readonly userScriptError: Error) { + super( + `error while running user-supplied JavaScript: ${userScriptError.toString()}` + ) + } } diff --git a/packages/string-templates/src/helpers/javascript.ts b/packages/string-templates/src/helpers/javascript.ts index 3e16d8a07b..ed8d4ad6d7 100644 --- a/packages/string-templates/src/helpers/javascript.ts +++ b/packages/string-templates/src/helpers/javascript.ts @@ -3,6 +3,7 @@ import cloneDeep from "lodash/fp/cloneDeep" import { LITERAL_MARKER } from "../helpers/constants" import { getJsHelperList } from "./list" import { iifeWrapper } from "../iife" +import { JsTimeoutError, UserScriptError } from "../errors" // The method of executing JS scripts depends on the bundle being built. // This setter is used in the entrypoint (either index.js or index.mjs). @@ -94,12 +95,41 @@ export function processJS(handlebars: string, context: any) { } catch (error: any) { onErrorLog && onErrorLog(error) + // The error handling below is quite messy, because it has fallen to + // string-templates to handle a variety of types of error specific to usages + // above it in the stack. It would be nice some day to refactor this to + // allow each user of processStringSync to handle errors in the way they see + // fit. + + // This is to catch the error vm.runInNewContext() throws when the timeout + // is exceeded. if (error.code === "ERR_SCRIPT_EXECUTION_TIMEOUT") { return "Timed out while executing JS" } - if (error.name === "ExecutionTimeoutError") { - return "Request JS execution limit hit" + + // This is to catch the JsRequestTimeoutError we throw when we detect a + // timeout across an entire request in the backend. We use a magic string + // because we can't import from the backend into string-templates. + if (error.code === "JS_REQUEST_TIMEOUT_ERROR") { + return error.message } + + // This is to catch the JsTimeoutError we throw when we detect a timeout in + // a single JS execution. + if (error.code === JsTimeoutError.code) { + return JsTimeoutError.message + } + + // This is to catch the error that happens if a user-supplied JS script + // throws for reasons introduced by the user. + if (error.code === UserScriptError.code) { + throw error + } + + if (error.name === "SyntaxError") { + return error.toString() + } + return "Error while executing JS" } } diff --git a/packages/string-templates/src/index.ts b/packages/string-templates/src/index.ts index 8d5fe4c16d..0f5326374f 100644 --- a/packages/string-templates/src/index.ts +++ b/packages/string-templates/src/index.ts @@ -16,6 +16,7 @@ import { removeJSRunner, setJSRunner } from "./helpers/javascript" import manifest from "./manifest.json" import { ProcessOptions } from "./types" +import { UserScriptError } from "./errors" export { helpersToRemoveForJs, getJsHelperList } from "./helpers/list" export { FIND_ANY_HBS_REGEX } from "./utilities" @@ -229,7 +230,10 @@ export function processStringSync( } else { return process(string) } - } catch (err) { + } catch (err: any) { + if (err.code === UserScriptError.code) { + throw err + } return input } } @@ -448,7 +452,7 @@ export function convertToJS(hbs: string) { return `${varBlock}${js}` } -export { JsErrorTimeout } from "./errors" +export { JsTimeoutError, UserScriptError } from "./errors" export function defaultJSSetup() { if (!isBackendService()) { @@ -463,7 +467,27 @@ export function defaultJSSetup() { setTimeout: undefined, } createContext(context) - return runInNewContext(js, context, { timeout: 1000 }) + + const wrappedJs = ` + result = { + result: null, + error: null, + }; + + try { + result.result = ${js}; + } catch (e) { + result.error = e; + } + + result; + ` + + const result = runInNewContext(wrappedJs, context, { timeout: 1000 }) + if (result.error) { + throw new UserScriptError(result.error) + } + return result.result }) } else { removeJSRunner()