Merge pull request #14693 from Budibase/js-stack-traces
Pull the error object out of isolated-vm when a user script throws an error.
This commit is contained in:
commit
c2358a6d6d
|
@ -66,6 +66,7 @@
|
||||||
let insertAtPos
|
let insertAtPos
|
||||||
let targetMode = null
|
let targetMode = null
|
||||||
let expressionResult
|
let expressionResult
|
||||||
|
let expressionError
|
||||||
let evaluating = false
|
let evaluating = false
|
||||||
|
|
||||||
$: useSnippets = allowSnippets && !$licensing.isFreePlan
|
$: useSnippets = allowSnippets && !$licensing.isFreePlan
|
||||||
|
@ -142,10 +143,16 @@
|
||||||
}
|
}
|
||||||
|
|
||||||
const debouncedEval = Utils.debounce((expression, context, snippets) => {
|
const debouncedEval = Utils.debounce((expression, context, snippets) => {
|
||||||
expressionResult = processStringSync(expression || "", {
|
try {
|
||||||
...context,
|
expressionError = null
|
||||||
snippets,
|
expressionResult = processStringSync(expression || "", {
|
||||||
})
|
...context,
|
||||||
|
snippets,
|
||||||
|
})
|
||||||
|
} catch (err) {
|
||||||
|
expressionResult = null
|
||||||
|
expressionError = err
|
||||||
|
}
|
||||||
evaluating = false
|
evaluating = false
|
||||||
}, 260)
|
}, 260)
|
||||||
|
|
||||||
|
@ -370,6 +377,7 @@
|
||||||
{:else if sidePanel === SidePanels.Evaluation}
|
{:else if sidePanel === SidePanels.Evaluation}
|
||||||
<EvaluationSidePanel
|
<EvaluationSidePanel
|
||||||
{expressionResult}
|
{expressionResult}
|
||||||
|
{expressionError}
|
||||||
{evaluating}
|
{evaluating}
|
||||||
expression={editorValue}
|
expression={editorValue}
|
||||||
/>
|
/>
|
||||||
|
|
|
@ -3,26 +3,37 @@
|
||||||
import { Icon, ProgressCircle, notifications } from "@budibase/bbui"
|
import { Icon, ProgressCircle, notifications } from "@budibase/bbui"
|
||||||
import { copyToClipboard } from "@budibase/bbui/helpers"
|
import { copyToClipboard } from "@budibase/bbui/helpers"
|
||||||
import { fade } from "svelte/transition"
|
import { fade } from "svelte/transition"
|
||||||
|
import { UserScriptError } from "@budibase/string-templates"
|
||||||
|
|
||||||
export let expressionResult
|
export let expressionResult
|
||||||
|
export let expressionError
|
||||||
export let evaluating = false
|
export let evaluating = false
|
||||||
export let expression = null
|
export let expression = null
|
||||||
|
|
||||||
$: error = expressionResult === "Error while executing JS"
|
$: error = expressionError != null
|
||||||
$: empty = expression == null || expression?.trim() === ""
|
$: empty = expression == null || expression?.trim() === ""
|
||||||
$: success = !error && !empty
|
$: success = !error && !empty
|
||||||
$: highlightedResult = highlight(expressionResult)
|
$: highlightedResult = highlight(expressionResult)
|
||||||
|
|
||||||
|
const formatError = err => {
|
||||||
|
if (err.code === UserScriptError.code) {
|
||||||
|
return err.userScriptError.toString()
|
||||||
|
}
|
||||||
|
return err.toString()
|
||||||
|
}
|
||||||
|
|
||||||
const highlight = json => {
|
const highlight = json => {
|
||||||
if (json == null) {
|
if (json == null) {
|
||||||
return ""
|
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 {
|
try {
|
||||||
json = JSON.stringify(JSON.parse(json), null, 2)
|
json = JSON.stringify(JSON.parse(json), null, 2)
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
// Ignore
|
// Ignore
|
||||||
}
|
}
|
||||||
|
|
||||||
return formatHighlight(json, {
|
return formatHighlight(json, {
|
||||||
keyColor: "#e06c75",
|
keyColor: "#e06c75",
|
||||||
numberColor: "#e5c07b",
|
numberColor: "#e5c07b",
|
||||||
|
@ -34,7 +45,7 @@
|
||||||
}
|
}
|
||||||
|
|
||||||
const copy = () => {
|
const copy = () => {
|
||||||
let clipboardVal = expressionResult
|
let clipboardVal = expressionResult.result
|
||||||
if (typeof clipboardVal === "object") {
|
if (typeof clipboardVal === "object") {
|
||||||
clipboardVal = JSON.stringify(clipboardVal, null, 2)
|
clipboardVal = JSON.stringify(clipboardVal, null, 2)
|
||||||
}
|
}
|
||||||
|
@ -73,6 +84,8 @@
|
||||||
<div class="body">
|
<div class="body">
|
||||||
{#if empty}
|
{#if empty}
|
||||||
Your expression will be evaluated here
|
Your expression will be evaluated here
|
||||||
|
{:else if error}
|
||||||
|
{formatError(expressionError)}
|
||||||
{:else}
|
{:else}
|
||||||
<!-- eslint-disable-next-line svelte/no-at-html-tags-->
|
<!-- eslint-disable-next-line svelte/no-at-html-tags-->
|
||||||
{@html highlightedResult}
|
{@html highlightedResult}
|
||||||
|
|
|
@ -1,11 +1,18 @@
|
||||||
import { Ctx } from "@budibase/types"
|
import { Ctx } from "@budibase/types"
|
||||||
import { IsolatedVM } from "../../jsRunner/vm"
|
import { IsolatedVM } from "../../jsRunner/vm"
|
||||||
import { iifeWrapper } from "@budibase/string-templates"
|
import { iifeWrapper, UserScriptError } from "@budibase/string-templates"
|
||||||
|
|
||||||
export async function execute(ctx: Ctx) {
|
export async function execute(ctx: Ctx) {
|
||||||
const { script, context } = ctx.request.body
|
const { script, context } = ctx.request.body
|
||||||
const vm = new IsolatedVM()
|
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) {
|
export async function save(ctx: Ctx) {
|
||||||
|
|
|
@ -49,6 +49,7 @@ import * as uuid from "uuid"
|
||||||
import { Knex } from "knex"
|
import { Knex } from "knex"
|
||||||
import { InternalTables } from "../../../db/utils"
|
import { InternalTables } from "../../../db/utils"
|
||||||
import { withEnv } from "../../../environment"
|
import { withEnv } from "../../../environment"
|
||||||
|
import { JsTimeoutError } from "@budibase/string-templates"
|
||||||
|
|
||||||
const timestamp = new Date("2023-01-26T11:48:57.597Z").toISOString()
|
const timestamp = new Date("2023-01-26T11:48:57.597Z").toISOString()
|
||||||
tk.freeze(timestamp)
|
tk.freeze(timestamp)
|
||||||
|
@ -3013,7 +3014,7 @@ describe.each([
|
||||||
let i = 0
|
let i = 0
|
||||||
for (; i < 10; i++) {
|
for (; i < 10; i++) {
|
||||||
const row = rows[i]
|
const row = rows[i]
|
||||||
if (row.formula !== "Timed out while executing JS") {
|
if (row.formula !== JsTimeoutError.message) {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -3027,7 +3028,7 @@ describe.each([
|
||||||
for (; i < 10; i++) {
|
for (; i < 10; i++) {
|
||||||
const row = rows[i]
|
const row = rows[i]
|
||||||
expect(row.text).toBe("foo")
|
expect(row.text).toBe("foo")
|
||||||
expect(row.formula).toBe("Request JS execution limit hit")
|
expect(row.formula).toStartWith("CPU time limit exceeded ")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
import { serializeError } from "serialize-error"
|
import { serializeError } from "serialize-error"
|
||||||
import env from "../environment"
|
import env from "../environment"
|
||||||
import {
|
import {
|
||||||
JsErrorTimeout,
|
JsTimeoutError,
|
||||||
setJSRunner,
|
setJSRunner,
|
||||||
setOnErrorLog,
|
setOnErrorLog,
|
||||||
} from "@budibase/string-templates"
|
} from "@budibase/string-templates"
|
||||||
|
@ -40,7 +40,7 @@ export function init() {
|
||||||
return vm.withContext(rest, () => vm.execute(js))
|
return vm.withContext(rest, () => vm.execute(js))
|
||||||
} catch (error: any) {
|
} catch (error: any) {
|
||||||
if (error.message === "Script execution timed out.") {
|
if (error.message === "Script execution timed out.") {
|
||||||
throw new JsErrorTimeout()
|
throw new JsTimeoutError()
|
||||||
}
|
}
|
||||||
throw error
|
throw error
|
||||||
}
|
}
|
||||||
|
|
|
@ -41,10 +41,11 @@ describe("jsRunner (using isolated-vm)", () => {
|
||||||
})
|
})
|
||||||
|
|
||||||
it("should prevent sandbox escape", async () => {
|
it("should prevent sandbox escape", async () => {
|
||||||
const output = await processJS(
|
await expect(
|
||||||
`return this.constructor.constructor("return process.env")()`
|
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", () => {
|
describe("helpers", () => {
|
||||||
|
|
|
@ -7,14 +7,12 @@ import querystring from "querystring"
|
||||||
|
|
||||||
import { BundleType, loadBundle } from "../bundles"
|
import { BundleType, loadBundle } from "../bundles"
|
||||||
import { Snippet, VM } from "@budibase/types"
|
import { Snippet, VM } from "@budibase/types"
|
||||||
import { iifeWrapper } from "@budibase/string-templates"
|
import { iifeWrapper, UserScriptError } from "@budibase/string-templates"
|
||||||
import environment from "../../environment"
|
import environment from "../../environment"
|
||||||
|
|
||||||
class ExecutionTimeoutError extends Error {
|
export class JsRequestTimeoutError extends Error {
|
||||||
constructor(message: string) {
|
static code = "JS_REQUEST_TIMEOUT_ERROR"
|
||||||
super(message)
|
code = JsRequestTimeoutError.code
|
||||||
this.name = "ExecutionTimeoutError"
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export class IsolatedVM implements VM {
|
export class IsolatedVM implements VM {
|
||||||
|
@ -29,6 +27,7 @@ export class IsolatedVM implements VM {
|
||||||
|
|
||||||
private readonly resultKey = "results"
|
private readonly resultKey = "results"
|
||||||
private runResultKey: string
|
private runResultKey: string
|
||||||
|
private runErrorKey: string
|
||||||
|
|
||||||
constructor({
|
constructor({
|
||||||
memoryLimit,
|
memoryLimit,
|
||||||
|
@ -47,6 +46,7 @@ export class IsolatedVM implements VM {
|
||||||
this.jail.setSync("global", this.jail.derefInto())
|
this.jail.setSync("global", this.jail.derefInto())
|
||||||
|
|
||||||
this.runResultKey = crypto.randomUUID()
|
this.runResultKey = crypto.randomUUID()
|
||||||
|
this.runErrorKey = crypto.randomUUID()
|
||||||
this.addToContext({
|
this.addToContext({
|
||||||
[this.resultKey]: { [this.runResultKey]: "" },
|
[this.resultKey]: { [this.runResultKey]: "" },
|
||||||
})
|
})
|
||||||
|
@ -210,13 +210,19 @@ export class IsolatedVM implements VM {
|
||||||
if (this.isolateAccumulatedTimeout) {
|
if (this.isolateAccumulatedTimeout) {
|
||||||
const cpuMs = Number(this.isolate.cpuTime) / 1e6
|
const cpuMs = Number(this.isolate.cpuTime) / 1e6
|
||||||
if (cpuMs > this.isolateAccumulatedTimeout) {
|
if (cpuMs > this.isolateAccumulatedTimeout) {
|
||||||
throw new ExecutionTimeoutError(
|
throw new JsRequestTimeoutError(
|
||||||
`CPU time limit exceeded (${cpuMs}ms > ${this.isolateAccumulatedTimeout}ms)`
|
`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)
|
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
|
// We can't rely on the script run result as it will not work for non-transferable values
|
||||||
const result = this.getFromContext(this.resultKey)
|
const result = this.getFromContext(this.resultKey)
|
||||||
|
if (result[this.runErrorKey]) {
|
||||||
|
throw new UserScriptError(result[this.runErrorKey])
|
||||||
|
}
|
||||||
return result[this.runResultKey]
|
return result[this.runResultKey]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
import { AutoFieldDefaultNames } from "../../constants"
|
import { AutoFieldDefaultNames } from "../../constants"
|
||||||
import { processStringSync } from "@budibase/string-templates"
|
import { processStringSync, UserScriptError } from "@budibase/string-templates"
|
||||||
import {
|
import {
|
||||||
AutoColumnFieldMetadata,
|
AutoColumnFieldMetadata,
|
||||||
FieldSchema,
|
FieldSchema,
|
||||||
|
@ -81,7 +81,14 @@ export async function processFormulas<T extends Row | Row[]>(
|
||||||
...row,
|
...row,
|
||||||
[column]: tracer.trace("processStringSync", {}, span => {
|
[column]: tracer.trace("processStringSync", {}, span => {
|
||||||
span?.addTags({ table_id: table._id, column, static: isStatic })
|
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
|
||||||
|
}
|
||||||
}),
|
}),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,3 +1,20 @@
|
||||||
export class JsErrorTimeout extends Error {
|
export class JsTimeoutError extends Error {
|
||||||
code = "ERR_SCRIPT_EXECUTION_TIMEOUT"
|
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()}`
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -3,6 +3,7 @@ import cloneDeep from "lodash/fp/cloneDeep"
|
||||||
import { LITERAL_MARKER } from "../helpers/constants"
|
import { LITERAL_MARKER } from "../helpers/constants"
|
||||||
import { getJsHelperList } from "./list"
|
import { getJsHelperList } from "./list"
|
||||||
import { iifeWrapper } from "../iife"
|
import { iifeWrapper } from "../iife"
|
||||||
|
import { JsTimeoutError, UserScriptError } from "../errors"
|
||||||
|
|
||||||
// The method of executing JS scripts depends on the bundle being built.
|
// 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).
|
// 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) {
|
} catch (error: any) {
|
||||||
onErrorLog && onErrorLog(error)
|
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") {
|
if (error.code === "ERR_SCRIPT_EXECUTION_TIMEOUT") {
|
||||||
return "Timed out while executing JS"
|
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"
|
return "Error while executing JS"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -16,6 +16,7 @@ import { removeJSRunner, setJSRunner } from "./helpers/javascript"
|
||||||
|
|
||||||
import manifest from "./manifest.json"
|
import manifest from "./manifest.json"
|
||||||
import { ProcessOptions } from "./types"
|
import { ProcessOptions } from "./types"
|
||||||
|
import { UserScriptError } from "./errors"
|
||||||
|
|
||||||
export { helpersToRemoveForJs, getJsHelperList } from "./helpers/list"
|
export { helpersToRemoveForJs, getJsHelperList } from "./helpers/list"
|
||||||
export { FIND_ANY_HBS_REGEX } from "./utilities"
|
export { FIND_ANY_HBS_REGEX } from "./utilities"
|
||||||
|
@ -229,7 +230,10 @@ export function processStringSync(
|
||||||
} else {
|
} else {
|
||||||
return process(string)
|
return process(string)
|
||||||
}
|
}
|
||||||
} catch (err) {
|
} catch (err: any) {
|
||||||
|
if (err.code === UserScriptError.code) {
|
||||||
|
throw err
|
||||||
|
}
|
||||||
return input
|
return input
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -448,7 +452,7 @@ export function convertToJS(hbs: string) {
|
||||||
return `${varBlock}${js}`
|
return `${varBlock}${js}`
|
||||||
}
|
}
|
||||||
|
|
||||||
export { JsErrorTimeout } from "./errors"
|
export { JsTimeoutError, UserScriptError } from "./errors"
|
||||||
|
|
||||||
export function defaultJSSetup() {
|
export function defaultJSSetup() {
|
||||||
if (!isBackendService()) {
|
if (!isBackendService()) {
|
||||||
|
@ -463,7 +467,27 @@ export function defaultJSSetup() {
|
||||||
setTimeout: undefined,
|
setTimeout: undefined,
|
||||||
}
|
}
|
||||||
createContext(context)
|
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 {
|
} else {
|
||||||
removeJSRunner()
|
removeJSRunner()
|
||||||
|
|
Loading…
Reference in New Issue