diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index 3585dacbed..ed8dc929d6 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -139,29 +139,61 @@ class InternalBuilder {
return this.table.schema[column]
}
- // Takes a string like foo and returns a quoted string like [foo] for SQL Server
- // and "foo" for Postgres.
- private quote(str: string): string {
+ private quoteChars(): [string, string] {
switch (this.client) {
- case SqlClient.SQL_LITE:
case SqlClient.ORACLE:
case SqlClient.POSTGRES:
- return `"${str}"`
+ return ['"', '"']
case SqlClient.MS_SQL:
- return `[${str}]`
+ return ["[", "]"]
case SqlClient.MARIADB:
case SqlClient.MY_SQL:
- return `\`${str}\``
+ case SqlClient.SQL_LITE:
+ return ["`", "`"]
}
}
- // Takes a string like a.b.c and returns a quoted identifier like [a].[b].[c]
- // for SQL Server and `a`.`b`.`c` for MySQL.
- private quotedIdentifier(key: string): string {
- return key
- .split(".")
- .map(part => this.quote(part))
- .join(".")
+ // Takes a string like foo and returns a quoted string like [foo] for SQL Server
+ // and "foo" for Postgres.
+ private quote(str: string): string {
+ const [start, end] = this.quoteChars()
+ return `${start}${str}${end}`
+ }
+
+ private isQuoted(key: string): boolean {
+ const [start, end] = this.quoteChars()
+ return key.startsWith(start) && key.endsWith(end)
+ }
+
+ // Takes a string like a.b.c or an array like ["a", "b", "c"] and returns a
+ // quoted identifier like [a].[b].[c] for SQL Server and `a`.`b`.`c` for
+ // MySQL.
+ private quotedIdentifier(key: string | string[]): string {
+ if (!Array.isArray(key)) {
+ key = this.splitIdentifier(key)
+ }
+ return key.map(part => this.quote(part)).join(".")
+ }
+
+ // Turns an identifier like a.b.c or `a`.`b`.`c` into ["a", "b", "c"]
+ private splitIdentifier(key: string): string[] {
+ const [start, end] = this.quoteChars()
+ if (this.isQuoted(key)) {
+ return key.slice(1, -1).split(`${end}.${start}`)
+ }
+ return key.split(".")
+ }
+
+ private qualifyIdentifier(key: string): string {
+ const tableName = this.getTableName()
+ const parts = this.splitIdentifier(key)
+ if (parts[0] !== tableName) {
+ parts.unshift(tableName)
+ }
+ if (this.isQuoted(key)) {
+ return this.quotedIdentifier(parts)
+ }
+ return parts.join(".")
}
private isFullSelectStatementRequired(): boolean {
@@ -231,8 +263,13 @@ class InternalBuilder {
// OracleDB can't use character-large-objects (CLOBs) in WHERE clauses,
// so when we use them we need to wrap them in to_char(). This function
// converts a field name to the appropriate identifier.
- private convertClobs(field: string): string {
- const parts = field.split(".")
+ private convertClobs(field: string, opts?: { forSelect?: boolean }): string {
+ if (this.client !== SqlClient.ORACLE) {
+ throw new Error(
+ "you've called convertClobs on a DB that's not Oracle, this is a mistake"
+ )
+ }
+ const parts = this.splitIdentifier(field)
const col = parts.pop()!
const schema = this.table.schema[col]
let identifier = this.quotedIdentifier(field)
@@ -244,7 +281,11 @@ class InternalBuilder {
schema.type === FieldType.OPTIONS ||
schema.type === FieldType.BARCODEQR
) {
- identifier = `to_char(${identifier})`
+ if (opts?.forSelect) {
+ identifier = `to_char(${identifier}) as ${this.quotedIdentifier(col)}`
+ } else {
+ identifier = `to_char(${identifier})`
+ }
}
return identifier
}
@@ -859,28 +900,58 @@ class InternalBuilder {
const fields = this.query.resource?.fields || []
const tableName = this.getTableName()
if (fields.length > 0) {
- query = query.groupBy(fields.map(field => `${tableName}.${field}`))
- query = query.select(fields.map(field => `${tableName}.${field}`))
+ const qualifiedFields = fields.map(field => this.qualifyIdentifier(field))
+ if (this.client === SqlClient.ORACLE) {
+ const groupByFields = qualifiedFields.map(field =>
+ this.convertClobs(field)
+ )
+ const selectFields = qualifiedFields.map(field =>
+ this.convertClobs(field, { forSelect: true })
+ )
+ query = query
+ .groupByRaw(groupByFields.join(", "))
+ .select(this.knex.raw(selectFields.join(", ")))
+ } else {
+ query = query.groupBy(qualifiedFields).select(qualifiedFields)
+ }
}
for (const aggregation of aggregations) {
const op = aggregation.calculationType
- const field = `${tableName}.${aggregation.field} as ${aggregation.name}`
- switch (op) {
- case CalculationType.COUNT:
- query = query.count(field)
- break
- case CalculationType.SUM:
- query = query.sum(field)
- break
- case CalculationType.AVG:
- query = query.avg(field)
- break
- case CalculationType.MIN:
- query = query.min(field)
- break
- case CalculationType.MAX:
- query = query.max(field)
- break
+ if (op === CalculationType.COUNT) {
+ if ("distinct" in aggregation && aggregation.distinct) {
+ if (this.client === SqlClient.ORACLE) {
+ const field = this.convertClobs(`${tableName}.${aggregation.field}`)
+ query = query.select(
+ this.knex.raw(
+ `COUNT(DISTINCT ${field}) as ${this.quotedIdentifier(
+ aggregation.name
+ )}`
+ )
+ )
+ } else {
+ query = query.countDistinct(
+ `${tableName}.${aggregation.field} as ${aggregation.name}`
+ )
+ }
+ } else {
+ query = query.count(`* as ${aggregation.name}`)
+ }
+ } else {
+ const field = `${tableName}.${aggregation.field} as ${aggregation.name}`
+ switch (op) {
+ case CalculationType.SUM:
+ query = query.sum(field)
+ break
+ case CalculationType.AVG:
+ query = query.avg(field)
+ break
+ case CalculationType.MIN:
+ query = query.min(field)
+ break
+ case CalculationType.MAX:
+ query = query.max(field)
+ break
+ }
}
}
return query
diff --git a/packages/builder/src/components/common/bindings/BindingPanel.svelte b/packages/builder/src/components/common/bindings/BindingPanel.svelte
index d8edf0cbb1..4186cb54cc 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,22 @@
}
const debouncedEval = Utils.debounce((expression, context, snippets) => {
- expressionResult = processStringSync(expression || "", {
- ...context,
- snippets,
- })
+ try {
+ expressionError = null
+ expressionResult = processStringSync(
+ expression || "",
+ {
+ ...context,
+ snippets,
+ },
+ {
+ noThrow: false,
+ }
+ )
+ } catch (err) {
+ expressionResult = null
+ expressionError = err
+ }
evaluating = false
}, 260)
@@ -370,6 +383,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/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts
index 4ce326c35f..ed8836626c 100644
--- a/packages/server/src/api/controllers/row/ExternalRequest.ts
+++ b/packages/server/src/api/controllers/row/ExternalRequest.ts
@@ -696,9 +696,8 @@ export class ExternalRequest
{
const calculationFields = helpers.views.calculationFields(this.source)
for (const [key, field] of Object.entries(calculationFields)) {
aggregations.push({
+ ...field,
name: key,
- field: field.field,
- calculationType: field.calculationType,
})
}
}
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/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts
index 0257c86ded..a0ffc5c460 100644
--- a/packages/server/src/api/controllers/view/viewsV2.ts
+++ b/packages/server/src/api/controllers/view/viewsV2.ts
@@ -11,14 +11,40 @@ import {
ViewCalculationFieldMetadata,
RelationSchemaField,
ViewFieldMetadata,
+ CalculationType,
} from "@budibase/types"
import { builderSocket, gridSocket } from "../../../websockets"
import { helpers } from "@budibase/shared-core"
function stripUnknownFields(
- field: BasicViewFieldMetadata
-): RequiredKeys {
+ field: ViewFieldMetadata
+): RequiredKeys {
if (helpers.views.isCalculationField(field)) {
+ if (field.calculationType === CalculationType.COUNT) {
+ if ("distinct" in field && field.distinct) {
+ return {
+ order: field.order,
+ width: field.width,
+ visible: field.visible,
+ readonly: field.readonly,
+ icon: field.icon,
+ distinct: field.distinct,
+ calculationType: field.calculationType,
+ field: field.field,
+ columns: field.columns,
+ }
+ } else {
+ return {
+ order: field.order,
+ width: field.width,
+ visible: field.visible,
+ readonly: field.readonly,
+ icon: field.icon,
+ calculationType: field.calculationType,
+ columns: field.columns,
+ }
+ }
+ }
const strippedField: RequiredKeys = {
order: field.order,
width: field.width,
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/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts
index 83b26bc687..a1a4475ee5 100644
--- a/packages/server/src/api/routes/tests/viewV2.spec.ts
+++ b/packages/server/src/api/routes/tests/viewV2.spec.ts
@@ -24,8 +24,8 @@ import {
RenameColumn,
FeatureFlag,
BBReferenceFieldSubType,
+ NumericCalculationFieldMetadata,
ViewV2Schema,
- ViewCalculationFieldMetadata,
} from "@budibase/types"
import { generator, mocks } from "@budibase/backend-core/tests"
import { DatabaseName, getDatasource } from "../../../integrations/tests/utils"
@@ -557,13 +557,13 @@ describe.each([
expect(Object.keys(view.schema!)).toHaveLength(1)
- let sum = view.schema!.sum as ViewCalculationFieldMetadata
+ let sum = view.schema!.sum as NumericCalculationFieldMetadata
expect(sum).toBeDefined()
expect(sum.calculationType).toEqual(CalculationType.SUM)
expect(sum.field).toEqual("Price")
view = await config.api.viewV2.get(view.id)
- sum = view.schema!.sum as ViewCalculationFieldMetadata
+ sum = view.schema!.sum as NumericCalculationFieldMetadata
expect(sum).toBeDefined()
expect(sum.calculationType).toEqual(CalculationType.SUM)
expect(sum.field).toEqual("Price")
@@ -864,6 +864,185 @@ describe.each([
}
)
})
+
+ !isLucene &&
+ describe("calculation views", () => {
+ let table: Table
+ let view: ViewV2
+
+ beforeEach(async () => {
+ table = await config.api.table.save(
+ saveTableRequest({
+ schema: {
+ name: {
+ name: "name",
+ type: FieldType.STRING,
+ constraints: {
+ presence: true,
+ },
+ },
+ country: {
+ name: "country",
+ type: FieldType.STRING,
+ },
+ age: {
+ name: "age",
+ type: FieldType.NUMBER,
+ },
+ },
+ })
+ )
+
+ view = await config.api.viewV2.create({
+ tableId: table._id!,
+ name: generator.guid(),
+ schema: {
+ country: {
+ visible: true,
+ },
+ age: {
+ visible: true,
+ calculationType: CalculationType.SUM,
+ field: "age",
+ },
+ },
+ })
+
+ await config.api.row.bulkImport(table._id!, {
+ rows: [
+ {
+ name: "Steve",
+ age: 30,
+ country: "UK",
+ },
+ {
+ name: "Jane",
+ age: 31,
+ country: "UK",
+ },
+ {
+ name: "Ruari",
+ age: 32,
+ country: "USA",
+ },
+ {
+ name: "Alice",
+ age: 33,
+ country: "USA",
+ },
+ ],
+ })
+ })
+
+ it("returns the expected rows prior to modification", async () => {
+ const { rows } = await config.api.row.search(view.id)
+ expect(rows).toHaveLength(2)
+ expect(rows).toEqual(
+ expect.arrayContaining([
+ {
+ country: "USA",
+ age: 65,
+ },
+ {
+ country: "UK",
+ age: 61,
+ },
+ ])
+ )
+ })
+
+ it("can remove a group by field", async () => {
+ delete view.schema!.country
+ await config.api.viewV2.update(view)
+
+ const { rows } = await config.api.row.search(view.id)
+ expect(rows).toHaveLength(1)
+ expect(rows).toEqual(
+ expect.arrayContaining([
+ {
+ age: 126,
+ },
+ ])
+ )
+ })
+
+ it("can remove a calculation field", async () => {
+ delete view.schema!.age
+ await config.api.viewV2.update(view)
+
+ const { rows } = await config.api.row.search(view.id)
+ expect(rows).toHaveLength(4)
+
+ // Because the removal of the calculation field actually makes this
+ // no longer a calculation view, these rows will now have _id and
+ // _rev fields.
+ expect(rows).toEqual(
+ expect.arrayContaining([
+ expect.objectContaining({ country: "UK" }),
+ expect.objectContaining({ country: "UK" }),
+ expect.objectContaining({ country: "USA" }),
+ expect.objectContaining({ country: "USA" }),
+ ])
+ )
+ })
+
+ it("can add a new group by field", async () => {
+ view.schema!.name = { visible: true }
+ await config.api.viewV2.update(view)
+
+ const { rows } = await config.api.row.search(view.id)
+ expect(rows).toHaveLength(4)
+ expect(rows).toEqual(
+ expect.arrayContaining([
+ {
+ name: "Steve",
+ age: 30,
+ country: "UK",
+ },
+ {
+ name: "Jane",
+ age: 31,
+ country: "UK",
+ },
+ {
+ name: "Ruari",
+ age: 32,
+ country: "USA",
+ },
+ {
+ name: "Alice",
+ age: 33,
+ country: "USA",
+ },
+ ])
+ )
+ })
+
+ it("can add a new calculation field", async () => {
+ view.schema!.count = {
+ visible: true,
+ calculationType: CalculationType.COUNT,
+ }
+ await config.api.viewV2.update(view)
+
+ const { rows } = await config.api.row.search(view.id)
+ expect(rows).toHaveLength(2)
+ expect(rows).toEqual(
+ expect.arrayContaining([
+ {
+ country: "USA",
+ age: 65,
+ count: 2,
+ },
+ {
+ country: "UK",
+ age: 61,
+ count: 2,
+ },
+ ])
+ )
+ })
+ })
})
describe("delete", () => {
@@ -2570,6 +2749,74 @@ describe.each([
expect(actual).toEqual(expected)
}
})
+
+ it("should be able to do a COUNT(DISTINCT)", async () => {
+ const table = await config.api.table.save(
+ saveTableRequest({
+ schema: {
+ name: {
+ name: "name",
+ type: FieldType.STRING,
+ },
+ },
+ })
+ )
+
+ const view = await config.api.viewV2.create({
+ tableId: table._id!,
+ name: generator.guid(),
+ schema: {
+ count: {
+ visible: true,
+ calculationType: CalculationType.COUNT,
+ distinct: true,
+ field: "name",
+ },
+ },
+ })
+
+ await config.api.row.bulkImport(table._id!, {
+ rows: [
+ {
+ name: "John",
+ },
+ {
+ name: "John",
+ },
+ {
+ name: "Sue",
+ },
+ ],
+ })
+
+ const { rows } = await config.api.row.search(view.id)
+ expect(rows).toHaveLength(1)
+ expect(rows[0].count).toEqual(2)
+ })
+
+ it("should not be able to COUNT(DISTINCT ...) against a non-existent field", async () => {
+ await config.api.viewV2.create(
+ {
+ tableId: table._id!,
+ name: generator.guid(),
+ schema: {
+ count: {
+ visible: true,
+ calculationType: CalculationType.COUNT,
+ distinct: true,
+ field: "does not exist oh no",
+ },
+ },
+ },
+ {
+ status: 400,
+ body: {
+ message:
+ 'Calculation field "count" references field "does not exist oh no" which does not exist in the table schema',
+ },
+ }
+ )
+ })
})
!isLucene &&
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..c7ab86f014 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")()`
- )
- expect(output).toBe("Error while executing JS")
+ expect(
+ await processJS(
+ `return this.constructor.constructor("return process.env")()`
+ )
+ ).toEqual("ReferenceError: process is not defined")
})
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/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts
index f0e47ce2b7..3847eb8f31 100644
--- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts
+++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts
@@ -1,5 +1,6 @@
import {
Aggregation,
+ CalculationType,
Datasource,
DocumentType,
FieldType,
@@ -369,11 +370,27 @@ export async function search(
continue
}
- aggregations.push({
- name: key,
- field: mapToUserColumn(field.field),
- calculationType: field.calculationType,
- })
+ if (field.calculationType === CalculationType.COUNT) {
+ if ("distinct" in field && field.distinct) {
+ aggregations.push({
+ name: key,
+ distinct: true,
+ field: mapToUserColumn(field.field),
+ calculationType: field.calculationType,
+ })
+ } else {
+ aggregations.push({
+ name: key,
+ calculationType: field.calculationType,
+ })
+ }
+ } else {
+ aggregations.push({
+ name: key,
+ field: mapToUserColumn(field.field),
+ calculationType: field.calculationType,
+ })
+ }
}
}
diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts
index 83ee78e165..73785edd98 100644
--- a/packages/server/src/sdk/app/views/index.ts
+++ b/packages/server/src/sdk/app/views/index.ts
@@ -1,4 +1,5 @@
import {
+ CalculationType,
FieldType,
PermissionLevel,
RelationSchemaField,
@@ -65,6 +66,15 @@ async function guardCalculationViewSchema(
const calculationFields = helpers.views.calculationFields(view)
for (const calculationFieldName of Object.keys(calculationFields)) {
const schema = calculationFields[calculationFieldName]
+ const isCount = schema.calculationType === CalculationType.COUNT
+ const isDistinct = isCount && "distinct" in schema && schema.distinct
+
+ // Count fields that aren't distinct don't need to reference another field,
+ // so we don't validate it.
+ if (isCount && !isDistinct) {
+ continue
+ }
+
const targetSchema = table.schema[schema.field]
if (!targetSchema) {
throw new HTTPError(
@@ -73,7 +83,7 @@ async function guardCalculationViewSchema(
)
}
- if (!helpers.schema.isNumeric(targetSchema)) {
+ if (!isCount && !helpers.schema.isNumeric(targetSchema)) {
throw new HTTPError(
`Calculation field "${calculationFieldName}" references field "${schema.field}" which is not a numeric field`,
400
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..2f850b0533 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,49 @@ export function processJS(handlebars: string, context: any) {
} catch (error: any) {
onErrorLog && onErrorLog(error)
+ const { noThrow = true } = context.__opts || {}
+
+ // 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) {
+ if (noThrow) {
+ return error.userScriptError.toString()
+ }
+ throw error
+ }
+
+ if (error.name === "SyntaxError") {
+ if (noThrow) {
+ return error.toString()
+ }
+ throw error
+ }
+
return "Error while executing JS"
}
}
diff --git a/packages/string-templates/src/index.ts b/packages/string-templates/src/index.ts
index 8d5fe4c16d..c2be63978d 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"
@@ -122,7 +123,7 @@ function createTemplate(
export async function processObject>(
object: T,
context: object,
- opts?: { noHelpers?: boolean; escapeNewlines?: boolean; onlyFound?: boolean }
+ opts?: ProcessOptions
): Promise {
testObject(object)
@@ -172,7 +173,7 @@ export async function processString(
export function processObjectSync(
object: { [x: string]: any },
context: any,
- opts: any
+ opts?: ProcessOptions
): object | Array {
testObject(object)
for (let key of Object.keys(object || {})) {
@@ -229,8 +230,12 @@ export function processStringSync(
} else {
return process(string)
}
- } catch (err) {
- return input
+ } catch (err: any) {
+ const { noThrow = true } = opts || {}
+ if (noThrow) {
+ return input
+ }
+ throw err
}
}
@@ -448,7 +453,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 +468,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()
diff --git a/packages/string-templates/src/types.ts b/packages/string-templates/src/types.ts
index 19785b9273..2a7a430bee 100644
--- a/packages/string-templates/src/types.ts
+++ b/packages/string-templates/src/types.ts
@@ -3,6 +3,7 @@ export interface ProcessOptions {
noEscaping?: boolean
noHelpers?: boolean
noFinalise?: boolean
+ noThrow?: boolean
escapeNewlines?: boolean
onlyFound?: boolean
disabledHelpers?: string[]
diff --git a/packages/types/src/documents/app/view.ts b/packages/types/src/documents/app/view.ts
index 77bbb3e4d1..5fcc9049c1 100644
--- a/packages/types/src/documents/app/view.ts
+++ b/packages/types/src/documents/app/view.ts
@@ -42,11 +42,31 @@ export interface RelationSchemaField extends UIFieldMetadata {
readonly?: boolean
}
-export interface ViewCalculationFieldMetadata extends BasicViewFieldMetadata {
- calculationType: CalculationType
+export interface NumericCalculationFieldMetadata
+ extends BasicViewFieldMetadata {
+ calculationType:
+ | CalculationType.MIN
+ | CalculationType.MAX
+ | CalculationType.SUM
+ | CalculationType.AVG
field: string
}
+export interface CountCalculationFieldMetadata extends BasicViewFieldMetadata {
+ calculationType: CalculationType.COUNT
+}
+
+export interface CountDistinctCalculationFieldMetadata
+ extends CountCalculationFieldMetadata {
+ distinct: true
+ field: string
+}
+
+export type ViewCalculationFieldMetadata =
+ | NumericCalculationFieldMetadata
+ | CountCalculationFieldMetadata
+ | CountDistinctCalculationFieldMetadata
+
export type ViewFieldMetadata =
| BasicViewFieldMetadata
| ViewCalculationFieldMetadata
diff --git a/packages/types/src/sdk/row.ts b/packages/types/src/sdk/row.ts
index 0b63e88229..ea46e82508 100644
--- a/packages/types/src/sdk/row.ts
+++ b/packages/types/src/sdk/row.ts
@@ -3,12 +3,33 @@ import { SearchFilters } from "./search"
import { CalculationType, Row } from "../documents"
import { WithRequired } from "../shared"
-export interface Aggregation {
+export interface BaseAggregation {
name: string
- calculationType: CalculationType
+}
+
+export interface NumericAggregation extends BaseAggregation {
+ calculationType:
+ | CalculationType.AVG
+ | CalculationType.MAX
+ | CalculationType.MIN
+ | CalculationType.SUM
field: string
}
+export interface CountAggregation extends BaseAggregation {
+ calculationType: CalculationType.COUNT
+}
+
+export interface CountDistinctAggregation extends CountAggregation {
+ distinct: true
+ field: string
+}
+
+export type Aggregation =
+ | NumericAggregation
+ | CountAggregation
+ | CountDistinctAggregation
+
export interface SearchParams {
tableId?: string
viewId?: string