From df242cc2ad25978786b5da80045579fde989241c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 3 Oct 2024 13:07:22 +0100 Subject: [PATCH] Don't break the fact that processStringSync returns a string. --- .../common/bindings/BindingPanel.svelte | 16 ++++++--- .../bindings/EvaluationSidePanel.svelte | 33 +++++++++++-------- .../server/src/jsRunner/vm/isolated-vm.ts | 2 ++ .../src/utilities/rowProcessor/utils.ts | 9 ++++- packages/string-templates/src/errors.ts | 10 ++++++ .../src/helpers/javascript.ts | 4 +-- packages/string-templates/src/index.ts | 14 ++++++-- 7 files changed, 64 insertions(+), 24 deletions(-) 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 3b71fad715..2c5c1d4587 100644 --- a/packages/builder/src/components/common/bindings/EvaluationSidePanel.svelte +++ b/packages/builder/src/components/common/bindings/EvaluationSidePanel.svelte @@ -5,32 +5,35 @@ import { fade } from "svelte/transition" export let expressionResult + export let expressionError export let evaluating = false export let expression = null - $: error = expressionResult && expressionResult.error != null + $: error = expressionError != null $: empty = expression == null || expression?.trim() === "" $: success = !error && !empty $: highlightedResult = highlight(expressionResult) - const highlight = result => { - if (result == null) { + const formatError = err => { + if (err.code === "USER_SCRIPT_ERROR") { + return err.userScriptError.toString() + } + return err.toString() + } + + const highlight = json => { + if (json == null) { return "" } - let str - if (result.error) { - str = result.error.toString() - } else { - // Attempt to parse and then stringify, in case this is valid result - try { - str = JSON.stringify(JSON.parse(result.result), null, 2) - } catch (err) { - // Ignore - } + // 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(str, { + return formatHighlight(json, { keyColor: "#e06c75", numberColor: "#e5c07b", stringColor: "#98c379", @@ -80,6 +83,8 @@
{#if empty} Your expression will be evaluated here + {:else if error} + {formatError(expressionError)} {:else} {@html highlightedResult} diff --git a/packages/server/src/jsRunner/vm/isolated-vm.ts b/packages/server/src/jsRunner/vm/isolated-vm.ts index e502b799bd..9262687925 100644 --- a/packages/server/src/jsRunner/vm/isolated-vm.ts +++ b/packages/server/src/jsRunner/vm/isolated-vm.ts @@ -11,6 +11,7 @@ import { iifeWrapper } from "@budibase/string-templates" import environment from "../../environment" class ExecutionTimeoutError extends Error { + code = "ERR_SCRIPT_EXECUTION_TIMEOUT" constructor(message: string) { super(message) this.name = "ExecutionTimeoutError" @@ -18,6 +19,7 @@ class ExecutionTimeoutError extends Error { } class UserScriptError extends Error { + code = "USER_SCRIPT_ERROR" constructor(readonly userScriptError: Error) { super( `error while running user-supplied JavaScript: ${userScriptError.message}`, diff --git a/packages/server/src/utilities/rowProcessor/utils.ts b/packages/server/src/utilities/rowProcessor/utils.ts index 0fa6f62807..3a1ce08ece 100644 --- a/packages/server/src/utilities/rowProcessor/utils.ts +++ b/packages/server/src/utilities/rowProcessor/utils.ts @@ -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 === "USER_SCRIPT_ERROR") { + return err.userScriptError.toString() + } + throw err + } }), } } diff --git a/packages/string-templates/src/errors.ts b/packages/string-templates/src/errors.ts index 79a8a525ef..4c7cbdb360 100644 --- a/packages/string-templates/src/errors.ts +++ b/packages/string-templates/src/errors.ts @@ -1,3 +1,13 @@ export class JsErrorTimeout extends Error { code = "ERR_SCRIPT_EXECUTION_TIMEOUT" } + +export class UserScriptError extends Error { + code = "USER_SCRIPT_ERROR" + + constructor(readonly userScriptError: Error) { + super( + `error while running user-supplied JavaScript: ${userScriptError.message}` + ) + } +} diff --git a/packages/string-templates/src/helpers/javascript.ts b/packages/string-templates/src/helpers/javascript.ts index 27a71da834..e1d7f3e8a3 100644 --- a/packages/string-templates/src/helpers/javascript.ts +++ b/packages/string-templates/src/helpers/javascript.ts @@ -100,8 +100,8 @@ export function processJS(handlebars: string, context: any) { if (error.name === "ExecutionTimeoutError") { return "Request JS execution limit hit" } - if ("userScriptError" in error) { - return error.userScriptError.toString() + if (error.code === "USER_SCRIPT_ERROR") { + throw error } return "Error while executing JS" } diff --git a/packages/string-templates/src/index.ts b/packages/string-templates/src/index.ts index 69ca05ef76..aceea1da7e 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" @@ -230,6 +231,9 @@ export function processStringSync( return process(string) } } catch (err) { + if (err.code === "USER_SCRIPT_ERROR") { + throw err + } return input } } @@ -448,7 +452,7 @@ export function convertToJS(hbs: string) { return `${varBlock}${js}` } -export { JsErrorTimeout } from "./errors" +export { JsErrorTimeout, UserScriptError } from "./errors" export function defaultJSSetup() { if (!isBackendService()) { @@ -473,13 +477,17 @@ export function defaultJSSetup() { try { result.result = ${js}; } catch (e) { - result.error = e.toString(); + result.error = e; } result; ` - return runInNewContext(js, context, { timeout: 1000 }) + const result = runInNewContext(js, context, { timeout: 1000 }) + if (result.error) { + throw new UserScriptError(result.error) + } + return result.result }) } else { removeJSRunner()