diff --git a/apps/core/src/operation-channel.ts b/apps/core/src/operation-channel.ts index 1ef5ddc9c..4f2cdd1fd 100644 --- a/apps/core/src/operation-channel.ts +++ b/apps/core/src/operation-channel.ts @@ -46,11 +46,25 @@ export interface ChannelCapabilities { readonly resource?: Capability; /** Brands an expected error the channel raised as the host's. */ readonly trust: (error: Error) => void; - /** The refusal for each capability it hasn't. */ + /** + * Told of every use the channel refused or that failed, wherever it + * failed (its arguments refused at the boundary included), with the + * host's own copy of the error. + */ + readonly refused?: ( + name: "read" | "write" | "call" | "resource", + error: CodedError + ) => void; + /** + * The refusal for each capability it hasn't (`operation.not_found` for + * one without its own). + */ readonly refusals: Readonly< - Record< - "read" | "write" | "call" | "resource", - Parameters[0] + Partial< + Record< + "read" | "write" | "call" | "resource", + Parameters[0] + > > >; } @@ -147,7 +161,7 @@ class Channel extends ChannelTarget implements OperationChannel { } async #use( - name: keyof ChannelCapabilities["refusals"], + name: "read" | "write" | "call" | "resource", args: unknown[] ): Promise { const capability = this.#capabilities[name]; @@ -156,12 +170,14 @@ class Channel extends ChannelTarget implements OperationChannel { throw operationErrors.create("operation.ended"); } if (capability === undefined) { - throw operationErrors.create(this.#capabilities.refusals[name]); + throw operationErrors.create( + this.#capabilities.refusals[name] ?? "operation.not_found" + ); } return await capability(...plainArgs(args)); } catch (error) { if (isExpectedError(error)) { - throw this.#raise(error); + throw this.#raise(name, error); } // A domain failure of a function this one called, as the host // rebuilt it: App code may handle it, or throw it on. @@ -176,6 +192,7 @@ class Channel extends ChannelTarget implements OperationChannel { ...errorFields(error), }); throw this.#raise( + name, internalErrors.create("internal.unexpected", { requestId }) ); } @@ -187,10 +204,14 @@ class Channel extends ChannelTarget implements OperationChannel { * so whatever App code does to the error it caught (its message, code or * details, now or after a later await) changes nothing the host answers. */ - #raise(error: CodedError): Error { + #raise( + name: "read" | "write" | "call" | "resource", + error: CodedError + ): Error { const kept = copyOf(error); this.#capabilities.trust(kept); this.#state.raised.set(kept.code, kept); + this.#capabilities.refused?.(name, kept); return copyOf(error); } } diff --git a/apps/core/src/operations.ts b/apps/core/src/operations.ts index d69cecefc..3ae31d540 100644 --- a/apps/core/src/operations.ts +++ b/apps/core/src/operations.ts @@ -12,6 +12,7 @@ import { } from "@grasp-os/sdk/host"; import type { AppManifest, OperationManifest } from "@grasp-os/sdk/host"; import type { OperationChannel } from "@grasp-os/sdk/runner"; +import { appCallLimits } from "@grasp-os/shared/apps"; import { sha256Hex } from "@grasp-os/shared/encoding"; import { internalErrors, isExpectedError } from "@grasp-os/shared/errors"; import { canonicalJson } from "@grasp-os/shared/json"; @@ -95,8 +96,9 @@ import { // manifest, hashed here; no channel method takes any of them. // - A query writes, a mutation reaches outside, an action holds the // database: each kind's channel has only its capabilities, whatever the -// App code sends (operation-channel.ts), and actions don't run in this -// release's host at all yet. +// App code sends (operation-channel.ts): an action reaches data only +// through the App's queries and mutations it lists, each an invocation +// of its own. // - A function's access is widened past its declaration: every read and // staged write is checked against the verbs inspection gave the // function (its kind and `operationAccess`, narrowing only), and the @@ -714,6 +716,17 @@ interface Staging { /** The bytes of the writes staged so far, as UTF-8 JSON. */ readonly bytes: () => number; readonly stage: (request: unknown) => Promise; + /** + * The first write the host refused to stage, if any: App code may not + * have waited for it, or may have caught it, but its change set lost a + * write, so the mutation can't commit as though it hadn't. + */ + readonly refused: () => Error | undefined; + /** + * Remembers a refused write: told by the channel, wherever the write + * was refused (at its boundary, before staging, included). + */ + readonly remember: (error: Error) => void; readonly preview: () => StagedWrites | undefined; readonly writes: () => StagedWrites["writes"]; } @@ -779,6 +792,7 @@ const stagingFor = ( staged += bytes; writes.push(write); }; + let refused: Error | undefined; const stage = async (request: unknown): Promise => { const write = operationErrors.parse( "contract.invalid_write", @@ -876,6 +890,10 @@ const stagingFor = ( stagedAt, bytes: () => staged, stage, + refused: () => refused, + remember: (error: Error) => { + refused ??= error; + }, preview: () => writes.length === 0 ? undefined : { stagedAt, writes: [...writes] }, writes: () => [...writes], @@ -1198,7 +1216,7 @@ const scopeOf = async ( * own function and three nested in it, as calls between Apps * (`appCallLimits.depth`). */ -export const callMaxDepth = 3; +export const callMaxDepth = appCallLimits.depth; /** One function's run, and where it sits in the call it belongs to. */ interface Run { @@ -1229,9 +1247,28 @@ export const callMaxResourceRequests = 32; interface CallBudget { nested: number; requests: number; - readonly reads: ReadBudget; + /** + * The current run's reads: each run of the call's own function (a + * query run again for stale reads, a mutation's next attempt) starts a + * fresh one, which everything nested in that run shares. The limits are + * per run, as their names say; the bounded number of reruns bounds the + * whole call. + */ + reads: ReadBudget; } +/** + * The read budget of a run of `run`'s function: a fresh one, shared by + * what it calls, when it is the call's own function; the run it is nested + * in's otherwise, so nested calls never multiply what one run reads. + */ +const runReads = (run: Run): ReadBudget => { + if (run.depth === 0) { + run.budget.reads = readBudget(); + } + return run.budget.reads; +}; + /** A new call's budget. */ const callBudget = (): CallBudget => ({ nested: 0, @@ -1313,8 +1350,15 @@ const trackedCalls = ( if (left <= 0) { throw hostError(operationErrors.create("operation.unfinished_call")); } + // The timer is cleared once the calls settle first, so no wait is + // left pending past the run. + const timer = new AbortController(); // oxlint-disable-next-line no-await-in-loop -- until none is left - await Promise.race([Promise.allSettled(running), scheduler.wait(left)]); + await Promise.race([ + Promise.allSettled(running), + scheduler.wait(left, { signal: timer.signal }).catch(() => null), + ]); + timer.abort(); } return unfinished?.error; }, @@ -1326,6 +1370,11 @@ const staleRun = new Error("A run's reads went stale"); /** The capabilities one handler's channel gets, besides its refusals. */ interface Capabilities { + /** Told of every refused or failed use, as the channel's `refused`. */ + readonly onRefused?: ( + name: "read" | "write" | "call" | "resource", + error: Error + ) => void; readonly read?: (request: unknown) => Promise; readonly write?: (request: unknown) => Promise; readonly call?: ( @@ -1343,22 +1392,14 @@ interface Capabilities { /** Why each kind's channel refuses what it hasn't. */ const refusalsOf = { query: { - read: "operation.no_database", write: "operation.read_only", call: "operation.read_only", resource: "operation.no_resources", }, - mutation: { - read: "operation.no_database", - write: "operation.read_only", - call: "operation.not_found", - resource: "operation.no_resources", - }, + mutation: { resource: "operation.no_resources" }, action: { read: "operation.no_database", write: "operation.no_database", - call: "operation.not_found", - resource: "operation.no_resources", }, } as const; @@ -1382,6 +1423,9 @@ const runHandler = async ( path: operation.path, ...calls.capabilities, trust, + ...(capabilities.onRefused === undefined + ? {} + : { refused: capabilities.onRefused }), refusals: refusalsOf[operation.kind], }); try { @@ -1587,7 +1631,7 @@ const runQuery = async ( return await trusted( async () => await readConsistently(invocation.store, scope, async (reader) => { - const budget = run.budget.reads; + const budget = runReads(run); return await runHandler(run, operation, args, { read: async (request) => { spendRead(budget, 0); @@ -1640,7 +1684,7 @@ const runMutation = async ( const scope = await scopeOf(compiled, invocation, roles); const staging = stagingFor(compiled, operation, Date.now()); const reader = new StoreReader(store, scope, staging.preview); - const budget = run.budget.reads; + const budget = runReads(run); const spend = () => { spendRead(budget, staging.bytes()); }; @@ -1659,6 +1703,14 @@ const runMutation = async ( return await readFor(operation, reader, request); }, write: staging.stage, + // Any write refused, at the channel's boundary or in staging, + // fails the mutation (`refused`); calls and resources stay + // catchable. + onRefused: (name, error) => { + if (name === "write") { + staging.remember(error); + } + }, call: async (kind, path, callArgs) => await nestedCall( run, @@ -1676,6 +1728,10 @@ const runMutation = async ( } throw error; } + const refusedWrite = staging.refused(); + if (refusedWrite !== undefined) { + throw sealed(hostError(refusedWrite)); + } // oxlint-disable-next-line no-await-in-loop -- see above const current = await trusted(invocation.authorize); // Its reads were scoped for roles the caller no longer has: run again diff --git a/apps/core/src/store-reader.ts b/apps/core/src/store-reader.ts index 104525d25..0f0330f24 100644 --- a/apps/core/src/store-reader.ts +++ b/apps/core/src/store-reader.ts @@ -1,6 +1,10 @@ import type { ReadScope } from "@grasp-os/shared/store-queries"; import type { ReadRequest } from "@grasp-os/shared/store-reads"; -import { queryErrors } from "@grasp-os/shared/stores"; +import { + commitMaxGuards, + dataErrors, + queryErrors, +} from "@grasp-os/shared/stores"; import type { StagedWrites } from "@grasp-os/shared/stores"; import type { OpenStore } from "./data-stores.ts"; @@ -42,8 +46,8 @@ export interface ReadGuard { /** * The generations a failed read's error carries (`details.generations`, - * StoreHost's `#withGenerations`), taken off it, so App code never sees - * them; none for an error raised before anything was read. + * StoreHost's `#readIn`), taken off it, so App code never sees them; none + * for an error raised before the read looked at a record. */ const takeGenerations = (error: unknown): Record => { if (typeof error !== "object" || error === null) { @@ -129,6 +133,16 @@ export class StoreReader { ({ table, id }) => table === request.table && id === request.id ) ) { + // One more than a commit can check would fail only at the commit, + // after the whole mutation ran: refused here, at the read. Only a + // mutation's reader (it stages writes) has guards a commit checks; a + // plain query never commits, so its reads are not capped here. + if ( + this.#staged !== undefined && + this.#guards.length >= commitMaxGuards + ) { + throw dataErrors.create("data.guard_limit"); + } this.#guards.push({ table: request.table, id: request.id, diff --git a/apps/core/test/operations.test.ts b/apps/core/test/operations.test.ts index e5c003fec..77cda3c54 100644 --- a/apps/core/test/operations.test.ts +++ b/apps/core/test/operations.test.ts @@ -252,6 +252,60 @@ const guardedOrFallback = mutation({ }, }); +/** Reads the table `reads` times. */ +const readTimes = query({ + args: { reads: v.number().integer().min(0).max(1000) }, + returns: v.number(), + handler: async (ctx, args) => { + for (let index = 0; index < args.reads; index += 1) { + // oxlint-disable-next-line no-await-in-loop -- one read after the other + await ctx.db.query("notes").first(); + } + return args.reads; + }, +}); + +/** + * Stages an insert its schema refuses without waiting for it, then + * stages one it accepts. + */ +const floatingBadWrite = mutation({ + args: {}, + returns: v.string(), + handler: async (ctx) => { + ctx.db.insert("notes", { title: " " }).catch(() => null); + await ctx.db.query("notes").first(); + await ctx.db.insert("notes", { title: "kept" }); + return "done"; + }, +}); + +/** Reads each of `ids` asserting it is at revision 1. */ +const assertMany = mutation({ + args: { ids: v.array(v.id("notes")).max(200) }, + returns: v.number(), + handler: async (ctx, args) => { + for (const id of args.ids) { + // oxlint-disable-next-line no-await-in-loop -- one read after the other + await ctx.db.get("notes", id, { expectedRevision: 1 }); + } + return args.ids.length; + }, +}); + +/** Reads each of `ids` asserting it is at revision 1, as a query. */ +const assertManyInQuery = query({ + args: { ids: v.array(v.id("notes")).max(200) }, + returns: v.number(), + handler: async (ctx, args) => { + for (const id of args.ids) { + // oxlint-disable-next-line no-await-in-loop -- one read after the other + await ctx.db.get("notes", id, { expectedRevision: 1 }); + } + return args.ids.length; + }, +}); + const secret = internalQuery({ args: {}, returns: v.string(), @@ -278,6 +332,10 @@ const notesApp = app({ editTwice, takeFirst, guardedOrFallback, + readTimes, + floatingBadWrite, + assertMany, + assertManyInQuery, }, }, internalOperations: { secret }, @@ -1089,6 +1147,122 @@ describe("a mutation's own staged state", () => { }); }); +describe("audit follow-ups", () => { + it("gives every run of a call its own reads, so reruns under contention still answer", async () => { + const fixture = await notes(); + const { owner } = fixture; + let runs = 0; + // Another caller commits during each of the first two runs. + const run: Runner = async (path, args, channel) => { + if (path === "notes.readTimes") { + runs += 1; + } + const answer = await runFunction(notesApp, path, args, channel); + if (path === "notes.readTimes" && runs < 3) { + await call(fixture, owner, "mutation", "notes.create", { + title: `c${runs}`, + }); + } + return answer; + }; + expect({ + answer: valueOf( + await call( + fixture, + owner, + "query", + "notes.readTimes", + { reads: 100 }, + { run } + ) + ), + runs, + }).toStrictEqual({ answer: 100, runs: 3 }); + }); + + it("gives every attempt of a mutation its own staged-bytes budget", async () => { + const fixture = await notes(); + const { owner } = fixture; + let runs = 0; + const run: Runner = async (path, args, channel) => { + if (path === "notes.stress") { + runs += 1; + } + const answer = await runFunction(notesApp, path, args, channel); + if (path === "notes.stress" && runs === 1) { + await call(fixture, owner, "mutation", "notes.create", { + title: "meanwhile", + }); + } + return answer; + }; + expect({ + answer: codeOf( + await call( + fixture, + owner, + "mutation", + "notes.stress", + { inserts: 6, size: 100_000, reads: 20 }, + { run } + ) + ), + runs, + }).toStrictEqual({ answer: "ok", runs: 2 }); + }); + + it("fails a mutation whose staged write was refused, awaited or not", async () => { + const fixture = await notes(); + const { owner } = fixture; + const result = await call( + fixture, + owner, + "mutation", + "notes.floatingBadWrite", + {} + ); + const page = valueOf( + await call(fixture, owner, "query", "notes.list", { pagination: {} }) + ); + expect({ code: codeOf(result), stored: idsOf(page).length }).toStrictEqual({ + code: "data.invalid", + stored: 0, + }); + }); + + it("refuses the read that asserts one revision more than a commit holds", async () => { + const fixture = await notes(); + const { owner } = fixture; + const ids = await Promise.all( + Array.from({ length: 101 }, async (_, index) => + valueOf( + await call(fixture, owner, "mutation", "notes.create", { + title: `n${index}`, + }) + ) + ) + ); + expect({ + within: codeOf( + await call(fixture, owner, "mutation", "notes.assertMany", { + ids: ids.slice(0, 100), + }) + ), + past: failureOf( + await call(fixture, owner, "mutation", "notes.assertMany", { ids }) + ), + }).toStrictEqual({ + within: "ok", + past: { + code: "data.guard_limit", + message: "A mutation asserts the revisions of at most 100 records.", + category: "contract", + retryable: false, + }, + }); + }); +}); + describe("bounds on one mutation", () => { it("refuses the staged write that takes a mutation past what its commit holds", async () => { const fixture = await notes(); @@ -1460,4 +1634,108 @@ describe("App code misusing its channel", () => { }, }); }); + + it("keeps the guard limit to mutations: a plain query may assert as many revisions as it reads", async () => { + const fixture = await notes(); + const { owner } = fixture; + const ids = await Promise.all( + Array.from({ length: 101 }, async (_, index) => + valueOf( + await call(fixture, owner, "mutation", "notes.create", { + title: `q${index}`, + }) + ) + ) + ); + expect({ + query: valueOf( + await call(fixture, owner, "query", "notes.assertManyInQuery", { ids }) + ), + mutation: codeOf( + await call(fixture, owner, "mutation", "notes.assertMany", { ids }) + ), + }).toStrictEqual({ query: 101, mutation: "data.guard_limit" }); + }); + + it("fails a mutation whose write was refused at the channel, before staging", async () => { + const fixture = await notes(); + const { owner } = fixture; + // A write that isn't JSON (NaN), caught, then a valid one. + const caughtNaN: Runner = async (path, args, channel) => { + if (path !== "notes.floatingBadWrite") { + return await runFunction(notesApp, path, args, channel); + } + try { + await channel.write({ + kind: "insert", + table: "notes", + fields: { title: "bad", body: Number.NaN }, + }); + } catch { + await channel.write({ + kind: "insert", + table: "notes", + fields: { title: "good" }, + }); + } + return "done"; + }; + const result = await call( + fixture, + owner, + "mutation", + "notes.floatingBadWrite", + {}, + { run: caughtNaN } + ); + const page = valueOf( + await call(fixture, owner, "query", "notes.list", { pagination: {} }) + ); + expect({ code: codeOf(result), stored: idsOf(page).length }).toStrictEqual({ + code: "contract.invalid_request", + stored: 0, + }); + }); + + it("lets a mutation catch a refused nested call or resource request and still commit", async () => { + const fixture = await notes(); + const { owner } = fixture; + const caughtOthers: Runner = async (path, args, channel) => { + if (path !== "notes.floatingBadWrite") { + return await runFunction(notesApp, path, args, channel); + } + const refused: unknown[] = []; + try { + await channel.resource("mail", "call", ["send", {}]); + } catch (error) { + refused.push(error); + } + try { + await channel.call("query", "notes.missing", {}); + } catch (error) { + refused.push(error); + } + await channel.write({ + kind: "insert", + table: "notes", + fields: { title: "kept" }, + }); + return `${refused.length}`; + }; + const result = await call( + fixture, + owner, + "mutation", + "notes.floatingBadWrite", + {}, + { run: caughtOthers } + ); + const page = valueOf( + await call(fixture, owner, "query", "notes.list", { pagination: {} }) + ); + expect({ + value: valueOf(result), + stored: idsOf(page).length, + }).toStrictEqual({ value: "2", stored: 1 }); + }); }); diff --git a/packages/sdk/src/server.ts b/packages/sdk/src/server.ts index eb97082df..e69fe82bc 100644 --- a/packages/sdk/src/server.ts +++ b/packages/sdk/src/server.ts @@ -158,6 +158,22 @@ export interface MutationCtx< * What an action's handler has: the functions and resources it `uses`. * No database: what it reads and writes goes through the App's queries * and mutations, each committed on its own. + * + * An action is not atomic across its calls. A query it runs and a + * mutation it runs after are two invocations: nothing fences the + * mutation on what the query read, so a record can change between them, + * and a mutation that committed stays committed if the action fails + * later. Read and change together in one mutation when the change depends + * on the read (a mutation's `expectedRevision` and read set fence it). + * + * Called again under the same idempotency key with the same input, an + * action gets the earlier outcomes of the mutations and resource requests + * it makes, matched by their order among those (queries don't count). + * That order follows the data: if, on the second call, the data has the + * action run the same mutation at a place with other arguments, it gets + * `submission.key_conflict`; a different mutation at that place runs + * anew. Keep what an action changes, and in what order, a function of its + * input. */ export interface ActionCtx { readonly runQuery: RunQuery; diff --git a/packages/shared/src/operations.ts b/packages/shared/src/operations.ts index ecd9ce623..5bea185ba 100644 --- a/packages/shared/src/operations.ts +++ b/packages/shared/src/operations.ts @@ -143,6 +143,7 @@ export const categoryOf = (code: string): ErrorCategory => { code === "data.unknown_table" || code === "data.unknown_index" || code === "data.invalid_cursor" || + code === "data.guard_limit" || code === "submission.key_conflict" || code === "submission.key_invalid" ) { diff --git a/packages/shared/src/stores.ts b/packages/shared/src/stores.ts index 2f7b6ee34..b276ce285 100644 --- a/packages/shared/src/stores.ts +++ b/packages/shared/src/stores.ts @@ -407,6 +407,7 @@ export const dataErrors = defineErrorFamily({ "data.unknown_index": "That table has no such index.", "data.access_denied": "You can't read this table.", "data.write_denied": "You can't change records of this table.", + "data.guard_limit": `A mutation asserts the revisions of at most ${commitMaxGuards} records.`, "data.reads_changed": "The records this change read changed before it committed. Try again.", "data.invalid_query": "That query isn't valid for this table.",