Merge branch 'master' into fix/related-rows-dont-exist

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
+  private quoteChars(): [string, string] {
-  // and "foo" for Postgres.
-  private quote(str: 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]
+  // Takes a string like foo and returns a quoted string like [foo] for SQL Server
-  // for SQL Server and `a`.`b`.`c` for MySQL.
+  // and "foo" for Postgres.
-  private quotedIdentifier(key: string): string {
+  private quote(str: string): string {
-    return key
+    const [start, end] = this.quoteChars()
-      .split(".")
+    return `${start}${str}${end}`
-      .map(part => this.quote(part))
+  }
-      .join(".")
+
+  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 {
+  private convertClobs(field: string, opts?: { forSelect?: boolean }): string {
-    const parts = field.split(".")
+    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,8 +281,12 @@ class InternalBuilder {
       schema.type === FieldType.OPTIONS ||
       schema.type === FieldType.BARCODEQR
     ) {
+      if (opts?.forSelect) {
+        identifier = `to_char(${identifier}) as ${this.quotedIdentifier(col)}`
+      } else {
         identifier = `to_char(${identifier})`
       }
+    }
     return identifier
   }
 
@@ -859,16 +900,45 @@ class InternalBuilder {
     const fields = this.query.resource?.fields || []
     const tableName = this.getTableName()
     if (fields.length > 0) {
-      query = query.groupBy(fields.map(field => `${tableName}.${field}`))
+      const qualifiedFields = fields.map(field => this.qualifyIdentifier(field))
-      query = query.select(fields.map(field => `${tableName}.${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
+      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.COUNT:
-          query = query.count(field)
-          break
           case CalculationType.SUM:
             query = query.sum(field)
             break
@@ -883,6 +953,7 @@ class InternalBuilder {
             break
         }
       }
+    }
     return query
   }
 

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 || "", {
+    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}
         <EvaluationSidePanel
           {expressionResult}
+          {expressionError}
           {evaluating}
           expression={editorValue}
         />

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 @@
   <div class="body">
     {#if empty}
       Your expression will be evaluated here
+    {:else if error}
+      {formatError(expressionError)}
     {:else}
       <!-- eslint-disable-next-line svelte/no-at-html-tags-->
       {@html highlightedResult}

packages/server/src/api/controllers/row/ExternalRequest.ts

@@ -696,9 +696,8 @@ export class ExternalRequest<T extends Operation> {
       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,
         })
       }
     }

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()
+  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) {

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
+  field: ViewFieldMetadata
-): RequiredKeys<BasicViewFieldMetadata> {
+): RequiredKeys<ViewFieldMetadata> {
   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<ViewCalculationFieldMetadata> = {
       order: field.order,
       width: field.width,

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 ")
             }
           }
         }

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 &&

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
       }

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(
+    expect(
+      await processJS(
         `return this.constructor.constructor("return process.env")()`
       )
-    expect(output).toBe("Error while executing JS")
+    ).toEqual("ReferenceError: process is not defined")
   })
 
   describe("helpers", () => {

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 {
+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 {
@@ -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]
   }
 

packages/server/src/sdk/app/rows/search/internal/sqs.ts

@@ -1,5 +1,6 @@
 import {
   Aggregation,
+  CalculationType,
   Datasource,
   DocumentType,
   FieldType,
@@ -369,6 +370,21 @@ export async function search(
         continue
       }
 
+      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),
@@ -376,6 +392,7 @@ export async function search(
         })
       }
     }
+  }
 
   const request: QueryJson = {
     endpoint: {

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

packages/string-templates/src/errors.ts

@@ -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()}`
+    )
+  }
 }

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"
   }
 }

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<T extends Record<string, any>>(
   object: T,
   context: object,
-  opts?: { noHelpers?: boolean; escapeNewlines?: boolean; onlyFound?: boolean }
+  opts?: ProcessOptions
 ): Promise<T> {
   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<any> {
   testObject(object)
   for (let key of Object.keys(object || {})) {
@@ -229,9 +230,13 @@ export function processStringSync(
     } else {
       return process(string)
     }
-  } catch (err) {
+  } 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()

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[]

packages/types/src/documents/app/view.ts

@@ -42,11 +42,31 @@ export interface RelationSchemaField extends UIFieldMetadata {
   readonly?: boolean
 }
 
-export interface ViewCalculationFieldMetadata extends BasicViewFieldMetadata {
+export interface NumericCalculationFieldMetadata
-  calculationType: CalculationType
+  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

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